commit 73c433a48a15808c87fafbbfe43da60cc4ab7b0e Author: Nick Mathewson <nickm@torproject.org> Date: Wed Feb 24 14:32:09 2016 -0500 Remove the freelist from memarea.c This is in accordance with our usual policy against freelists, now that working allocators are everywhere. It should also make memarea.c's coverage higher. I also doubt that this code ever helped performance. --- changes/remove_memarea_freelist | 4 +++ src/common/memarea.c | 75 +++++++++++------------------------------ src/common/memarea.h | 1 - src/common/timing.c | 0 src/or/main.c | 1 - src/test/test_dir.c | 2 -- src/test/test_options.c | 24 ------------- 7 files changed, 23 insertions(+), 84 deletions(-) diff --git a/changes/remove_memarea_freelist b/changes/remove_memarea_freelist new file mode 100644 index 0000000..dd9520c --- /dev/null +++ b/changes/remove_memarea_freelist @@ -0,0 +1,4 @@ + o Removed code: + - We no longer maintain an internal freelist in memarea.c. Allocators + should be good enough to make this code unnecessary, and it's doubtful + that it ever had any performance benefit. diff --git a/src/common/memarea.c b/src/common/memarea.c index a8e6d45..b648c29 100644 --- a/src/common/memarea.c +++ b/src/common/memarea.c @@ -105,56 +105,32 @@ struct memarea_t { memarea_chunk_t *first; /**< Top of the chunk stack: never NULL. */ }; -/** How many chunks will we put into the freelist before freeing them? */ -#define MAX_FREELIST_LEN 4 -/** The number of memarea chunks currently in our freelist. */ -static int freelist_len=0; -/** A linked list of unused memory area chunks. Used to prevent us from - * spinning in malloc/free loops. */ -static memarea_chunk_t *freelist = NULL; - /** Helper: allocate a new memarea chunk of around <b>chunk_size</b> bytes. */ static memarea_chunk_t * -alloc_chunk(size_t sz, int freelist_ok) +alloc_chunk(size_t sz) { tor_assert(sz < SIZE_T_CEILING); - if (freelist && freelist_ok) { - memarea_chunk_t *res = freelist; - freelist = res->next_chunk; - res->next_chunk = NULL; - --freelist_len; - CHECK_SENTINEL(res); - return res; - } else { - size_t chunk_size = freelist_ok ? CHUNK_SIZE : sz; - memarea_chunk_t *res; - chunk_size += SENTINEL_LEN; - res = tor_malloc(chunk_size); - res->next_chunk = NULL; - res->mem_size = chunk_size - CHUNK_HEADER_SIZE - SENTINEL_LEN; - res->next_mem = res->U_MEM; - tor_assert(res->next_mem+res->mem_size+SENTINEL_LEN == - ((char*)res)+chunk_size); - tor_assert(realign_pointer(res->next_mem) == res->next_mem); - SET_SENTINEL(res); - return res; - } + + size_t chunk_size = sz < CHUNK_SIZE ? CHUNK_SIZE : sz; + memarea_chunk_t *res; + chunk_size += SENTINEL_LEN; + res = tor_malloc(chunk_size); + res->next_chunk = NULL; + res->mem_size = chunk_size - CHUNK_HEADER_SIZE - SENTINEL_LEN; + res->next_mem = res->U_MEM; + tor_assert(res->next_mem+res->mem_size+SENTINEL_LEN == + ((char*)res)+chunk_size); + tor_assert(realign_pointer(res->next_mem) == res->next_mem); + SET_SENTINEL(res); + return res; } -/** Release <b>chunk</b> from a memarea, either by adding it to the freelist - * or by freeing it if the freelist is already too big. */ +/** Release <b>chunk</b> from a memarea. */ static void chunk_free_unchecked(memarea_chunk_t *chunk) { CHECK_SENTINEL(chunk); - if (freelist_len < MAX_FREELIST_LEN) { - ++freelist_len; - chunk->next_chunk = freelist; - freelist = chunk; - chunk->next_mem = chunk->U_MEM; - } else { - tor_free(chunk); - } + tor_free(chunk); } /** Allocate and return new memarea. */ @@ -162,7 +138,7 @@ memarea_t * memarea_new(void) { memarea_t *head = tor_malloc(sizeof(memarea_t)); - head->first = alloc_chunk(CHUNK_SIZE, 1); + head->first = alloc_chunk(CHUNK_SIZE); return head; } @@ -197,19 +173,6 @@ memarea_clear(memarea_t *area) area->first->next_mem = area->first->U_MEM; } -/** Remove all unused memarea chunks from the internal freelist. */ -void -memarea_clear_freelist(void) -{ - memarea_chunk_t *chunk, *next; - freelist_len = 0; - for (chunk = freelist; chunk; chunk = next) { - next = chunk->next_chunk; - tor_free(chunk); - } - freelist = NULL; -} - /** Return true iff <b>p</b> is in a range that has been returned by an * allocation from <b>area</b>. */ int @@ -241,12 +204,12 @@ memarea_alloc(memarea_t *area, size_t sz) if (sz+CHUNK_HEADER_SIZE >= CHUNK_SIZE) { /* This allocation is too big. Stick it in a special chunk, and put * that chunk second in the list. */ - memarea_chunk_t *new_chunk = alloc_chunk(sz+CHUNK_HEADER_SIZE, 0); + memarea_chunk_t *new_chunk = alloc_chunk(sz+CHUNK_HEADER_SIZE); new_chunk->next_chunk = chunk->next_chunk; chunk->next_chunk = new_chunk; chunk = new_chunk; } else { - memarea_chunk_t *new_chunk = alloc_chunk(CHUNK_SIZE, 1); + memarea_chunk_t *new_chunk = alloc_chunk(CHUNK_SIZE); new_chunk->next_chunk = chunk; area->first = chunk = new_chunk; } diff --git a/src/common/memarea.h b/src/common/memarea.h index d14f3a2..4b2f3b4 100644 --- a/src/common/memarea.h +++ b/src/common/memarea.h @@ -18,7 +18,6 @@ char *memarea_strdup(memarea_t *area, const char *s); char *memarea_strndup(memarea_t *area, const char *s, size_t n); void memarea_get_stats(memarea_t *area, size_t *allocated_out, size_t *used_out); -void memarea_clear_freelist(void); void memarea_assert_ok(memarea_t *area); #endif diff --git a/src/common/timing.c b/src/common/timing.c new file mode 100644 index 0000000..e69de29 diff --git a/src/or/main.c b/src/or/main.c index 96a56fc..11caea5 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -3100,7 +3100,6 @@ tor_free_all(int postfork) connection_free_all(); connection_edge_free_all(); scheduler_free_all(); - memarea_clear_freelist(); nodelist_free_all(); microdesc_free_all(); ext_orport_free_all(); diff --git a/src/test/test_dir.c b/src/test/test_dir.c index 72e619f..49719b9 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -497,7 +497,6 @@ test_dir_routerinfo_parsing(void *arg) #undef CHECK_FAIL #undef CHECK_OK done: - memarea_clear_freelist(); routerinfo_free(ri); } @@ -601,7 +600,6 @@ test_dir_extrainfo_parsing(void *arg) done: escaped(NULL); - memarea_clear_freelist(); extrainfo_free(ei); routerinfo_free(ri); digestmap_free((digestmap_t*)map, routerinfo_free_wrapper_); diff --git a/src/test/test_options.c b/src/test/test_options.c index b2be8a9..547464a 100644 --- a/src/test/test_options.c +++ b/src/test/test_options.c @@ -158,7 +158,6 @@ test_options_validate_impl(const char *configuration, } done: - memarea_clear_freelist(); escaped(NULL); policies_free_all(); config_free_lines(cl); @@ -1949,7 +1948,6 @@ test_options_validate__use_bridges(void *ignored) done: NS_UNMOCK(geoip_get_country); - memarea_clear_freelist(); policies_free_all(); free_options_test_data(tdata); tor_free(msg); @@ -2187,7 +2185,6 @@ test_options_validate__publish_server_descriptor(void *ignored) done: teardown_capture_of_logs(previous_log); - memarea_clear_freelist(); policies_free_all(); free_options_test_data(tdata); tor_free(msg); @@ -2263,7 +2260,6 @@ test_options_validate__testing(void *ignored) done: escaped(NULL); // This will free the leaking memory from the previous escaped - memarea_clear_freelist(); policies_free_all(); free_options_test_data(tdata); tor_free(msg); @@ -2311,7 +2307,6 @@ test_options_validate__hidserv(void *ignored) done: teardown_capture_of_logs(previous_log); - memarea_clear_freelist(); policies_free_all(); free_options_test_data(tdata); tor_free(msg); @@ -2336,7 +2331,6 @@ test_options_validate__predicted_ports(void *ignored) done: teardown_capture_of_logs(previous_log); - memarea_clear_freelist(); policies_free_all(); free_options_test_data(tdata); tor_free(msg); @@ -2539,7 +2533,6 @@ test_options_validate__bandwidth(void *ignored) tor_free(msg); done: - memarea_clear_freelist(); policies_free_all(); free_options_test_data(tdata); tor_free(msg); @@ -2627,7 +2620,6 @@ test_options_validate__circuits(void *ignored) tor_free(msg); done: - memarea_clear_freelist(); policies_free_all(); teardown_capture_of_logs(previous_log); free_options_test_data(tdata); @@ -2661,7 +2653,6 @@ test_options_validate__port_forwarding(void *ignored) done: free_options_test_data(tdata); - memarea_clear_freelist(); policies_free_all(); tor_free(msg); } @@ -2691,7 +2682,6 @@ test_options_validate__tor2web(void *ignored) tor_free(msg); done: - memarea_clear_freelist(); policies_free_all(); free_options_test_data(tdata); tor_free(msg); @@ -2758,7 +2748,6 @@ test_options_validate__rend(void *ignored) tor_free(msg); done: - memarea_clear_freelist(); policies_free_all(); teardown_capture_of_logs(previous_log); free_options_test_data(tdata); @@ -2878,7 +2867,6 @@ test_options_validate__accounting(void *ignored) done: teardown_capture_of_logs(previous_log); - memarea_clear_freelist(); policies_free_all(); free_options_test_data(tdata); tor_free(msg); @@ -3210,7 +3198,6 @@ test_options_validate__proxy(void *ignored) done: teardown_capture_of_logs(previous_log); free_options_test_data(tdata); - memarea_clear_freelist(); policies_free_all(); // sandbox_free_getaddrinfo_cache(); tor_free(msg); @@ -3438,7 +3425,6 @@ test_options_validate__control(void *ignored) done: teardown_capture_of_logs(previous_log); - memarea_clear_freelist(); policies_free_all(); free_options_test_data(tdata); tor_free(msg); @@ -3511,7 +3497,6 @@ test_options_validate__families(void *ignored) done: teardown_capture_of_logs(previous_log); - memarea_clear_freelist(); policies_free_all(); free_options_test_data(tdata); tor_free(msg); @@ -3536,7 +3521,6 @@ test_options_validate__addr_policies(void *ignored) tor_free(msg); done: - memarea_clear_freelist(); policies_free_all(); free_options_test_data(tdata); tor_free(msg); @@ -3623,7 +3607,6 @@ test_options_validate__dir_auth(void *ignored) tor_free(msg); done: - memarea_clear_freelist(); policies_free_all(); teardown_capture_of_logs(previous_log); free_options_test_data(tdata); @@ -3749,7 +3732,6 @@ test_options_validate__transport(void *ignored) done: escaped(NULL); // This will free the leaking memory from the previous escaped - memarea_clear_freelist(); policies_free_all(); teardown_capture_of_logs(previous_log); free_options_test_data(tdata); @@ -3833,7 +3815,6 @@ test_options_validate__constrained_sockets(void *ignored) tor_free(msg); done: - memarea_clear_freelist(); policies_free_all(); teardown_capture_of_logs(previous_log); free_options_test_data(tdata); @@ -4053,7 +4034,6 @@ test_options_validate__v3_auth(void *ignored) tor_free(msg); done: - memarea_clear_freelist(); policies_free_all(); teardown_capture_of_logs(previous_log); free_options_test_data(tdata); @@ -4088,7 +4068,6 @@ test_options_validate__virtual_addr(void *ignored) done: escaped(NULL); // This will free the leaking memory from the previous escaped - memarea_clear_freelist(); policies_free_all(); free_options_test_data(tdata); tor_free(msg); @@ -4130,7 +4109,6 @@ test_options_validate__exits(void *ignored) tor_free(msg); done: - memarea_clear_freelist(); policies_free_all(); teardown_capture_of_logs(previous_log); free_options_test_data(tdata); @@ -4299,7 +4277,6 @@ test_options_validate__testing_options(void *ignored) tor_free(msg); done: - memarea_clear_freelist(); policies_free_all(); teardown_capture_of_logs(previous_log); free_options_test_data(tdata); @@ -4353,7 +4330,6 @@ test_options_validate__accel(void *ignored) tor_free(msg); done: - memarea_clear_freelist(); policies_free_all(); free_options_test_data(tdata); tor_free(msg);