Opened 4 months ago

Closed 4 months ago

Last modified 3 months ago

#22608 closed enhancement (implemented)

Refactor connection_or_set_state(OPEN) to connection_or_set_state_open()

Reported by: nickm Owned by: nickm
Priority: Medium Milestone: Tor: 0.3.2.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: review-group-18 technical-debt
Cc: Actual Points: .3
Parent ID: Points: .2
Reviewer: Sponsor:

Description

I have a patch in branch "callgraph_reduction" that takes the size of our largest SCC from down to 26 functions (from ~64).

Child Tickets

Change History (9)

comment:1 Changed 4 months ago by nickm

Owner: set to nickm
Status: newaccepted

comment:2 Changed 4 months ago by nickm

Status: acceptedneeds_review

comment:3 Changed 4 months ago by nickm

Keywords: review-group-18 added

comment:4 Changed 4 months ago by catalyst

Status: needs_reviewneeds_revision

Looks mostly good. A few nits: callgraph reduction numbers don't match between commit message and this ticket; also it fails check-spaces. If it's a factor-of-three reduction in the largest SCC then maybe it should get a changes file, too?

comment:5 in reply to:  4 Changed 4 months ago by nickm

Replying to catalyst:

Looks mostly good.

Thanks for the review!

A few nits: callgraph reduction numbers don't match between commit message and this ticket; also it fails check-spaces.

Will fix; the numbers have fluctuated between when I originally wrote the patch and now. Right now the reduction is 58 -> 28.

If it's a factor-of-three reduction in the largest SCC then maybe it should get a changes file, too?

Will fix that too. Thanks!

comment:6 Changed 4 months ago by nickm

Status: needs_revisionneeds_review

new branch is callgraph_reduction_v2. Better?

comment:7 Changed 4 months ago by catalyst

Status: needs_reviewmerge_ready

Thanks! Looks good to me. Definitely a noteworthy achievement!

comment:8 Changed 4 months ago by nickm

Actual Points: .3
Resolution: implemented
Status: merge_readyclosed

Merged; thanks for reviewing!

comment:9 Changed 3 months ago by catalyst

Keywords: technical-debt added
Note: See TracTickets for help on using tickets.