Opened 8 years ago

Closed 2 years ago

#4519 closed defect (wontfix)

circuit_unlink_all_from_orconn is too expensive

Reported by: nickm Owned by:
Priority: High Milestone: Tor: unspecified
Component: Core Tor/Tor Version:
Severity: Normal Keywords: easy tor-relay
Cc: Actual Points:
Parent ID: Points: small/medium
Reviewer: Sponsor:

Description

Moritz's relay spends a lot of time in circuit_unlink_all_from_orconn, which does a linear walk over the entire circuit list.

To make this faster, we'll need data structures. Currently, each or_connection has a doubly-linked list of its active circuits, but not of _all_ its circuits. Adding such a doubly-linked-list is the obvious solution.

Child Tickets

Attachments (1)

0001-Bug-4519-make-global_circuitlist-a-doubly-linked-lis.patch (22.2 KB) - added by marek 6 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 8 years ago by nickm

Milestone: Tor: 0.2.3.x-finalTor: 0.2.4.x-final

This was no longer a killer part of moritz's profiles last I looked. Still worth fixing, but not for 0.2.3.x

comment:2 Changed 7 years ago by nickm

Keywords: easy added

comment:3 Changed 7 years ago by nickm

Keywords: tor-relay added

comment:4 Changed 7 years ago by nickm

Component: Tor RelayTor

comment:5 Changed 7 years ago by nickm

Milestone: Tor: 0.2.4.x-finalTor: unspecified

comment:6 Changed 6 years ago by marek

Status: newneeds_review

If we want to be able to unlink circuit_t without the need of walking through all of the circuits, first we need to make global_circuitlist a doubly-linked list. Attached patch does just that, and improves some other minor things:

  • Doesn't expose global_circuitlist via 'extern', instead calls circuit_get_global_list_() to get the structure.
  • Removes unused circuit_dump_by_chan and its helper circuit_dump_chan_details.
  • Adds #include tor_queue.h to or.h as it's needed.

I don't think this patch is enough to "resolve" this bug, but is definitely necessary.

comment:7 in reply to:  6 ; Changed 6 years ago by rransom

Replying to marek:

If we want to be able to unlink circuit_t without the need of walking through all of the circuits, first we need to make global_circuitlist a doubly-linked list. Attached patch does just that, and improves some other minor things:

  • Doesn't expose global_circuitlist via 'extern', instead calls circuit_get_global_list_() to get the structure.

This should be a separate commit/patch, before your other changes.

  • Removes unused circuit_dump_by_chan and its helper circuit_dump_chan_details.

This should be not only a separate commit/patch, but a separate Trac ticket. (Perhaps the real bug here is that those functions are unused.)

  • Adds #include tor_queue.h to or.h as it's needed.

I don't think this patch is enough to "resolve" this bug, but is definitely necessary.

comment:8 in reply to:  7 Changed 6 years ago by marek

Status: needs_reviewnew

Replying to rransom:

After reading the description of this bug again, I don't think my patch has much to do with it. Sorry. Let's move the discussion to #9107 and #9108 .

comment:9 Changed 4 years ago by nickm

Points: small/medium
Severity: Normal

comment:10 Changed 2 years ago by nickm

Resolution: wontfix
Status: newclosed

If it's not in profiles, let's not speed it up.

Note: See TracTickets for help on using tickets.