diff --git a/doc/TODO b/doc/TODO index 2087b7600d..f317ca9ecf 100644 --- a/doc/TODO +++ b/doc/TODO @@ -79,7 +79,7 @@ Things we'd like to do in 0.2.0.x: o Implement pool-allocation o Have Tor use it for packed cells. o Document it. - - Do something smart with freeing unused chunks. + o Do something smart with freeing unused chunks. - Benchmark pool-allocation vs straightforward malloc. - Can we stop doing so many memcpys on cells? o Also, only package data from exitconns when there is space on the @@ -271,7 +271,7 @@ Minor items for 0.1.2.x as time permits: R - add d64 and fp64 along-side d and fp so people can paste status entries into a url. since + is a valid base64 char, only allow one at a time. spec and then do. - - When we export something from foo.c file for testing purposes only, + o When we export something from foo.c file for testing purposes only, make a foo_test.h file for test.c to include... or put them behind an #ifdef FOO_PRIVATE. - The Debian package now uses --verify-config when (re)starting, diff --git a/src/common/mempool.c b/src/common/mempool.c index 06f23ad94f..ca196530db 100644 --- a/src/common/mempool.c +++ b/src/common/mempool.c @@ -210,6 +210,8 @@ mp_pool_get(mp_pool_t *pool) ASSERT(!chunk->prev); --pool->n_empty_chunks; + if (pool->n_empty_chunks < pool->min_empty_chunks) + pool->min_empty_chunks = pool->n_empty_chunks; } else { /* We have no used or empty chunks: allocate a new chunk. */ chunk = mp_chunk_new(pool); @@ -375,11 +377,20 @@ mp_pool_new(size_t item_size, size_t chunk_capacity) } /** If there are more than n empty chunks in pool, free the - * exces ones that have been empty for the longest. */ + * exces ones that have been empty for the longest. (If n is less + * than zero, free only empty chunks that were not used since the last + * call to mp_pool_clean(), leaving only -n.) */ void mp_pool_clean(mp_pool_t *pool, int n) { mp_chunk_t *chunk, **first_to_free; + if (n < 0) { + n = pool->min_empty_chunks + (-n); + if (n < pool->n_empty_chunks) + pool->min_empty_chunks = n; + } + ASSERT(n>=0); + first_to_free = &pool->empty_chunks; while (*first_to_free && n > 0) { first_to_free = &(*first_to_free)->next; diff --git a/src/common/mempool.h b/src/common/mempool.h index 1f9e2f5534..09174b2a55 100644 --- a/src/common/mempool.h +++ b/src/common/mempool.h @@ -38,6 +38,9 @@ struct mp_pool_t { struct mp_chunk_t *full_chunks; /** Length of empty_chunks. */ int n_empty_chunks; + /** Lowest value of empty_chunks since last call to + * mp_pool_clean(-1). */ + int min_empty_chunks; /** Size of each chunk (in items). */ int new_chunk_capacity; /** Size to allocate for each item, including overhead and alignment diff --git a/src/or/config.c b/src/or/config.c index 33065427c6..c7b7fdf24b 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -11,6 +11,8 @@ const char config_c_id[] = \ * \brief Code to parse and interpret configuration files. **/ +#define CONFIG_PRIVATE + #include "or.h" #ifdef MS_WINDOWS #include @@ -594,8 +596,6 @@ static le_version_t decode_libevent_version(void); static void check_libevent_version(const char *m, int server); #endif -/*static*/ or_options_t *options_new(void); - /** Magic value for or_options_t. */ #define OR_OPTIONS_MAGIC 9090909 diff --git a/src/or/control.c b/src/or/control.c index e8ed0acdde..17744939fb 100644 --- a/src/or/control.c +++ b/src/or/control.c @@ -10,6 +10,8 @@ const char control_c_id[] = * See control-spec.txt for full details on protocol. **/ +#define CONTROL_PRIVATE + #include "or.h" /** Yield true iff s is the state of a control_connection_t that has @@ -84,10 +86,6 @@ typedef int event_format_t; static void connection_printf_to_buf(control_connection_t *conn, const char *format, ...) CHECK_PRINTF(2,3); -/*static*/ size_t write_escaped_data(const char *data, size_t len, - int translate_newlines, char **out); -/*static*/ size_t read_escaped_data(const char *data, size_t len, - int translate_newlines, char **out); static void send_control_done(control_connection_t *conn); static void send_control_event(uint16_t event, event_format_t which, const char *format, ...) @@ -317,7 +315,7 @@ write_escaped_data(const char *data, size_t len, int translate_newlines, * that appears at the start of a line. If translate_newlines * is true, replace all CRLF sequences with LF. Return the number of * bytes in *out. */ -/*static*/ size_t +/* static */ size_t read_escaped_data(const char *data, size_t len, int translate_newlines, char **out) { diff --git a/src/or/dirserv.c b/src/or/dirserv.c index 0ee7fc067f..048f7f7b63 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -35,6 +35,7 @@ static void directory_remove_invalid(void); static cached_dir_t *dirserv_regenerate_directory(void); static char *format_versions_list(config_line_t *ln); /* Should be static; exposed for testing */ +/* XXXX020 not actually tested. */ struct authdir_config_t; int add_fingerprint_to_dir(const char *nickname, const char *fp, struct authdir_config_t *list); @@ -72,6 +73,7 @@ typedef struct authdir_config_t { } authdir_config_t; /** Should be static; exposed for testing. */ +/* XXXX020 not actually tested. */ authdir_config_t *fingerprint_list = NULL; /** Allocate and return a new, empty, authdir_config_t. */ @@ -88,7 +90,7 @@ authdir_config_new(void) * the smartlist of fingerprint_entry_t's list. Return 0 if it's * new, or 1 if we replaced the old value. */ -int /* Should be static; exposed for testing */ +/* static */ int add_fingerprint_to_dir(const char *nickname, const char *fp, authdir_config_t *list) { diff --git a/src/or/main.c b/src/or/main.c index 9a063fb09f..f42223641d 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -117,8 +117,9 @@ static char* nt_strerror(uint32_t errnum); #define CHECK_DESCRIPTOR_INTERVAL (60) /** How often do we (as a router) check whether our IP address has changed? */ #define CHECK_IPADDRESS_INTERVAL (15*60) -/** How often do we check buffers for empty space that can be deallocated? */ -#define BUF_SHRINK_INTERVAL (60) +/** How often do we check buffers and pools for empty space that can be + * deallocated? */ +#define MEM_SHRINK_INTERVAL (60) /** How often do we check for router descriptors that we should download? */ #define DESCRIPTOR_RETRY_INTERVAL (10) /** How often do we 'forgive' undownloadable router descriptors and attempt @@ -747,7 +748,7 @@ run_scheduled_events(time_t now) static time_t time_to_check_listeners = 0; static time_t time_to_check_descriptor = 0; static time_t time_to_check_ipaddress = 0; - static time_t time_to_shrink_buffers = 0; + static time_t time_to_shrink_memory = 0; static time_t time_to_try_getting_descriptors = 0; static time_t time_to_reset_descriptor_failures = 0; static time_t time_to_add_entropy = 0; @@ -941,7 +942,7 @@ run_scheduled_events(time_t now) for (i=0;ioutbuf) @@ -949,7 +950,8 @@ run_scheduled_events(time_t now) if (conn->inbuf) buf_shrink(conn->inbuf); } - time_to_shrink_buffers = now + BUF_SHRINK_INTERVAL; + clean_cell_pool(); + time_to_shrink_memory = now + MEM_SHRINK_INTERVAL; } /** 6. And remove any marked circuits... */ diff --git a/src/or/or.h b/src/or/or.h index 048e6793c5..9ecf46d63e 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -2137,6 +2137,11 @@ int or_state_save(time_t now); int getinfo_helper_config(control_connection_t *conn, const char *question, char **answer); +#ifdef CONFIG_PRIVATE +/* Used only by config.c and test.c */ +or_options_t *options_new(void); +#endif + /********************************* connection.c ***************************/ const char *conn_type_to_string(int type); @@ -2406,6 +2411,14 @@ int decode_hashed_password(char *buf, const char *hashed); void disable_control_logging(void); void enable_control_logging(void); +#ifdef CONTROL_PRIVATE +/* Used only by control.c and test.c */ +size_t write_escaped_data(const char *data, size_t len, + int translate_newlines, char **out); +size_t read_escaped_data(const char *data, size_t len, + int translate_newlines, char **out); +#endif + /********************************* cpuworker.c *****************************/ void cpu_init(void); @@ -2669,6 +2682,7 @@ extern uint64_t stats_n_data_bytes_received; void init_cell_pool(void); void free_cell_pool(void); +void clean_cell_pool(void); void cell_queue_clear(cell_queue_t *queue); void cell_queue_append(cell_queue_t *queue, packed_cell_t *cell); @@ -2885,6 +2899,11 @@ void router_reset_warnings(void); void router_reset_reachability(void); void router_free_all(void); +#ifdef ROUTER_PRIVATE +/* Used only by router.c and test.c */ +void get_platform_str(char *platform, size_t len); +#endif + /********************************* routerlist.c ***************************/ /** Represents information about a single trusted directory server. */ diff --git a/src/or/relay.c b/src/or/relay.c index f6b4cbb4be..1f949e5a3a 100644 --- a/src/or/relay.c +++ b/src/or/relay.c @@ -1497,6 +1497,14 @@ free_cell_pool(void) cell_pool = NULL; } +/** Free excess storage in cell pool. */ +void +clean_cell_pool(void) +{ + tor_assert(cell_pool); + mp_pool_clean(cell_pool, -1); +} + /** Release storage held by cell. */ static INLINE void packed_cell_free(packed_cell_t *cell) @@ -1523,6 +1531,11 @@ free_cell_pool(void) { } +void +clean_cell_pool(void) +{ +} + static INLINE void packed_cell_free(packed_cell_t *cell) { diff --git a/src/or/router.c b/src/or/router.c index 6815554ea3..fc6c62e07d 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -6,6 +6,8 @@ const char router_c_id[] = "$Id$"; +#define ROUTER_PRIVATE + #include "or.h" /** @@ -16,8 +18,6 @@ const char router_c_id[] = extern long stats_n_seconds_working; -/* Exposed for test.c. */ void get_platform_str(char *platform, size_t len); - /************************************************************/ /***** diff --git a/src/or/test.c b/src/or/test.c index 68fb97e44c..0d6cfd270c 100644 --- a/src/or/test.c +++ b/src/or/test.c @@ -23,7 +23,12 @@ const char test_c_id[] = #include #endif +/* These macros pull in declarations for some functions and structures that + * are typically file-private. */ +#define CONFIG_PRIVATE +#define CONTROL_PRIVATE #define MEMPOOL_PRIVATE +#define ROUTER_PRIVATE #include "or.h" #include "../common/test.h" @@ -32,14 +37,6 @@ const char test_c_id[] = int have_failed = 0; -/* These functions are file-local, but are exposed so we can test. */ -void get_platform_str(char *platform, size_t len); -size_t read_escaped_data(const char *data, size_t len, int translate_newlines, - char **out); -or_options_t *options_new(void); -int parse_addr_policy(config_line_t *cfg, addr_policy_t **dest, - int assume_action); - static char temp_dir[256]; static void @@ -2142,7 +2139,7 @@ test_mempool(void) //mp_pool_assert_ok(pool); } if (crypto_rand_int(777)==0) - mp_pool_clean(pool, 2); + mp_pool_clean(pool, -1); if (i % 777) mp_pool_assert_ok(pool);