Opened 7 years ago

Closed 7 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 7 years ago by jch

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

Fixed in r12378. Please test.

-- jch

comment:2 Changed 7 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 7 years ago by charles

jch, ping

comment:4 Changed 7 years ago by jordan

  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:5 Changed 7 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

Last edited 7 years ago by jch (previous) (diff)

comment:6 Changed 7 years ago by charles

r12382:

(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 some of the extra function call overhead described by jch, we avoid calling calling tr_sessionIsUTPEnabled() and tr_sessionIsDHTEnabled(), and instead apply those simple tests by hand.

Last edited 7 years ago by charles (previous) (diff)

comment:7 Changed 7 years ago by charles

r12381:

(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 7 years ago by charles

  • Resolution set to fixed
  • Status changed from reopened to closed
Note: See TracTickets for help on using tickets.