• J
    refactor SavePoints (#5192) · de769094
    jsteemann 提交于
    Summary:
    Savepoints are assumed to be used in a stack-wise fashion (only
    the top element should be used), so they were stored by `WriteBatch`
    in a member variable `save_points` using an std::stack.
    
    Conceptually this is fine, but the implementation had a few issues:
    - the `save_points_` instance variable was a plain pointer to a heap-
      allocated `SavePoints` struct. The destructor of `WriteBatch` simply
      deletes this pointer. However, the copy constructor of WriteBatch
      just copied that pointer, meaning that copying a WriteBatch with
      active savepoints will very likely have crashed before. Now a proper
      copy of the savepoints is made in the copy constructor, and not just
      a copy of the pointer
    - `save_points_` was an std::stack, which defaults to `std::deque` for
      the underlying container. A deque is a bit over the top here, as we
      only need access to the most recent savepoint (i.e. stack.top()) but
      never any elements at the front. std::deque is rather expensive to
      initialize in common environments. For example, the STL implementation
      shipped with GNU g++ will perform a heap allocation of more than 500
      bytes to create an empty deque object. Although the `save_points_`
      container is created lazily by RocksDB, moving from a deque to a plain
      `std::vector` is much more memory-efficient. So `save_points_` is now
      a vector.
    - `save_points_` was changed from a plain pointer to an `std::unique_ptr`,
      making ownership more explicit.
    Pull Request resolved: https://github.com/facebook/rocksdb/pull/5192
    
    Differential Revision: D15024074
    
    Pulled By: maysamyabandeh
    
    fbshipit-source-id: 5b128786d3789cde94e46465c9e91badd07a25d7
    de769094
write_batch.cc 66.6 KB