Possible improvement to churn_allocator

[Dr15Jones]

The churn_allocator

cmssw/TrackingTools/TrajectoryState/interface/ChurnAllocator.h

Line 6 in b205513

class churn_allocator : public std::allocator<T> {

causes memory coupling between modules which use the TrajectoryStateOnSurface class. This makes it difficult to look for memory issues within a module as memory allocation/deallocation spans across module boundaries.

A possible change could use a ChurnManager class which is put onto the stack and then the manager is passed to the churn_allocator's constructor. This would alleviate the need for a thread_local as the manager's lifetime would be limited to one thread. When the ChurnManager goes out of scope, it would make sure the cache is properly released if necessary (unlike now where the cache's pointer is never deleted). To avoid possible problems where the lifetime of the copied churn_allocator is longer than ChurnManger, the churn_allocator could hold a std::weak_pointer to the cache and if the cache is already gone the memory will just be deallocated.

[cmsbuild]

cms-bot internal usage

[cmsbuild]

A new Issue was created by @Dr15Jones.

@Dr15Jones, @antoniovilela, @makortel, @mandrenguyen, @rappoccio, @sextonkennedy, @smuzaffar can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

[makortel]

assign TrackingTools/TrajectoryState

[cmsbuild]

New categories assigned: reconstruction

@jfernan2,@mandrenguyen you have been requested to review this Pull request/Issue and eventually sign? Thanks

[makortel]

type tracking

[Dr15Jones]

The code could look something like

template<T>
class ChurnManager {
   public:

  using pointer = typename std::allocator_traits<std::allocator<T>>::pointer;

    struct Cache {
    pointer cache = nullptr;
    size_type size = 0;
    bool guard = false;
    ~Cache() {
        if (guard) {
            std::allocator<T>::deallocate(cache, size);
        }
    }
  };
  
   std::weak_ptr<Cache>  cache() { return std::weak_ptr<Cache>(cache_); }
   private:
   std::shared_ptr<Cache> cache_;
};

class churn_allocator : public std::allocator<T> {
public:
  using Base = std::allocator<T>;
  using pointer = typename std::allocator_traits<std::allocator<T>>::pointer;
  using size_type = typename Base::size_type;

  explicit churn_allocator(CacheManager<T>& iMananger) : cache_{iManager.cache()} {}

  pointer allocate(size_type n) {
    auto c = cache_.lock();
    if (c) { 
      if (!c->guard) {
        c->cache = std::allocator<T>::allocate(n);
      }
      c->guard = false;
      return c->cache;
    }
    returns std::allocator<T>::allocate(n);
  }

  void deallocate(pointer p, size_type n) {
     auto c = cache_.lock();
    if (c and p == c->cache) {
      c->guard = true;
      c->size = n;
    } else
      std::allocator<T>::deallocate(p, n);
  }
 ...
};