Proposal for an alternative to read_piece_alert

[AllSeeingEyeTolledEweSew]

Current state

The details of set_piece_deadline() + read_piece_alert cause some trouble, especially in python.

1. Memory management complexity: a caller must collect all read_piece_alerts for the entire range they want to read. Since alerts must be processed as soon as they're delivered, and read_piece_alerts may be delivered out-of-order anyway, this means callers may accumulate a lot of memory if they want to read a large range.
   1. No good choices: to mitigate 1., the caller must manage how many read_piece_alerts are "in-flight" at a given time, globally in the app. But the design of set_piece_deadline(alert_when_available) makes this difficult: if a piece is available then it posts an alert immediately; otherwise it waits. The caller won't know which, so it must be conservative to stay within desired memory limits; but if few pieces are available, then this is wasteful as the buffer will just remain empty for a long time.
   2. Tedious interface: due to i., the caller must either make few set_piece_deadline calls, or else make many set_piece_deadline(p, 0) calls which are later overridden with set_piece_deadline(p, alert_when_available) when we want more alerts. But messing with piece deadlines is onerous due to the limited interface of set_piece_deadline.
2. Unnecessary copies: the read_piece code copies data into a heap buffer. In python it must gets copied again into a bytes object, and again if we create slices with bytes[lo:hi]. If we want to send() or write() the data, that's another copy.
3. Unaligned reads: read_piece_alert only reads on piece boundaries, but most I/O will naturally occur on file boundaries or aligned with files.

A caller who wants to read data can either

• set_piece_deadline() + read_piece_alert, or
• watch piece_finished_alerts (for all torrents) + manually open and read files (working around move_storage and rename_file, etc)

Personally I think the second one is easier, more optimal and less error-prone.

Goals

Now that libtorrent 2.0 uses mmap, I claim our goal should be to make it easy for callers to work with the torrent files' page cache.

In particular, since my project is a python app to vend bittorrent data over http, I'd like to just use sendfile() to vend that data. It's guaranteed to be visible as soon as libtorrent is done writing, is always file-aligned, saves 3-4 copies, and saves the data from ever needing to be touched by python.

Proposal

I propose the following changes:

1. Add void torrent_handle::open_file(int file_index), which opens a new file descriptor to a torrent's file storage, and posts the result in an open_file_alert
   1. The intended use is to be atomic among storage changes like rename_file() and move_storage()
   2. libtorrent opens the file descriptor, but the caller is responsible for closing it
   3. This would disable partfiles if they're in use
   4. rename_file() / move_storage() should operate agnostic of any outstanding open file handles
   5. In particular, open_file() followed by various operations (especially move_storage() to a different volume) may mean the file descriptor refers to old storage that won't receive new data
   6. To reconcile this, we should guarantee that the fd in an open_file_alert will contain all the pieces referenced by piece_finished_alert up to that point. The caller understands that further storage alerts may make the fd "obsolete".
2. Add deadline_flags_t::lite_alert, such that set_piece_deadline(alert_when_available | lite_alert) will post piece_finished_alert instead of read_piece_alert.
   1. The intent is to subscribe to piece_finished_alert of particular pieces, but not all pieces for all torrents.
   2. The alert will be posted "unconditionally" (without alert_mask & piece_progress).
   3. The alert will not cause any disk reads.

[arvidn]

I agree that there should be a more direct way to access the file storage, cutting through most of the layers right now.

I would still hope that there could be some interface implemented by the underlying disk_interface that would give it some control over how files are stored and accessed, but still be efficient in the case of the data just being in the page cache.

[AllSeeingEyeTolledEweSew]

I saw you wrote that you're in favor of buffer-oriented APIs as being future-proof.

In Python, there's the buffer protocol to wrap underlying buffers, which would be a nice way to present arbitrary data to python (memory management questions aside). Unfortunately I find a lot of places where the buffer protocol isn't treated as first-class (namely: there's no first-party type in the type system for the buffer protocol; and asgi doesn't specify support for it; many APIs only accept python-owned bytes rather than buffers). I generally find better support for sendfile() than the buffer protocol, as the current state-of-the-art for moving data.

open_file() could just fail with EOPNOTSUPP if the disk_interface doesn't map torrent files to disk files.

open_file() is abstraction-breaking and my case is motivated, but I expect there will always be APIs that work better with file descriptors than memory buffers.

To be clear, I already find read_piece_alert too burdensome for my use case, so my plan is to try to open a torrent's file by path, and retry when I see file_renamed_alert or storage_moved_alert, and hope to eventually succeed. open_file() would just make this less brittle.

[elgatito]

It would be great to have a global read virtual method, called from torrent_handle that would come directly to disk_interface implementation.
That would, for example, make you able to store data somewhere in the database or cloud, or whatever, and read that without knowing the details.

[arvidn]

Maybe a blocking read() call where you pass in the buffer to be filled. That way you can at least reduce it down to a single copy of the data. Not quite as efficient as returning a pointer to memory mapped files, but it's not obvious how to do that either.

[elgatito]

@arvidn That is also fine. Something like:

std::int64_t read(char* buf, std::int64_t size, std::int64_t file_offset);

[AllSeeingEyeTolledEweSew]

@elgatito for input