Opened 8 years ago

Closed 8 years ago

Last modified 7 years ago

#4518 closed defect (fixed)

assign_onionskin_to_cpuworker is too expensive

Reported by: nickm Owned by:
Priority: High Milestone: Tor: 0.2.2.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: tor-relay
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

The function "assign_onionskin_to_cpuworker()" is 20% of CPU usage on Moritz's profile. If that's so, the likeliest culprit is one of the functions getting inlined there: probably cull_wedged_cpuworkers, which does a linear walk over all the connections.

(I'm guessing that connection_get_by_type_state, which also does a linear walk over all the connections, doesn't show up in the profile because, on a busy server, the average cpuworker is always busy, so assign_onionskin_to_cpuworker mostly gets called from cpuworker.c to assign an onionskin to a cpuworker that just became idle.)

The easiest fix for this would be to only call cull_wedged_cpuworkers every N seconds, or every N invocations of assign_onionskin_to_cpuworker(). This is easy enough that I'm marking this one for 0.2.2

If connection_get_by_type_state shows up in profiles, we can look into another data structure on connections.

Child Tickets

Change History (6)

comment:1 Changed 8 years ago by nickm

Status: newneeds_review

See branch "bug4518" on my public repository for inclusion in 0.2.2.x. This seems like a trivially correct fix to me, and also a big win.

comment:2 Changed 8 years ago by Sebastian

That patch seems fine to me.

comment:3 Changed 8 years ago by arma

Looks great. (s/could/count/ if you want it to be perfect.)

comment:4 Changed 8 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Fixed and merging. Thanks!

comment:5 Changed 7 years ago by nickm

Keywords: tor-relay added

comment:6 Changed 7 years ago by nickm

Component: Tor RelayTor
Note: See TracTickets for help on using tickets.