Transmission

This patch is against official 1.75 source code. It might work with 1.76 (haven't tested). Its quick and dirty and may not help you at all, but my downloads used to peak at 200 kb/sec (on a 20mbit line), now I regularly get 1000kb/sec+. Give it a shot and post your results here.

http://pastie.org/670537

This patch is much better-written than most of the submissions we get, and it dives much deeper into the guts of the code. Thanks very much for reading through the code and improving it!

The sort-by-reverse-liveliness patch when pruning peers is a good idea.

The block request count might be a good idea, but I'd like to get more information on what behavior you were seeing that required this. The block iterator's list of pieces is sorted by compareRequestPieces, which already prioritizes the pieces with fewer pending requests... so does the block request count actually wind up filtering many requests?

Regardless of how the details of this particular patch work out, I hope you write more patches for Transmission.

The block request limit idea came after I tried changing the max outstanding requests per peer floor from 8->64 (in ratePulse in peer-msgs.c). It helped a little bit with regards to speed until I noticed that the refillPulse() in peer-mgr.c function was simply requesting the same 64 blocks from a number of peers instead of 64 different blocks. I decided to keep a block request count so block requests would be spread evenly amongst available peers. Then I found this strategy doesn't work very well with a large number of peers and few blocks left to request hence the switch back to original behavior after 90% completion. 90% is a bit arbitrary as I mentioned in my comment but it seems like a good point to switch off the block request limit.

I appreciate the compliment on my work and here's hoping you decide to include a version of this in the official code.

Looking at the code again, I'm not 100% sure why that behavior occurs. All I know is it was happening by using some debugging output. I think what happens is getPreferredPieces() returns the same pieces on each invocation (possibly in a different order, but they are still there until they are completed) and so tr_peerMsgsAddRequest() will assign the same block to another peer.

http://trac.transmissionbt.com/changeset/9416

Thanks! And also, I recommend opening tickets on trac for stuff like this so we don't miss it.

Ok will do next time sorry.

livings124 wrote:http://trac.transmissionbt.com/changeset/9416

Thanks! And also, I recommend opening tickets on trac for stuff like this so we don't miss it.

hexum wrote:Ok will do next time sorry.

That was gratitude and a suggestion so patches can be seen faster and taken care of easier. You weren't being scolded :)

I guess one more concern I have about the remaining part of this patch, is that an array of tor->info.blockCount integers will be *huge* for some torrents. A 4.3 GB iso is going to use over a meg of memory on that array, assuming 32-bit ints and 16K blocks. Alternate approaches to tabulating that request count would use less memory, but more CPU. :/

Two other things about the patch:

1. I'm not sure why you cut the lines that remove the peer from the peers array when it says its request queue is full. If its request queue is full, it's not going to accept any further requests this time around... it's a waste of time asking them.

2. The reason we remove unhandled pieces is that, if no peer accepted the block request, it's because all the peers have full request queues and/or don't have that piece... so asking for other blocks in the same piece is just going to waste CPU cycles.

Yes the block count array will be huge. It could probably be an array of shorts or maybe even chars. I doubt the outstanding block request count would ever overflow a char. It's capped at 2 by my code until the 90% completion percent, at which point it becomes irrelevant anyway. And at the risk of repeating everything you said yes there could be a space/time tradeoff.

1. Yeah I've re-added those lines in my local code. I was trying a different approach to selecting a peer to request a block from and removed those lines. The way I've got it now they should go back in.

2. Ok, makes sense, I hadn't thought about it that much.

How would one go about installing this patch?

The majority is already implemented in the nightly builds.

Maybe this is a very unlikely situation but wouldn't it be possible for all uploading peers to not supply a certain block in a piece while being able to supply other blocks in that piece, therefore incorrectly triggering blockIteratorSkipCurrentPiece() when continuing to the next block in that piece may succeed? Or is there something in the code/bittorrent protocol I don't understand? Or does it even really matter with regards to throughput? Honestly I'm new to the inner workings of transmission and bittorrent.