Opened 5 years ago

Last modified 3 years ago

#16764 new enhancement

Simplify Tor's control flow graph to the extent we can.

Reported by: nickm Owned by:
Priority: High Milestone: Tor: unspecified
Component: Core Tor/Tor Version:
Severity: Normal Keywords: blob, tor-modularity, callgraph, technical-debt
Cc: Actual Points:
Parent ID: Points: parent
Reviewer: Sponsor: SponsorS-can

Description

For background, see http://archives.seul.org/tor/dev/Mar-2015/msg00197.html .

As of this writing, the largest strongly-connected component in Tor's callgraph has 407 functions in it. Nobody can actually understand a program that's so complex. Let's simplify it!

(This is a parent ticket.)

Child Tickets

TicketTypeStatusOwnerSummary
#16695defectclosedDecouple generating controller events from sending them to controllers
#16762enhancementclosedMove most of directory_all_unreachable into a backend callback
#16763defectclosedExtract client-only parts of init_keys()
#16766enhancementclosedDon't call process_signal directly from control.c
#16788enhancementclosedDon't call tor_cleanup() directly from lost_owning_controller()
#16789enhancementclosedDon't launch downloads directly from routerlist_retry_directory_downloads()
#16826defectnewAdd a mechanism to allow callgraph generator to find vtbl-like constructions
#17218enhancementclosedMove most of "circuit_mark_for_close" into "circuit_free" or "circuit_close_all_marked".
#19328defectclosedTry not to log from inside functions called from inside log functions
#19329defectnewIntegrate callgraph complexity measures into our regular process

Change History (20)

comment:1 Changed 5 years ago by nickm

In addition to the tickets currently open, these seem like promising areas to try removing a single call dependency on. Each of them would remove the stated number of functions from the largest SCC:

[(5, 'cell_queues_check_size', 'called by', 'append_cell_to_circuit_queue'),
 (5, 'circuit_build_failed', 'called by', 'circuit_mark_for_close_'),
 (5, 'circuits_handle_oom', 'called by', 'cell_queues_check_size'),
 (5,
  'initiate_descriptor_downloads',
  'called by',
  'launch_descriptor_downloads'),
 (6,
  'connection_or_write_var_cell_to_buf',
  'called by',
  'connection_or_send_versions'),
 (6,
  'connection_tls_continue_handshake',
  'called by',
  'connection_tls_start_handshake'),
 (6,
  'connection_tls_start_handshake',
  'called by',
  'connection_or_finished_connecting'),
 (7,
  'connection_dir_request_failed',
  'called by',
  'directory_initiate_command_rend'),
 (7,
  'update_all_descriptor_downloads',
  'called by',
  'routerlist_retry_directory_downloads'),
 (8,
  'connection_or_finished_connecting',
  'called by',
  'connection_or_connect'),
 (12, 'accounting_run_housekeeping', 'called by', 'or_state_save'),
 (12, 'accounting_set_wakeup_time', 'called by', 'configure_accounting'),
 (12, 'configure_accounting', 'called by', 'accounting_run_housekeeping'),
 (12, 'init_keys', 'called by', 'accounting_set_wakeup_time'),
 (12, 'or_state_save', 'called by', 'tor_cleanup'),
 (13, 'connection_or_connect', 'called by', 'channel_tls_connect'),
 (16, 'lost_owning_controller', 'called by', 'connection_control_closed'),
 (16, 'tor_cleanup', 'called by', 'lost_owning_controller'),
 (18, 'connection_write_to_buf_impl_', 'called by', 'connection_write_to_buf'),
 (19, 'connection_control_closed', 'called by', 'connection_free')]

And these are the functions which (after the currently open tickets are applied) which pull the most other functions into the largest SCC.

[(8, 'connection_or_finished_connecting'),
 (9, 'connection_ap_handshake_attach_circuit'),
 (10, 'circuit_has_opened'),
 (10, 'routerlist_retry_directory_downloads'),
 (11, 'directory_get_from_dirserver'),
 (12, 'accounting_run_housekeeping'),
 (12, 'accounting_set_wakeup_time'),
 (12, 'configure_accounting'),
 (12, 'init_keys'),
 (12, 'or_state_save'),
 (13, 'channel_tls_connect'),
 (13, 'connection_or_connect'),
 (16, 'lost_owning_controller'),
 (16, 'tor_cleanup'),
 (18, 'connection_write_to_buf'),
 (18, 'connection_write_to_buf_impl_'),
 (19, 'connection_control_closed'),
 (19, 'connection_free'),
 (34, 'directory_initiate_command_rend'),
 (40, 'circuit_mark_for_close_')]
Version 0, edited 5 years ago by nickm (next)

comment:2 Changed 5 years ago by nickm

Milestone: Tor: 0.2.7.x-finalTor: 0.2.8.x-final

comment:3 Changed 5 years ago by nickm

Keywords: 028-triage added

comment:4 Changed 5 years ago by nickm

Keywords: SponsorS removed
Sponsor: SponsorS

Bulk-replace SponsorS keyword with SponsorS sponsor field in Tor component.

comment:5 Changed 5 years ago by nickm

Points: medium/large

comment:6 Changed 5 years ago by nickm

Milestone: Tor: 0.2.8.x-finalTor: 0.2.9.x-final

Deferring these two to 0.2.9; we have already made big progress in simplifying the Blob.

comment:7 Changed 5 years ago by isabela

Sponsor: SponsorSSponsorS-can

comment:8 Changed 5 years ago by nickm

Priority: MediumHigh

comment:9 Changed 5 years ago by nickm

Owner: set to nickm
Status: newaccepted

comment:10 Changed 5 years ago by nickm

Keywords: tor-modularity added

comment:11 Changed 4 years ago by isabela

Points: medium/large4.5

comment:12 Changed 4 years ago by nickm

Points: 4.5parent
Severity: Normal

comment:13 Changed 4 years ago by nickm

Owner: nickm deleted
Status: acceptedassigned

comment:14 Changed 4 years ago by nickm

Status: assignednew

Put all unowned "assigned" tickets back into "new".

comment:15 Changed 4 years ago by nickm

Milestone: Tor: 0.2.9.x-finalTor: 0.2.???

This is very largely done. The blob is tiny now. Dropping this from 0.2.9.

comment:16 Changed 4 years ago by teor

Milestone: Tor: 0.2.???Tor: 0.3.???

Milestone renamed

comment:17 Changed 4 years ago by nickm

Keywords: tor-03-unspecified-201612 added
Milestone: Tor: 0.3.???Tor: unspecified

Finally admitting that 0.3.??? was a euphemism for Tor: unspecified all along.

comment:18 Changed 3 years ago by nickm

Keywords: tor-03-unspecified-201612 removed

Remove an old triaging keyword.

comment:19 Changed 3 years ago by nickm

Keywords: 028-triage removed

comment:20 Changed 3 years ago by nickm

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