1. 19 8月, 2020 2 次提交
    • E
      mem-pool: use more standard initialization and finalization · 44c7e1a7
      Elijah Newren 提交于
      A typical memory type, such as strbuf, hashmap, or string_list can be
      stored on the stack or embedded within another structure.  mem_pool
      cannot be, because of how mem_pool_init() and mem_pool_discard() are
      written.  mem_pool_init() does essentially the following (simplified
      for purposes of explanation here):
      
          void mem_pool_init(struct mem_pool **pool...)
          {
              *pool = xcalloc(1, sizeof(*pool));
      
      It seems weird to require that mem_pools can only be accessed through a
      pointer.  It also seems slightly dangerous: unlike strbuf_release() or
      strbuf_reset() or string_list_clear(), all of which put the data
      structure into a state where it can be re-used after the call,
      mem_pool_discard(pool) will leave pool pointing at free'd memory.
      read-cache (and split-index) are the only current users of mem_pools,
      and they haven't fallen into a use-after-free mistake here, but it seems
      likely to be problematic for future users especially since several of
      the current callers of mem_pool_init() will only call it when the
      mem_pool* is not already allocated (i.e. is NULL).
      
      This type of mechanism also prevents finding synchronization
      points where one can free existing memory and then resume more
      operations.  It would be natural at such points to run something like
          mem_pool_discard(pool...);
      and, if necessary,
          mem_pool_init(&pool...);
      and then carry on continuing to use the pool.  However, this fails badly
      if several objects had a copy of the value of pool from before these
      commands; in such a case, those objects won't get the updated value of
      pool that mem_pool_init() overwrites pool with and they'll all instead
      be reading and writing from free'd memory.
      
      Modify mem_pool_init()/mem_pool_discard() to behave more like
         strbuf_init()/strbuf_release()
      or
         string_list_init()/string_list_clear()
      In particular: (1) make mem_pool_init() just take a mem_pool* and have
      it only worry about allocating struct mp_blocks, not the struct mem_pool
      itself, (2) make mem_pool_discard() free the memory that the pool was
      responsible for, but leave it in a state where it can be used to
      allocate more memory afterward (without the need to call mem_pool_init()
      again).
      Signed-off-by: NElijah Newren <newren@gmail.com>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      44c7e1a7
    • E
      mem-pool: add convenience functions for strdup and strndup · a762c8c1
      Elijah Newren 提交于
      fast-import had a special mem_pool_strdup() convenience function that I
      want to be able to use from the new merge algorithm I am writing.  Move
      it from fast-import to mem-pool, and also add a mem_pool_strndup()
      while at it that I also want to use.
      Signed-off-by: NElijah Newren <newren@gmail.com>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      a762c8c1
  2. 04 7月, 2018 5 次提交
    • J
      block alloc: add validations around cache_entry lifecyle · 8616a2d0
      Jameson Miller 提交于
      Add an option (controlled by an environment variable) perform extra
      validations on mem_pool allocated cache entries. When set:
      
        1) Invalidate cache_entry memory when discarding cache_entry.
      
        2) When discarding index_state struct, verify that all cache_entries
           were allocated from expected mem_pool.
      
        3) When discarding mem_pools, invalidate mem_pool memory.
      
      This should provide extra checks that mem_pools and their allocated
      cache_entries are being used as expected.
      Signed-off-by: NJameson Miller <jamill@microsoft.com>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      8616a2d0
    • J
      block alloc: allocate cache entries from mem_pool · 8e72d675
      Jameson Miller 提交于
      When reading large indexes from disk, a portion of the time is
      dominated in malloc() calls. This can be mitigated by allocating a
      large block of memory and manage it ourselves via memory pools.
      
      This change moves the cache entry allocation to be on top of memory
      pools.
      
      Design:
      
      The index_state struct will gain a notion of an associated memory_pool
      from which cache_entries will be allocated from. When reading in the
      index from disk, we have information on the number of entries and
      their size, which can guide us in deciding how large our initial
      memory allocation should be. When an index is discarded, the
      associated memory_pool will be discarded as well - so the lifetime of
      a cache_entry is tied to the lifetime of the index_state that it was
      allocated for.
      
      In the case of a Split Index, the following rules are followed. 1st,
      some terminology is defined:
      
      Terminology:
        - 'the_index': represents the logical view of the index
      
        - 'split_index': represents the "base" cache entries. Read from the
          split index file.
      
      'the_index' can reference a single split_index, as well as
      cache_entries from the split_index. `the_index` will be discarded
      before the `split_index` is.  This means that when we are allocating
      cache_entries in the presence of a split index, we need to allocate
      the entries from the `split_index`'s memory pool.  This allows us to
      follow the pattern that `the_index` can reference cache_entries from
      the `split_index`, and that the cache_entries will not be freed while
      they are still being referenced.
      
      Managing transient cache_entry structs:
      Cache entries are usually allocated for an index, but this is not always
      the case. Cache entries are sometimes allocated because this is the
      type that the existing checkout_entry function works with. Because of
      this, the existing code needs to handle cache entries associated with an
      index / memory pool, and those that only exist transiently. Several
      strategies were contemplated around how to handle this:
      
      Chosen approach:
      An extra field was added to the cache_entry type to track whether the
      cache_entry was allocated from a memory pool or not. This is currently
      an int field, as there are no more available bits in the existing
      ce_flags bit field. If / when more bits are needed, this new field can
      be turned into a proper bit field.
      
      Alternatives:
      
      1) Do not include any information about how the cache_entry was
      allocated. Calling code would be responsible for tracking whether the
      cache_entry needed to be freed or not.
        Pro: No extra memory overhead to track this state
        Con: Extra complexity in callers to handle this correctly.
      
      The extra complexity and burden to not regress this behavior in the
      future was more than we wanted.
      
      2) cache_entry would gain knowledge about which mem_pool allocated it
        Pro: Could (potentially) do extra logic to know when a mem_pool no
             longer had references to any cache_entry
        Con: cache_entry would grow heavier by a pointer, instead of int
      
      We didn't see a tangible benefit to this approach
      
      3) Do not add any extra information to a cache_entry, but when freeing a
         cache entry, check if the memory exists in a region managed by existing
         mem_pools.
        Pro: No extra memory overhead to track state
        Con: Extra computation is performed when freeing cache entries
      
      We decided tracking and iterating over known memory pool regions was
      less desirable than adding an extra field to track this stae.
      Signed-off-by: NJameson Miller <jamill@microsoft.com>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      8e72d675
    • J
      mem-pool: fill out functionality · 0e58301d
      Jameson Miller 提交于
      Add functions for:
      
          - combining two memory pools
      
          - determining if a memory address is within the range managed by a
            memory pool
      
      These functions will be used by future commits.
      Signed-off-by: NJameson Miller <jamill@microsoft.com>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      0e58301d
    • J
      mem-pool: add life cycle management functions · 158dfeff
      Jameson Miller 提交于
      Add initialization and discard functions to mem_pool type. As the
      memory allocated by mem_pool can now be freed, we also track the large
      allocations.
      
      If the there are existing mp_blocks in the mem_poo's linked list of
      mp_blocksl, then the mp_block for a large allocation is inserted
      behind the head block. This is because only the head mp_block is considered
      when searching for availble space. This results in the following
      desirable properties:
      
      1) The mp_block allocated for the large request will not be included
      not included in the search for available in future requests, the large
      mp_block is sized for the specific request and does not contain any
      spare space.
      
      2) The head mp_block will not bumped from considation for future
      memory requests just because a request for a large chunk of memory
      came in.
      
      These changes are in preparation for a future commit that will utilize
      creating and discarding memory pool.
      Signed-off-by: NJameson Miller <jamill@microsoft.com>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      158dfeff
    • J
      mem-pool: only search head block for available space · 8fb8e3f6
      Jameson Miller 提交于
      Instead of searching all memory blocks for available space to fulfill
      a memory request, only search the head block. If the head block does
      not have space, assume that previous block would most likely not be
      able to fulfill request either. This could potentially lead to more
      memory fragmentation, but also avoids searching memory blocks that
      probably will not be able to fulfill request.
      
      This pattern will benefit consumers that are able to generate a good
      estimate for how much memory will be needed, or if they are performing
      fixed sized allocations, so that once a block is exhausted it will
      never be able to fulfill a future request.
      Signed-off-by: NJameson Miller <jamill@microsoft.com>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      8fb8e3f6
  3. 12 4月, 2018 1 次提交