Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#25008 closed defect (implemented)

Call protocol_list_supports_protocol less often to save time and allocation

Reported by: nickm Owned by: nickm
Priority: Medium Milestone: Tor: 0.3.3.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: 032-backport?, malloc, performance, protover, review-group-31
Cc: Actual Points:
Parent ID: #23777 Points:
Reviewer: dgoulet Sponsor:


This function appeared on the heap profile of #23777, and there's no reason it should have done so. It appears to have been getting called by circuit_extend(), which suggests that we have some dodgy logic somewhere. I blame node_supports_ed25519_link_allocation.

Child Tickets

Change History (7)

comment:1 Changed 3 years ago by nickm

Status: assignedneeds_review

Proposed change in branch bug25008.

comment:2 Changed 3 years ago by nickm

Keywords: review-group-31 added

comment:3 Changed 3 years ago by dgoulet

Reviewer: dgoulet
Status: needs_reviewneeds_revision

Oh hello wonderful patch! I very much like this! It adds a nice semantic to the code and optimize it at the same time.

I may ask if we can get the documentation of which protocol version value each member of protover_summary_flags_t is for. For instance, supports_ed25519_hs_intro requires HSIntro=4 but half of them aren't documented. And easy map with the spec would rock.

comment:4 Changed 3 years ago by nickm

Status: needs_revisionneeds_review

I've fixed that issue, and added a fix for #25105. (See also #25105). I've also added a fix for a unit test that wasn't passing any more.

comment:5 Changed 3 years ago by dgoulet

Status: needs_reviewmerge_ready


Good stuff.

comment:6 Changed 3 years ago by nickm

Resolution: implemented
Status: merge_readyclosed


comment:7 Changed 3 years ago by teor

This patch triggers a crash on authorities when a relay stops advertising a protocol list (#25415).

Note: See TracTickets for help on using tickets.