Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Design for the refactor of the Libtorrent wrapping code #3418

Closed
3 tasks
synctext opened this issue Jan 30, 2018 · 16 comments
Closed
3 tasks

Design for the refactor of the Libtorrent wrapping code #3418

synctext opened this issue Jan 30, 2018 · 16 comments

Comments

@synctext
Copy link
Member

synctext commented Jan 30, 2018

Design by @vandenheuvel after the work they did with @MaxVanDeursen on #2620.

This moves far towards a lock-free architecture, lock isolation (merely Libtorrent facing modules) and step towards async code architecture. Now code has more dependency (hell). We need discussion and consensus on what we would like as a result of a solid re-factor. on-demand streaming could also use a cleanup, not yet taken into account. Not yet the new idea/interface with Libtorrent in recent Arvid discussion regarding the tunnel matters
img_20180130_174137

And if all this is not yet sufficient, it should be more or less Python3 compatible and perhaps allow us to one glorious year far far into the future to replace twisted with a leaner standard system lib

Goals:

@arvidn
Copy link

arvidn commented Jan 30, 2018

If I may make some comments (this may be what you're thinking, I saw the word "async" in there).

I would highly recommend that if your GUI thread needs to render a list of torrents (or peers or file progress), it should not ask libtorrent for it on-demand, with blocking calls.

A better design would be to have your own list of torrents, holding all the current states, owned by your GUI thread. Periodically (with whatever update frequency you like) you call post_torrent_updates(). This will make libtorrent compile a list of all torrents that have had any metrics/attributes changed and post it back (asynchronously) in a state_update_alert. When you handle this alert, you can update your list of torrents and possibly re-render any that may be visible to the user.

The main benefit this buys you is to minimise the GUI thread stalls while waiting for the libtorrent network thread (which under heavy load quickly can become quite costly). But perhaps more importantly, you can have hundreds of thousands of torrents loaded, and as long as they're idle, they won't affect the performance of your updating and rendering of the active ones.

(there's a similar mechanism for polling session stats counters, post_session_stats).

There's an old blog post on this topic.

@devos50
Copy link
Contributor

devos50 commented Feb 3, 2018

@arvidn thank you for your comment. This sounds like a good improvement.

The GUI and Tribler core (which makes the calls to libtorrent), run in different processes since Tribler 7. Since we're planning a redesign of the whole download engine, we will take your suggestion into consideration when designing it! 👍

@ichorid
Copy link
Contributor

ichorid commented Mar 28, 2018

Related #3402

@vandenheuvel
Copy link

diagram

@ichorid
Copy link
Contributor

ichorid commented Aug 17, 2018

I guess I could start by thoroughly reviewing the effects of #1410.
If removing all this locks introduces no new problems, we can move to refactor the structure of the wrapper.

@ichorid
Copy link
Contributor

ichorid commented Aug 17, 2018

A note on general rules for API design.

@synctext
Copy link
Member Author

synctext commented Jan 30, 2019

RESTmanager is probably the first component to start and has overall control. Not session. Removal of LaunchManyCore.
(Idea from Developers meeting discussion)

@synctext
Copy link
Member Author

@ichorid Maybe higher priority then Libpath work ? #4563

@ichorid
Copy link
Contributor

ichorid commented Jun 14, 2019

@synctext , moving to Pathlib will make solving Libtorrent refactor considerably easier.

@ichorid
Copy link
Contributor

ichorid commented Jan 4, 2020

Recent refactorings by @egbertbouman have put Libtorrent wrapper in a much better shape. So, moving this to backlog.

@ichorid ichorid modified the milestones: V7.5: core refactoring, Backlog Jan 4, 2020
@arvidn
Copy link

arvidn commented Jan 4, 2020

also, feel free to reach out if there are any usability issues with the libtorrent API.

@ichorid
Copy link
Contributor

ichorid commented Jan 4, 2020

@arvidn , thanks! We'll definitely will if it be of use for Libtorrent.

@synctext
Copy link
Member Author

As a wise man said:

if your GUI thread needs to render a list of torrents (or peers or file progress), it should not ask libtorrent for it on-demand, with blocking calls.

What is the progress with lots of stuff mentioned above, like?

@egbertbouman
Copy link
Member

@ichorid As far as I'm concerned, the refactoring of the download core has completed.

@synctext
Copy link
Member Author

Seems most work has been done. Document changes here please and then close this ticket?

@ichorid
Copy link
Contributor

ichorid commented Jul 14, 2020

Well, I don't think there are any changes to document, as nothing was documented initially... We cleaned up the stuff, removing duplicate functions, etc.
I don't think, we need any dedicated ticket for improving our current libtorrent wrapper. Instead, we can go with gradual changes.

@ichorid ichorid closed this as completed Jul 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

7 participants