Revert fastresume functionality

[sledgehammer999]

Now that the problem behind PR #11104 is correctly addressed via arvidn/libtorrent#4112, let's go back to our original way of storing/loading fastresumes.

[Chocobo1]

let's go back to our original way of storing/loading fastresumes.

May I ask why? I'm under the impression that the future is saving all the required info in .fastresume file and ditch the .torrent for resuming torrents.
arvidn/libtorrent#3946 (comment)
arvidn/libtorrent#3946 (comment)

ps. I haven't tried the latest patch in libtorrent yet.
Update: tried the patch. Now it works as intended why do you need the revert?

[sledgehammer999]

I'm under the impression that the future is saving all the required info in .fastresume file and ditch the .torrent for resuming torrents.

That future isn't certain yet.

The benefit of going back to the original way: 1) Reducing fastresume size. This hopefully helps generate and write the fastresume faster (probably marginally). 2) Why bother writing the same data corresponding to the info-dict again and again to disk when we only need the torrent state in the fastresume?

From a quick lookup in my fastresumes: Old ones rarely go above 5KB in size. New ones usually are bigger than 50KB.

[Chocobo1]

That future isn't certain yet.

How come? IMO it is more like not fully implemented yet, because the original idea is to preserve the downgrade path in case qbt v4.2.0 or libtorrent RC_1_2 isn't stable.
At least from my viewpoint, the links in my previous post and libtorrent tutorial/examples seems to mildly suggest that future. Or we could ask the library author for clarification?

Why bother writing the same data corresponding to the info-dict again and again to disk when we only need the torrent state in the fastresume?

This is for the downgrade path as mentioned above, otherwise we would have ditched all unnecessary .torrent from qbt.

Old ones rarely go above 5KB in size. New ones usually are bigger than 50KB.

This might be a slowdown problem if there are many torrents, but even so the more important question is still the aforementioned "future".

[sledgehammer999]

How come? IMO it is more like not fully implemented yet, because the original idea is to preserve the downgrade path in case qbt v4.2.0 or libtorrent RC_1_2 isn't stable.

It was just a thought of the libtorrent author. It isn't implemented yet.

This is for the downgrade path as mentioned above, otherwise we would have ditched all unnecessary .torrent from qbt.

Let me rephrase. Part of the fastresume is now the info-dict. Which is static data. Fastresumes get rewritten multiple times. Why should we keep re-writing those static data? The old way avoids that.

And to me, there wasn't a real reason to switch apart from fixing the bug in PR #11104, which is now properly fixed in libtorrent.

And if we want to revamp the saving system it is better to go eg the sqlite route than keep messing with individual files. (IIRC I have an old and forgotten PR pending for that).

[Chocobo1]

@arvidn
May we have your opinion on the following question? The question is whether you intend to simplify the API so that (ideally) read_resume_data() would only need .fastresume file and doesn't rely on .torrent file providing the torrent_info when resuming torrents?
Because to realize that vision we need to call save_resume_data(torrent_handle::save_info_dict); for every torrents which writes the same and big info dict field over and over again.

@sledgehammer999
I don't really insist on either way, but I do feel we need a common vision for both the library and client implementation. Also if you really intend to revert it before v4.2.0, I would hope to see another RC/beta for testers to be sure nothing breaks.

[xavier2k6]

And if we want to revamp the saving system it is better to go eg the sqlite route than keep messing with individual files. (IIRC I have an old and forgotten PR pending for that).

@sledgehammer999 #10099

[arvidn]

I'm envisioning a future where torrent_info is immutable, and only contains the info-dict part of the torrent. It's kind-of used as if it was immutable now, as the mutable fields are copied into the torrent object and not changed in the torrent_info object once added. This is why it's safe to share the same instance between the libtorrent thread and client threads for instance.

In such a future, instead of loading a .torrent file into a torrent_info object, it would probably be loaded into an add_torrent_params object. That way, all the non-info-dict fields are naturally mutable, and naturally are saved and restored via resume data.

I don't have any measurements on the cost of saving the info dict to the resume file regularly, but last time I measured starting a session and loading back all torrents from the previous session, loading both .torrent and .resume files dominated the cost. i.e. just opening the files. So, putting them in a single file saved a lot of time for me. The next step, of putting them all in a sqlite database sped it up further. I imagine because it could optimize the order of loading them better.

I would think that the cost of saving resume data is less important too, since you can stagger saving the torrents (i.e. you don't need to save them all at once), whereas when you load them, you do have to load them all at once.

[arvidn]

just to be clear, this is a vision that's pretty far out and I'm not actively working towards it (yet). I'm open to suggestions, criticism or better alternatives.

[glassez]

I'm envisioning a future where torrent_info is immutable, and only contains the info-dict part of the torrent. It's kind-of used as if it was immutable now, as the mutable fields are copied into the torrent object and not changed in the torrent_info object once added. This is why it's safe to share the same instance between the libtorrent thread and client threads for instance.

Splitting mutable and immutable part of torrent metadata is really good idea 👍
It should fix some currently existing inconvenience.