Some very bad decisions in transmission

Discussion of Transmission that doesn't fit in the other categories
blacke4dawn
Posts: 552
Joined: Sun Dec 13, 2009 10:44 pm

Re: Some very bad decisions in transmission

Post by blacke4dawn »

Wait Jordan, are you telling us that when one hits "verify local data" Transmission loads the entire torrent into memory as one big chunk of data? Instead of, say, initially loading the data for 10-20 pieces and then loading in new one as existing ones are verified?

If the former, then I would see that as a design flaw and the root of the "problem" in this thread.
If the latter then I don't really see any problem since (to my understanding) the lesser used pieces that have been verified will be dropped in favor of keeping the more used pieces of other torrents when loading the new "to be verified" pieces.
porter
Posts: 20
Joined: Fri Sep 12, 2008 10:59 pm

Re: Some very bad decisions in transmission

Post by porter »

Jordan wrote:I hope this response helps you to understand the question I'm asking.
It does, and thank you for taking the time to express your concerns clearly. And my apologies if I was a little bit harsh at times, I didn't understand your point completely until now.
Jordan wrote:When I talk about Transmission prefetching blocks, I'm referring to the use of POSIX_FADV_WILLNEED and F_RDADVISE to tell the OS about upcoming reads. We know in advance that we'll need these blocks because we can peek the request lists that the peers have sent us:

Code: Select all

int
tr_prefetch (int fd UNUSED, off_t offset UNUSED, size_t count UNUSED)
{
#ifdef HAVE_POSIX_FADVISE
  return posix_fadvise (fd, offset, count, POSIX_FADV_WILLNEED);
#elif defined (SYS_DARWIN)
  struct radvisory radv;
  radv.ra_offset = offset;
  radv.ra_count = count;
  return fcntl (fd, F_RDADVISE, &radv);
#else
  return 0;
#endif
}
I agree that this is a perfectly good usage of fadvise(). You know you'll need those blocks soon, and you advise kernel to get them for you, ASAP.
Jordan wrote:
Porter wrote:And it has the best view overall, because transmission is not the only app running on the OS, typically, right?
This is true to a point, but the app has a role. If it has specific knowledge about upcoming IO, it can give hints to the OS to optimize for it. This is why things like posix_fadvise() exist.

This paragraph isn't relevant to the topic of prefetching, but as an aside, there's also a place for an app-level write cache in BitTorrent clients. The client has unique knowledge about what disk writes are upcoming because it knows (1) which blocks it's requested (2) from which peers and (3) how fast each peer is. A smart torrent client can lower the total number of disk writes with even a small in-memory write cache.
This I could almost buy, but I'm still sceptic that this improves anything. Possibly it just adds overhead, and in the worst case, it might also be killing performance. The reason being that write I/O path is also heavily optimized and controlled by the kernel. When you say write() in the app, it doesn't go to the disk immediately, but instead lingers in the memory for a quite long time. If ${OS} eq Linux, that time is up to 30 seconds, by default. It could be shorter if there's a severe memory shortage, but in those cases it's pointless to discuss any type of caching anyway. When kernel detects that it has dirty memory pages that are 30 seconds old, it knows it needs to write them down. What happens next are complex elevator and scatter/gather algorithms that take into account disk geometry (the best they can), interface limits, coalesce buddy pages and all sorts of stuff to make the writing process as efficient as it can be. Of course, once again, the OS is doing it on behalf of all the running processes, and can optimize better than one single app could ever do. In the best case, the mechanical disk arm will do one sweep from the start to the end of the disk visiting and writing to all disk blocks that were written to, in large contiguous chunks.

I've checked that disk cache setting in the app since your previous reply, and found that I have it at 0. I guess that disables it alltogether? I don't remember when I put it at 0, it could've been a year or two ago. And I don't remember why I put it to 0, because of CPU or memory usage? But believe me when I say that concurrent fast speed torrent download (say, 4 or 5 really active torrents on a fast network) is working perfectly fine here. I see about 25 seconds of almost completely idle disk, then I get a storm of writeouts running for a few seconds close to the theoretical disk speed limit (60-80MB/s).

Please also consider that OS has a really big chunk of available memory to cache pages/blocks that are to be written (it can be gigabytes of memory, and on any decent hw today it actually is!), and then also disks have internal RAM caches that cache and optimize writes, which are also 16-64MB on nowadays hw. I just don't see what kind of write cache you can add to the mix, and of what size, to further improve performance?
Jordan wrote:This finally gets to the question I asked. Let's say ${OS} is using LRU. Transmission prefetches the blocks it knows it's going to need soon. Then "verify local data" loads a torrent larger than the cache, causing the prefetched blocks to fall off the end of the LRU list. The end result is we lose blocks we know we're going to need, to make room for a torrent that may or may not have any peers.

That's the question I'm asking, anyway. Ideally I'd like to see some testing about how this plays out in practice on different OSes and different cache sizes.
OK, I finally understand what your concern is. You're worried that verifying a torrent, due to heavy I/O, will discard blocks that you know you will need soon. That indeed can happen. But I can only repeat what my POV on the issue is:

- it can, but it's not catastrophic!

- once again, the kernel knows the best! If those prefetched blocks are really used, they (those memory pages!) will have PG_referenced bit set (at least if ${OS} == linux) by the time the page comes into consideration for freeing. Then the kernel will clear that bit, rotate that page to the furthest end of the LRU, and let it live. That process will repeat over and over, and if the page is really constantly accessed then it could live forever in memory. Linux is even more clever than that simplified explanation, because it has two LRU's, inactive and active, where if pages are really accessed often, they're are moved from the inactive LRU (where they start to exist) to the active one, where they can survive much longer. But we're getting out of scope. The important thing here is that if the prefetched blocks travel the whole LRU, and don't get referenced in that time, then they certainly deserve to be freed, because readahead mechanism was too optimistic. And as I said in bullet one, it's not catastrophic, the block will be read in synchronously and everything works.

- as things stand now, you've built into the application the presumption that the torrent that has been verified won't be seeded again, so in your book it's perfectly fine to discard the cached content completely. In my book, that's plain wrong. I'm practically always putting the torrent back to seed after I've verified it. I had various reasons for verification in the past, and I almost grew a habit to make a quick verification of important stuff, soon after the torrent is completely downloaded, and then return it to seeding shortly after. I do verification soon because the torrent is perfectly cached at that time, and I do also return it to seeding fast, because I still expect it to be perfectly cached. By perfectly I mean 100% or close (of the torrent content is in memory). Unfortunately, by the time I return it to seed, it is perfectly 0% cached, and all I see is gigabytes of memory FREE, exact gigabytes that had all the needed content cached a millisecond before? And in unixland, there's a proverb "free memory is wasted memory". Can you agree that it's a bad user experience from my POV.

So, I understand your concern, but let's try to quantify and compare it. Say 20 torrents are active, say their pieces are large, 2MB each. So you're trying to protect 2 * 20 = 40MB worth of memory, you're actually just trying to make sure it's intact, and to accomplish that you're bravely killing, freeing, call-it-what-you want say 4GB of memory just because the user dared to click verify? And by doing that you're asserting that the verified torrent won't be needed soon, although you really don't know that for sure.

Let me finish this reply with what I started this thread with, transmission is an absolutly excellent application! Thank you very much for the countless hours you put in it to make it such great. I would never go in such lengths trying to explain deficiencies that I see as an user, unless it was a really great software, software that I've been happily using for many years now. I see bad behaviour in this one aspect and I'd like it fixed. You can decline my plea to fix it (i have a workaround), you can introduce tunable to make everyone happy, you can fix it however you see fit, it's all good. My only motivation is to make such a great product a bit better, if it's at all possible. Apologies if I come harsh at a time, or speak rubbish (english is not my primary language).
Jordan
Transmission Developer
Posts: 2312
Joined: Sat May 26, 2007 3:39 pm
Location: Titania's Room

Re: Some very bad decisions in transmission

Post by Jordan »

blacke4dawn wrote:Wait Jordan, are you telling us that when one hits "verify local data" Transmission loads the entire torrent into memory as one big chunk of data? Instead of, say, initially loading the data for 10-20 pieces and then loading in new one as existing ones are verified?
No, of course not. libtransmission/verify.c's verifyTorrent() is where this action takes place. It uses a 128 KiB buffer which in testing seemed to be a "sweet spot" in the sense that smaller buffer sizes were slower but larger buffer sizes were not meaningfully faster. However this testing was three years ago so it might be worth revisiting that choice.
If the latter then I don't really see any problem since (to my understanding) the lesser used pieces that have been verified will be dropped in favor of keeping the more used pieces of other torrents when loading the new "to be verified" pieces.
I'm not following this line of reasoning and think I may be misunderstanding you. Could you rephrase this?
blacke4dawn
Posts: 552
Joined: Sun Dec 13, 2009 10:44 pm

Re: Some very bad decisions in transmission

Post by blacke4dawn »

Jordan wrote:
If the latter then I don't really see any problem since (to my understanding) the lesser used pieces that have been verified will be dropped in favor of keeping the more used pieces of other torrents when loading the new "to be verified" pieces.
I'm not following this line of reasoning and think I may be misunderstanding you. Could you rephrase this?
Lets see if this makes it more clear.

When loading new data that is to be verified then the kernel will look at all the pages in memory that belongs to cached data and throw out the one that is least used, which could be data that was an earlier part of the torrent is being verified instead of the cached data from another torrent that has been sent to several other peers. If my understanding is correct then the data from the other torrent has a higher "prio" of being kept in memory due to having been accessed more (maybe just requesting it is enough). Heck, it may not even be data from a torrent that is kicked out but rather from another program.

This means that "dumping" all the cached data that was loaded during the verification process won't protect any cached data from the other torrents since at that point it's too late due to the "fact" that if it's going to kick out cached data from other torrents it's going to do so during the loading of new data which is done before the verification itself is done.


I'm not that well versed in the general memory handling but this is my understanding of it. My personal experience is also that if one verifies a torrent then the likelihood of them activating it afterwards is higher than not doing so, so not "dumping" that data would in most cases be beneficial.
porter
Posts: 20
Joined: Fri Sep 12, 2008 10:59 pm

Re: Some very bad decisions in transmission

Post by porter »

blacke4dawn wrote:When loading new data that is to be verified then the kernel will look at all the pages in memory that belongs to cached data and throw out the one that is least used, which could be data that was an earlier part of the torrent is being verified instead of the cached data from another torrent that has been sent to several other peers. If my understanding is correct then the data from the other torrent has a higher "prio" of being kept in memory due to having been accessed more (maybe just requesting it is enough). Heck, it may not even be data from a torrent that is kicked out but rather from another program.

This means that "dumping" all the cached data that was loaded during the verification process won't protect any cached data from the other torrents since at that point it's too late due to the "fact" that if it's going to kick out cached data from other torrents it's going to do so during the loading of new data which is done before the verification itself is done.


I'm not that well versed in the general memory handling but this is my understanding of it. My personal experience is also that if one verifies a torrent then the likelihood of them activating it afterwards is higher than not doing so, so not "dumping" that data would in most cases be beneficial.
Your understanding is completely correct. On all accounts.
porter
Posts: 20
Joined: Fri Sep 12, 2008 10:59 pm

Re: Some very bad decisions in transmission

Post by porter »

Here's a bit more clever preload library that disables proven defective POSIX_FADV_DONTNEED, but passes POSIX_FADV_SEQUENTIAL and POSIX_FADV_WILLNEED. Just for fun, to see if those other two have any positive impact in the whole picture. I suspect not, but lets see...

Code: Select all

#define _GNU_SOURCE
#include <dlfcn.h>
#include <fcntl.h>

static int (*real_fadvise)(int, off_t, off_t, int) = 0;

int posix_fadvise(int fd, off_t offset, off_t len, int advice) {
    if (advice != POSIX_FADV_DONTNEED) {
        if (!real_fadvise)
            real_fadvise = dlsym(RTLD_NEXT, "posix_fadvise");
        if (real_fadvise)
            return (*real_fadvise)(fd, offset, len, advice);
    }

    return 0;
}

/*
 * gcc -fPIC -rdynamic -c libdisable_posix_fadvise.c
 * gcc -shared -o libdisable_posix_fadvise.so libdisable_posix_fadvise.o -lc -ldl
 */
Post Reply