Remove mempools and buf freelists

They have been off-by-default since 0.2.5 and nobody has complained. :)

Also remove the buf_shrink() function, which hasn't done anything
since we first stopped using contiguous memory to store buffers.

Closes ticket 14848.

4 files changed, 2 insertions, 288 deletions

diff --git a/src/or/buffers.c b/src/or/buffers.c
index ca0e815e33..9f5dc70ed5 100644
--- a/src/or/buffers.c
+++ b/src/or/buffers.c
@@ -105,114 +105,6 @@ chunk_repack(chunk_t *chunk)
 
 /** Keep track of total size of allocated chunks for consistency asserts */
 static size_t total_bytes_allocated_in_chunks = 0;
-
-#if defined(ENABLE_BUF_FREELISTS) || defined(RUNNING_DOXYGEN)
-/** A freelist of chunks. */
-typedef struct chunk_freelist_t {
-  size_t alloc_size; /**< What size chunks does this freelist hold? */
-  int max_length; /**< Never allow more than this number of chunks in the
-                   * freelist. */
-  int slack; /**< When trimming the freelist, leave this number of extra
-              * chunks beyond lowest_length.*/
-  int cur_length; /**< How many chunks on the freelist now? */
-  int lowest_length; /**< What's the smallest value of cur_length since the
-                      * last time we cleaned this freelist? */
-  uint64_t n_alloc;
-  uint64_t n_free;
-  uint64_t n_hit;
-  chunk_t *head; /**< First chunk on the freelist. */
-} chunk_freelist_t;
-
-/** Macro to help define freelists. */
-#define FL(a,m,s) { a, m, s, 0, 0, 0, 0, 0, NULL }
-
-/** Static array of freelists, sorted by alloc_len, terminated by an entry
- * with alloc_size of 0. */
-static chunk_freelist_t freelists[] = {
-  FL(4096, 256, 8), FL(8192, 128, 4), FL(16384, 64, 4), FL(32768, 32, 2),
-  FL(0, 0, 0)
-};
-#undef FL
-/** How many times have we looked for a chunk of a size that no freelist
- * could help with? */
-static uint64_t n_freelist_miss = 0;
-
-static void assert_freelist_ok(chunk_freelist_t *fl);
-
-/** Return the freelist to hold chunks of size <b>alloc</b>, or NULL if
- * no freelist exists for that size. */
-static INLINE chunk_freelist_t *
-get_freelist(size_t alloc)
-{
-  int i;
-  for (i=0; (freelists[i].alloc_size <= alloc &&
-             freelists[i].alloc_size); ++i ) {
-    if (freelists[i].alloc_size == alloc) {
-      return &freelists[i];
-    }
-  }
-  return NULL;
-}
-
-/** Deallocate a chunk or put it on a freelist */
-static void
-chunk_free_unchecked(chunk_t *chunk)
-{
-  size_t alloc;
-  chunk_freelist_t *freelist;
-
-  alloc = CHUNK_ALLOC_SIZE(chunk->memlen);
-  freelist = get_freelist(alloc);
-  if (freelist && freelist->cur_length < freelist->max_length) {
-    chunk->next = freelist->head;
-    freelist->head = chunk;
-    ++freelist->cur_length;
-  } else {
-    if (freelist)
-      ++freelist->n_free;
-#ifdef DEBUG_CHUNK_ALLOC
-    tor_assert(alloc  == chunk->DBG_alloc);
-#endif
-    tor_assert(total_bytes_allocated_in_chunks >= alloc);
-    total_bytes_allocated_in_chunks -= alloc;
-    tor_free(chunk);
-  }
-}
-
-/** Allocate a new chunk with a given allocation size, or get one from the
- * freelist.  Note that a chunk with allocation size A can actually hold only
- * CHUNK_SIZE_WITH_ALLOC(A) bytes in its mem field. */
-static INLINE chunk_t *
-chunk_new_with_alloc_size(size_t alloc)
-{
-  chunk_t *ch;
-  chunk_freelist_t *freelist;
-  tor_assert(alloc >= sizeof(chunk_t));
-  freelist = get_freelist(alloc);
-  if (freelist && freelist->head) {
-    ch = freelist->head;
-    freelist->head = ch->next;
-    if (--freelist->cur_length < freelist->lowest_length)
-      freelist->lowest_length = freelist->cur_length;
-    ++freelist->n_hit;
-  } else {
-    if (freelist)
-      ++freelist->n_alloc;
-    else
-      ++n_freelist_miss;
-    ch = tor_malloc(alloc);
-#ifdef DEBUG_CHUNK_ALLOC
-    ch->DBG_alloc = alloc;
-#endif
-    total_bytes_allocated_in_chunks += alloc;
-  }
-  ch->next = NULL;
-  ch->datalen = 0;
-  ch->memlen = CHUNK_SIZE_WITH_ALLOC(alloc);
-  ch->data = &ch->mem[0];
-  return ch;
-}
-#else
 static void
 chunk_free_unchecked(chunk_t *chunk)
 {
@@ -241,7 +133,6 @@ chunk_new_with_alloc_size(size_t alloc)
   ch->data = &ch->mem[0];
   return ch;
 }
-#endif
 
 /** Expand <b>chunk</b> until it can hold <b>sz</b> bytes, and return a
  * new pointer to <b>chunk</b>.  Old pointers are no longer valid. */
@@ -284,115 +175,6 @@ preferred_chunk_size(size_t target)
   return sz;
 }
 
-/** Remove from the freelists most chunks that have not been used since the
- * last call to buf_shrink_freelists().   Return the amount of memory
- * freed. */
-size_t
-buf_shrink_freelists(int free_all)
-{
-#ifdef ENABLE_BUF_FREELISTS
-  int i;
-  size_t total_freed = 0;
-  disable_control_logging();
-  for (i = 0; freelists[i].alloc_size; ++i) {
-    int slack = freelists[i].slack;
-    assert_freelist_ok(&freelists[i]);
-    if (free_all || freelists[i].lowest_length > slack) {
-      int n_to_free = free_all ? freelists[i].cur_length :
-        (freelists[i].lowest_length - slack);
-      int n_to_skip = freelists[i].cur_length - n_to_free;
-      int orig_length = freelists[i].cur_length;
-      int orig_n_to_free = n_to_free, n_freed=0;
-      int orig_n_to_skip = n_to_skip;
-      int new_length = n_to_skip;
-      chunk_t **chp = &freelists[i].head;
-      chunk_t *chunk;
-      while (n_to_skip) {
-        if (!(*chp) || ! (*chp)->next) {
-          log_warn(LD_BUG, "I wanted to skip %d chunks in the freelist for "
-                   "%d-byte chunks, but only found %d. (Length %d)",
-                   orig_n_to_skip, (int)freelists[i].alloc_size,
-                   orig_n_to_skip-n_to_skip, freelists[i].cur_length);
-          assert_freelist_ok(&freelists[i]);
-          goto done;
-        }
-        // tor_assert((*chp)->next);
-        chp = &(*chp)->next;
-        --n_to_skip;
-      }
-      chunk = *chp;
-      *chp = NULL;
-      while (chunk) {
-        chunk_t *next = chunk->next;
-#ifdef DEBUG_CHUNK_ALLOC
-        tor_assert(chunk->DBG_alloc == CHUNK_ALLOC_SIZE(chunk->memlen));
-#endif
-        tor_assert(total_bytes_allocated_in_chunks >=
-                   CHUNK_ALLOC_SIZE(chunk->memlen));
-        total_bytes_allocated_in_chunks -= CHUNK_ALLOC_SIZE(chunk->memlen);
-        total_freed += CHUNK_ALLOC_SIZE(chunk->memlen);
-        tor_free(chunk);
-        chunk = next;
-        --n_to_free;
-        ++n_freed;
-        ++freelists[i].n_free;
-      }
-      if (n_to_free) {
-        log_warn(LD_BUG, "Freelist length for %d-byte chunks may have been "
-                 "messed up somehow.", (int)freelists[i].alloc_size);
-        log_warn(LD_BUG, "There were %d chunks at the start.  I decided to "
-                 "keep %d. I wanted to free %d.  I freed %d.  I somehow think "
-                 "I have %d left to free.",
-                 freelists[i].cur_length, n_to_skip, orig_n_to_free,
-                 n_freed, n_to_free);
-      }
-      // tor_assert(!n_to_free);
-      freelists[i].cur_length = new_length;
-      tor_assert(orig_n_to_skip == new_length);
-      log_info(LD_MM, "Cleaned freelist for %d-byte chunks: original "
-               "length %d, kept %d, dropped %d. New length is %d",
-               (int)freelists[i].alloc_size, orig_length,
-               orig_n_to_skip, orig_n_to_free, new_length);
-    }
-    freelists[i].lowest_length = freelists[i].cur_length;
-    assert_freelist_ok(&freelists[i]);
-  }
- done:
-  enable_control_logging();
-  return total_freed;
-#else
-  (void) free_all;
-  return 0;
-#endif
-}
-
-/** Describe the current status of the freelists at log level <b>severity</b>.
- */
-void
-buf_dump_freelist_sizes(int severity)
-{
-#ifdef ENABLE_BUF_FREELISTS
-  int i;
-  tor_log(severity, LD_MM, "====== Buffer freelists:");
-  for (i = 0; freelists[i].alloc_size; ++i) {
-    uint64_t total = ((uint64_t)freelists[i].cur_length) *
-      freelists[i].alloc_size;
-    tor_log(severity, LD_MM,
-        U64_FORMAT" bytes in %d %d-byte chunks ["U64_FORMAT
-        " misses; "U64_FORMAT" frees; "U64_FORMAT" hits]",
-        U64_PRINTF_ARG(total),
-        freelists[i].cur_length, (int)freelists[i].alloc_size,
-        U64_PRINTF_ARG(freelists[i].n_alloc),
-        U64_PRINTF_ARG(freelists[i].n_free),
-        U64_PRINTF_ARG(freelists[i].n_hit));
-  }
-  tor_log(severity, LD_MM, U64_FORMAT" allocations in non-freelist sizes",
-      U64_PRINTF_ARG(n_freelist_miss));
-#else
-  (void)severity;
-#endif
-}
-
 /** Collapse data from the first N chunks from <b>buf</b> into buf->head,
  * growing it as necessary, until buf->head has the first <b>bytes</b> bytes
  * of data from the buffer, or until buf->head has all the data in <b>buf</b>.
@@ -488,15 +270,6 @@ buf_get_first_chunk_data(const buf_t *buf, const char **cp, size_t *sz)
 }
 #endif
 
-/** Resize buf so it won't hold extra memory that we haven't been
- * using lately.
- */
-void
-buf_shrink(buf_t *buf)
-{
-  (void)buf;
-}
-
 /** Remove the first <b>n</b> bytes from buf. */
 static INLINE void
 buf_remove_from_front(buf_t *buf, size_t n)
@@ -2672,23 +2445,3 @@ assert_buf_ok(buf_t *buf)
   }
 }
 
-#ifdef ENABLE_BUF_FREELISTS
-/** Log an error and exit if <b>fl</b> is corrupted.
- */
-static void
-assert_freelist_ok(chunk_freelist_t *fl)
-{
-  chunk_t *ch;
-  int n;
-  tor_assert(fl->alloc_size > 0);
-  n = 0;
-  for (ch = fl->head; ch; ch = ch->next) {
-    tor_assert(CHUNK_ALLOC_SIZE(ch->memlen) == fl->alloc_size);
-    ++n;
-  }
-  tor_assert(n == fl->cur_length);
-  tor_assert(n >= fl->lowest_length);
-  tor_assert(n <= fl->max_length);
-}
-#endif
-
diff --git a/src/or/buffers.h b/src/or/buffers.h
index 6dd3d1762b..6d0c68500b 100644
--- a/src/or/buffers.h
+++ b/src/or/buffers.h
@@ -20,9 +20,6 @@ size_t buf_get_default_chunk_size(const buf_t *buf);
 void buf_free(buf_t *buf);
 void buf_clear(buf_t *buf);
 buf_t *buf_copy(const buf_t *buf);
-void buf_shrink(buf_t *buf);
-size_t buf_shrink_freelists(int free_all);
-void buf_dump_freelist_sizes(int severity);
 
 MOCK_DECL(size_t, buf_datalen, (const buf_t *buf));
 size_t buf_allocation(const buf_t *buf);
@@ -108,9 +105,9 @@ STATIC void buf_pullup(buf_t *buf, size_t bytes, int nulterminate);
 void buf_get_first_chunk_data(const buf_t *buf, const char **cp, size_t *sz);
 
 #define DEBUG_CHUNK_ALLOC
-/** A single chunk on a buffer or in a freelist. */
+/** A single chunk on a buffer. */
 typedef struct chunk_t {
-  struct chunk_t *next; /**< The next chunk on the buffer or freelist. */
+  struct chunk_t *next; /**< The next chunk on the buffer. */
   size_t datalen; /**< The number of bytes stored in this chunk */
   size_t memlen; /**< The number of usable bytes of storage in <b>mem</b>. */
 #ifdef DEBUG_CHUNK_ALLOC
diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c
index d964e66922..5355b17c1d 100644
--- a/src/or/circuitlist.c
+++ b/src/or/circuitlist.c
@@ -2063,17 +2063,6 @@ circuits_handle_oom(size_t current_allocation)
              "MaxMemInQueues.)");
 
   {
-    const size_t recovered = buf_shrink_freelists(1);
-    if (recovered >= current_allocation) {
-      log_warn(LD_BUG, "We somehow recovered more memory from freelists "
-               "than we thought we had allocated");
-      current_allocation = 0;
-    } else {
-      current_allocation -= recovered;
-    }
-  }
-
-  {
     size_t mem_target = (size_t)(get_options()->MaxMemInQueues *
                                  FRACTION_OF_DATA_TO_RETAIN_ON_OOM);
     if (current_allocation <= mem_target)
@@ -2156,12 +2145,6 @@ circuits_handle_oom(size_t current_allocation)
 
  done_recovering_mem:
 
-#ifdef ENABLE_MEMPOOLS
-  clean_cell_pool(); /* In case this helps. */
-#endif /* ENABLE_MEMPOOLS */
-  buf_shrink_freelists(1); /* This is necessary to actually release buffer
-                              chunks. */
-
   log_notice(LD_GENERAL, "Removed "U64_FORMAT" bytes by killing %d circuits; "
              "%d circuits remain alive. Also killed %d non-linked directory "
              "connections.",
diff --git a/src/or/main.c b/src/or/main.c
index 136043c117..09144cb1fb 100644
--- a/src/or/main.c
+++ b/src/or/main.c
@@ -1223,7 +1223,6 @@ run_scheduled_events(time_t now)
   static time_t time_to_check_v3_certificate = 0;
   static time_t time_to_check_listeners = 0;
   static time_t time_to_download_networkstatus = 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;
@@ -1573,22 +1572,6 @@ run_scheduled_events(time_t now)
   for (i=0;i<smartlist_len(connection_array);i++) {
     run_connection_housekeeping(i, now);
   }
-  if (time_to_shrink_memory < now) {
-    SMARTLIST_FOREACH(connection_array, connection_t *, conn, {
-        if (conn->outbuf)
-          buf_shrink(conn->outbuf);
-        if (conn->inbuf)
-          buf_shrink(conn->inbuf);
-      });
-#ifdef ENABLE_MEMPOOL
-    clean_cell_pool();
-#endif /* ENABLE_MEMPOOL */
-    buf_shrink_freelists(0);
-/** How often do we check buffers and pools for empty space that can be
- * deallocated? */
-#define MEM_SHRINK_INTERVAL (60)
-    time_to_shrink_memory = now + MEM_SHRINK_INTERVAL;
-  }
 
   /* 6. And remove any marked circuits... */
   circuit_close_all_marked();
@@ -2260,7 +2243,6 @@ dumpmemusage(int severity)
   dump_routerlist_mem_usage(severity);
   dump_cell_pool_usage(severity);
   dump_dns_mem_usage(severity);
-  buf_dump_freelist_sizes(severity);
   tor_log_mallinfo(severity);
 }
 
@@ -2652,7 +2634,6 @@ tor_free_all(int postfork)
   channel_free_all();
   connection_free_all();
   scheduler_free_all();
-  buf_shrink_freelists(1);
   memarea_clear_freelist();
   nodelist_free_all();
   microdesc_free_all();