Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#3959 closed Enhancement (fixed)

by default, disable prefetch for lightweight builds

Reported by: jusid Owned by: jordan
Priority: Normal Milestone: 2.20
Component: Transmission Version: 2.13
Severity: Major Keywords: patch
Cc:

Description

Since Transmission 2.0, I noticed that it produces heavy load for my NMT device when seeding. Even several connected peers cause Load average to be 3-4. On the same time, Transmission 1.75 does not have such problem and the load average in the same scenario is about 1.

I started digging and found that transmission 2.x produces excessive disk activity, compared to pre 2.0 versions. On further investigation, I found that the prefetching, introduced in v2.0 is guilty for this. I disabled the prefetching and built the transmission for my NMT. Voila - system load dropped to <1 instead of 3-4. The explanation is simple, my NMT have small amount of RAM and system have very small disk cache for that reason. Prefetched data do not remain in disk cache and disk reads just wasted.

Attached patch fixes this problem by disabling prefetch if TR_LIGHTWEIGHT is defined. Transmission builds with --enable-lightweight will have prefetching disabled. Please apply this patch before final 2.20 release.

Attachments (2)

prefetch.patch (600 bytes) - added by jusid 11 years ago.
Disable prefetch for lightweight builds
prefetch-pref.diff (3.9 KB) - added by jordan 11 years ago.

Download all attachments as: .zip

Change History (18)

Changed 11 years ago by jusid

Disable prefetch for lightweight builds

comment:1 Changed 11 years ago by jusid

  • Keywords patch added

comment:2 Changed 11 years ago by jordan

jusid, it might be better for prefetch to be configurable in settings.json, and for the default value to be true or false depending on TR_EMBEDDED.

Could you give this patch a try?

Changed 11 years ago by jordan

comment:3 Changed 11 years ago by jordan

  • Version changed from 2.13+ to 2.13

comment:4 Changed 11 years ago by jusid

I checked your patch. It works well. Thanks. Please apply it.

One note. In r11784 you used powl() function, which is not available for uclibc (embedded) targets. Also using powl() for simple power of 2 is not a good idea. Please replace powl() by simple multiplication: const int minutes = t->consecutiveAnnounceFailures * t->consecutiveAnnounceFailures;

comment:5 Changed 11 years ago by jordan

jordan * r11790 libtransmission/announcer.c:

#3931 "announce is queued" -- minor revision for uClibc compatibility

jusid reports that powl() doesn't exist on uClibc, so getRetryInterval() needs to work without it. A simple left-bit shifting would be fine, but since we max out after a handful of cases, a switch statement seems slightly more readable than shifting or powl().

comment:6 Changed 11 years ago by jordan

  • Type changed from Bug to Enhancement

comment:7 Changed 11 years ago by jusid

Jordan, Any chances, that the prefetch-pref.diff patch will be applied before final 2.20 release? As I said before it works perfectly. All users of slow devices suffer from prefetching issue since 2.0 release...

comment:8 Changed 11 years ago by jordan

jusid, yes I'd like to make some form of this available for 2.20. I've left it open through 2.20 beta 3 in order to give other developers a chance to weigh in on this ticket.

comment:9 Changed 11 years ago by jusid

Jordan, thanks.

comment:10 Changed 11 years ago by jordan

  • Component changed from libtransmission to Transmission
  • Milestone changed from None Set to 2.20
  • Status changed from new to assigned
  • Summary changed from [PATCH] Disable prefetch for lightweight builds to by default, disable prefetch for lightweight builds

jordan * r11800 libtransmission/ (peer-msgs.c session.c session.h transmission.h):

(trunk libT) #3959 "by default, disable prefetch for lightweight builds" -- fixed.

User jusid reports prefetch causes load on his NMT to jump from <1 to 3-4. He requests a way to disable prefetch, and suggests that prefetch be disabled by default on lightweight builds. This commit adds a new settings.json key, "prefetch-enabled", which defaults to "true" on standard builds and "false" when compiled with --enable-lightweight.

comment:11 Changed 11 years ago by jordan

  • Resolution set to fixed
  • Status changed from assigned to closed

comment:12 Changed 11 years ago by jch

Jordan, could you please explain your reasoning here? Do you understand what's going on?

I'm a little surprised by the way you disable important functionality without even a convincing profiling run.

--jch

comment:13 Changed 11 years ago by jusid

jch, prefetch is disabled by default only for custom builds with --enable-lightweight option. Read the problem description and explanation in the first post.

comment:14 Changed 11 years ago by jch

I understand that.

What I don't understand is why prefetching should be disabled in the lightweight build. I'd like your explanation for that, as well as the evidence you base it on.

--jch

comment:15 Changed 11 years ago by jusid

Because lightweight build is specially developed for low-memory low-speed devices. Prefetch on such devices kills performance. Other transmission parameters also altered for lightweight builds (encryption, disk cache, etc)...

comment:16 Changed 11 years ago by jch

Jusid, I'm not doubting you know what you're doing. So please give me the data that you have and that allowed you to make this decision.

In particular, I want a copy of your profiling dumps, as well as a comparison between running with prefetching disabled (which is what you recommend) as compared with running with a very small prefetch queue (say, limited to just one or two chunks).

--jch

Note: See TracTickets for help on using tickets.