Non-threaded file transfers

This change is very technical, but it's also a very big one. Qt sockets aren't generally supposed to be used in secondary threads, instead each socket emits a signal that says when data is available to read on the socket, when data was successfully written, when the socket disconnects... I originally implemented file transfer in SoulseekQt using signals, but transfer speeds ended up being really slow. Not sure if it was due to Qt at the time or my own implementation, but I switched to a threaded model very early on (where each download or upload is managed in its own thread) and transfer speeds improved greatly. At the same time, I've been getting crash reports related to download threads for years, and with Qt 5 almost every new upload or download emits a console warning message: "socket notifiers cannot be enabled from another thread". It's never crashed for me though so I'm not sure how big of a problem it really is.

As an experiment, I decided to try implementing file transfers as non-threaded again using signals and slots. Transfer speeds seem very good this time around. Let me know how it works for you.

Windows: SoulseekQt-2015-2-10.7z
Mac: SoulseekQt-2015-2-10.dmg

Thanks, Nir

Comments

psynaturecybine's picture

speed (limit) is not divided between slots

Thanks! I updated the links.

MELERIX's picture

not sure if a bug...

but for example if I limit upload speed to 25, and two users are downloading from me at same time, each user will download at 12.0.

but if you divide 25 / 2 the result should be 12.5 and not 12.0, so probably there is something wrong with decimals in the formula that SoulseekQt is using to divide the speed ?

psynaturecybine's picture

confirmed.
...also refresh rate seems slower, so it "feels" like the connection is laggy.
Nir, what about previous subject? is the work on taglib done yet? i really hope it's not. also, please take a look into our dropbox, there is a small request for You. Thanks!

Yeah, the new speed limiting algorithm is a tad inaccurate, and I haven't been able to pin down why yet. Are you talking about the transfer speed refresh rate?

I'm not done with TagLib, no. Not sure how I want to use it exactly, or whether I want to use it at all. What are you hoping to see?

psynaturecybine's picture

if You decide to use it, make it show bit depth\sample rate for lossless instead of bit rate.
refresh speed of "speed" and ETA is about 0,5s for download tab, and about full 5seconds at upload tab.
more about laggy connection - i can't verify it on my own due to poor internet connection i'm struggling with lately, but when i start new download, user who shares appears offline for few noticeable seconds.

Good so far.
Less memory. Seems faster?

That's really good to hear. It's possible using blocking Qt socket functions in a different thread was causing a lot of friction. It does seem smoother to me as well. Might just be wishful thinking ;)

1st: All running uploads freeze when I limit the upload speed. had to delete all the download queue after upgrading to the new version. Now it's working
2nd: clear complete/aborted uploads isn't working, the uploads still remain in queue.

beside that, speeds are as fast as with threaded version

The new build should fix the freezing issue. Clear complete/aborted uploads is working, but it takes five minutes to clear.

Thanks! Nir

Network activity is fine for me so far. I do notice the rounding error mentioned above.

On Windows, .lnk files are being indexed now, and they're being followed too, but only for indexing. This needs to be turned off so it's like it was before, completely ignoring shortcuts. Some people queue files that are "under" a folder shortcut, and the transfer always fails with a Local Error on my side, and their client just keeps requeuing it. Other people queue a .lnk file itself, and my upload to them hangs forever because my client thinks the file is ~39 MB or whatever but only the first ~1K can be transferred, since that's the actual size of the .lnk file.

I'll look into the shortcut issue over the weekend. If I can't get it to work properly I'll turn it off.

(running with no upload speed limit)
everything fine, though that strange issue with missing shares is still there.
Maybe this is folks who have not restarted in ages, and keep coming back to an item?

You mean people requesting files you're not sharing anymore?

yes.

I'd corrected some folder names weeks back and have run several debug sessions since then.

edit: just seen one I'd not seen before, where the Shared File Not Found list contains someone's share that doesn't exist on my PC!

I updated the links. New build should have a much more accurate speed limit and fix the freezing transfers problem when the speed limit is lowered.

Using the updated build for about 24 hours, and got a crash immediately after (or perhaps during, I was off in another window) a manual Rescan Shares operation. I wasn't running it through gdb. I did get a DrMingw report, but it's probably not very helpful: http://pastebin.com/C0W41hin

The reason I was doing a Rescan Shares is because I tried to do a manual upload to someone, but when browsing my files, I could only see the folders; it was as if they were all empty. However, people were downloading and queuing new things from me, so I don't know what was going on.

The empty folders problem is persisting after restarting the client and doing Rescan Shares again. No crash though. If I send the folder, the files in it all get queued. I just can't pick individual files from the browse window.

It almost sounds like a UI problem. I wonder if resetting size settings via Options->UI will fix it? This is the debug version of the newest build if you're interested:

https://www.dropbox.com/s/c0vusoawcl50r34/SoulseekQt-2015-2-14-debug.7z?...

It also disables shortcut indexing on Windows as per our previous discussion.

-shotgun-'s picture

It works fine for me:
- Limiting u/l speeds no prob
- Messing around with ppl's que (hihi) no prob
- But, 2 things: speed reading would start off (way too high, even for new uploads)
- I had a situation with 2 users sharing u/l speed between them at 70% and 30% of total. Once the 30 finished, a new user 'took over' the speed and pushed the 70 to like 10. This act of barbarism occurred both when u/l was limited and when not. Just FYI!

From the bottom line, what does all this signal-threading thingy do? better speedz?

Speed reading being a lot faster when it starts is probably due to Qt's heavy use of buffering for both sending and receiving. You get a very high speed to begin with because Qt writes a whole lot of data to the buffer before it even starts sending any of it. There's actually no way to measure exactly how much data is being sent over the socket. You just have to limit the buffer and things eventually even out. One possibility would be not to show any transfer speed until a few seconds have passed... I'm probably gonna end up doing that.

Regarding the situation you describe with users downloading at different speeds, it's hard to say exactly what happened. If a user downloads slower than is allotted for their slot, the extra speed will be added as global credit which will be consumed by faster downloaders. It's very likely that with the 70/30 split one user couldn't download at full speed so the other one downloaded faster. The change to a 10/90 split is a lot harder to explain... in my tests with a very fast downloader and a relatively slow downloader, upload speeds to both were exactly equal. If both downloaders are going faster than they should in general, no credit would be generated and both would be throttled so it should have been equal. If it wasn't, it's possible the original downloader's speed might have gone down on their end. I'll keep looking for more evidence of this problem.

Bottom line, this has the potential of resulting in a more reliable client. As I said in the original post, I still get an occasional crash report happening in a file transfer thread. As far as I can tell, Qt doesn't officially support using sockets in threads other than the ones they were created in. This design is much more in line with Qt's approach.

-shotgun-'s picture

Yup, I agree about the no-speed show at transfer start.

Thanks for the reply :)

-shotgun-'s picture

I've had a couple of users trying to download unsuccessfully. They would go from 'connecting' to 'uploading', but no speed would show and no data sent. This would repeat on several attempts. I don't recall seeing that before so it may be related. If there's anything u'd like me to look up lemme know.

-shotgun-'s picture

Just a heads-up: I've tried to revert to the previous build by both replacing the EXE and reinstalling entirely. In both cases I get a C++ runtime error on startup saying it requested to terminate in an unusual way, and shuts down.

The new client updated the config data schema. The old client can't read it. You have to keep using the new.

The 2/22 build on the download page is basically the same as the 12/19 build (no TagLib or non-threaded file transfers), except it's been patched to be able to read the newer configuration files and to fix the problem of the client data not being saved periodically.

psynaturecybine's picture

build 3.16?

You mean the download is frozen?

psynaturecybine's picture

kind of. user went offline, but soulseek "thinks" it's still downloading. numbers didn't change, there was no progress, it just stood still like on the picture ;)

Interesting. When we were using threaded file transfer with blocking socket functions, the download would automatically fail after blocking for more than 30 seconds on a socket read operation. Now that we're relying on events, if no more data comes on the socket and the socket isn't recognized as closed (which does appear to happen sometimes, especially on OSX and Linux for some reason) no read event comes in and the download is left as is. I'll look into implementing a timeout mechanism for file transfers over the weekend.

I updated the nightly builds with a probable fix. I know this isn't a very common problem, but let me know if you run into any new issues.

Thanks, Nir

Just out of interest, I tried to clear down the chat log. Soulseek froze.
This is the tail of the debug log, but I'm not sure whether this is from me trying to kill the process, or something to do with the chat lockup.
using 3.25 debug btw

warning: HEAP[soulseekqt.exe]:
warning: HEAP: Free Heap block dd23bf0 modified at dd23d78 after it was freed

Program received signal SIGTRAP, Trace/breakpoint trap.
0x779256bd in ntdll!RtlpNtMakeTemporaryKey ()
from C:\Windows\SYSTEM32\ntdll.dll

I just had another crash with:::

Program received signal SIGSEGV, Segmentation fault.
0x00431b2c in Data::removeDisconnectedItems (this=)
at ..\SoulseekQt\Data.cpp:488
488 ..\SoulseekQt\Data.cpp: No such file or directory.

on the end of the log. However, I see a new nightly is up, so will see what happens with that - debug version any time?

Hmm, new version shouldn't affect this particular crash, but I think I know what's causing it. I'll ready a new debug version soon. How often is it crashing for you?

It was running for a couple of days at least

This one should hopefully work around the crash you posted:

https://www.dropbox.com/s/mp0szkd1k88jny0/SoulseekQt-2015-4-6-debug.7z?dl=0

Thanks, Nir

a good round of benchmarking and regression testing between the old and new implementation could clear things up a bit