search

steeve / libtorrent

Mirror of libtorrent-rasterbar's SVN https://svn.code.sf.net/p/libtorrent/code/
Other
13 stars 2 forks source link

Sequential mode now handles priority 7 pieces #4

Closed sharkone closed 10 years ago

sharkone commented 10 years ago

Pieces with priority 7 are now added in order in the queue before the rest of the pieces which are still added sequentially. This is useful to download the first and last piece of a file before downloading the remaining pieces sequentially.

steeve commented 10 years ago

cc @arvidn

steeve commented 10 years ago

Patch version: https://github.com/steeve/libtorrent/pull/4.patch

arvidn commented 10 years ago

one concern I have with this patch is that it adds an unconditional pass over all pieces, whether or not you have any prio-7 pieces. this could be quite expensive. is there a way to only loop over the prio 7 pieces? for instance, maybe the regular piece-picker data structure (that's ordered by priority) could be used for this, and terminate as soon as it runs out of prio-7 pieces. that way it would essentially not add any overhead when not used, and be cheap when used.

sharkone commented 10 years ago

Do you mean looping on the piece indices starting at m_priority_boundries[6] if I understand correctly?

arvidn commented 10 years ago

yes, except the m_priority_boundaries is inversed.. it starts with high priority pieces, and it's not a 1 to 1 mapping of priority. but looping over m_pieces until you hit a piece whose priority < 7. An overview of the data structure can be found in this blog post: http://blog.libtorrent.org/2011/11/writing-a-fast-piece-picker/

arvidn commented 10 years ago

also, it would be great if you wanted to add a test case in the piece_picker unit test to cover this new feature. the piece picker is one of the components that has the highest test coverage (because it's one of the most complex data structures), and it would be nice to keep it that way. mentioning it in the documentation would also be nice. The documentation is generated from the comments in the header files.

sharkone commented 10 years ago

Hi @arvidn and thanks for your comments. I just added a new commit that should address your concern. If that's ok with you, I'll then work on the tests and the documentation.

arvidn commented 10 years ago

this looks reasonable. it doesn't break any of the existing unit tests as far as I can tell.

sharkone commented 10 years ago

I'm really not sure how to change the docs or unit tests. Could we merge this branch? Also I can't manage to build a windows version, I'd really need a build, or rather I really need a build of the python module. Haven't tried Linux yet. Works fine on Mac using Homebrew.

arvidn commented 10 years ago

how about this patch? http://dpaste.com/hold/1768227/

sharkone commented 10 years ago

Looks good to me! Can I expect a new release with those changes anytime soon?

arvidn commented 10 years ago

I've merged this into trunk, which I'm hoping to release as 1.0 soon

sharkone commented 10 years ago

Great news! Do you have an ETA?