Opened 12 years ago
Closed 12 years ago
#4209 closed Enhancement (fixed)
Shortcut UDP tracker test
Reported by: | jch | Owned by: | |
---|---|---|---|
Priority: | High | Milestone: | 2.30 |
Component: | Transmission | Version: | 2.22+ |
Severity: | Minor | Keywords: | udp |
Cc: |
Description
In tr-udp.c line 209, we call tau_handle_message for every µTP packet, which is a waste. We should either put the test for the UDP tracker protocol last (that's assuming no UDP tracker protocol packet can start with a 'd'), or implement a shortcut test as we do for the DHT. (If that's impossible, we should consider having a dedicated UDP socket and port for the announces -- which is undesirable since it will complicate folks' firewall setups.)
Scheduling for 2.30, since it's a stupid performance regression, and should be easy to fix by someone who is intimately familiar with the packet structure of all three UDP-based sub-protocols.
-- jch
Change History (8)
comment:1 Changed 12 years ago by jch
- Resolution set to fixed
- Status changed from new to closed
comment:2 Changed 12 years ago by jordan
Other than the call to evbuffer_new() and evbuffer_free(), I don't see anything in tau_handle_message() that would generate load when testing to see whether or not a packet is from a UDP tracker.
jch, If I were to change tau_handle_message() s.t. the evbuffer creation was deferred until after the action type were tested, would that be sufficient?
comment:3 Changed 12 years ago by charles
jch, ping
comment:4 Changed 12 years ago by jordan
- Resolution fixed deleted
- Status changed from closed to reopened
comment:5 Changed 12 years ago by jch
What's wrong with r12378?
If I were to change tau_handle_message() s.t. the evbuffer creation was deferred until after the action type were tested, would that be sufficient?
The version in r12378 puts all the tests inline, and hence avoids making any function calls when the tests don't trigger. A non-local function call costs around 10ns (assuming the target is already in cache). At 1MB/s, you should expect to be receiving around 5000 µTP packets per second (including acks). (Recall that µTorrent sends small-sized µTP packets, which Transmission does not, by design.) So that's some 50ms/s, or 0.5% of CPU time.
Another, possibly better, solution would be to reorder the tests in order of decreasing likelihood, i.e. µTP first, DHT second, and UDP tracker last.
--jch
comment:6 Changed 12 years ago by charles
(trunk libT) #4209 "Shortcut UDP tracker test" -- prioritize the UDP handler functions based on frequency (uTP, DHT, UTP tracker) as outlined by jch in comment:5. Also, don't call the uTP or DHT handlers when uTP or DHT is disabled in the system preferences.
To avoid the function call overhead described by jch, instead of calling tr_sessionIsUTPEnabled() and tr_sessionIsDHTEnabled(), we test for WITH_UTP to be defined and test the tr_session.isUTPEnabled and tr_session.isDHTEnabled flags directly.
comment:7 Changed 12 years ago by charles
(trunk libT) #4209 "Shortcut UDP tracker test" -- the goal of #4209 is to minimize the cost of the UDP event callback function, so apply the heap pruning eye to that by removing an unnecessary malloc/free call there.
Previously we allocated a 4096 character buffer each time; now we allocate it on the stack. It seems all the distros and OS flavors that Transmission runs on have multi-MB default stack sizes, so a hardwired 4K array should be safe.
comment:8 Changed 12 years ago by charles
- Resolution set to fixed
- Status changed from reopened to closed
Fixed in r12378. Please test.
-- jch