Skip to content

Commit

Permalink
maint: Remove use of folly/ThreadCachedInt (#1486)
Browse files Browse the repository at this point in the history
#### Reference Issues/PRs

Fixes the 10th item of the folly replacement plan, issue #1412.

#### What does this implement or fix?

This removes the single use of `folly/ThreadCachedInt`. It is replaced
by a partial vendoring of the `folly` code plus use of
`boost::thread_specific_ptr`.

`ThreadCachedInt` is used to count the number of freed memory blocks. It
is (presumably) not just implemented as an atomic integer count as
thread locking would be too slow, so instead each thread has its own
count and when a single thread's count exceeds some threshold it is
added to the overall count. The original `folly` implementation has two
ways of reading the count which are slow (fully accurate) and fast (not
fully accurate). ArcticDB only uses the fast option, so the
implementation is much simpler than `folly`'s, requiring fewer
`atomic`s.

New class `ThreadCachedInt` in `thread_cached_int.hpp` is derived from
https://github.com/facebook/folly/blob/main/folly/ThreadCachedInt.h but
containing only the subset of functionality required. Instead of using
`folly'`s own `ThreadLocalPtr` this uses `boost::thread_specific_ptr`.
The `folly` source code claims that their implementation is 4x faster
than the `boost` one:


https://github.com/facebook/folly/blob/dbc9e565f54eabb40ad6600656ad9dea919f51c0/folly/ThreadLocal.h#L18-L20

but that claim dates from 12 years ago and this solution is simpler than
theirs. This does need to be benchmarked against `master` to confirm
that it is not measurably slower.

#### Any other comments?

The only place this is ultimately used is to control when `malloc_trim`
is called here


https://github.com/man-group/ArcticDB/blob/23b3b943b7c4a10889a563a063b2a616fe00d9fa/cpp/arcticdb/util/allocator.cpp#L286-L288

to release memory back to the system. This only occurs on Linux. Other
OSes could have all of this code removed but this would be a bigger
change with many `#ifdef` blocks, etc.

---------

Signed-off-by: Ian Thomas <[email protected]>
  • Loading branch information
ianthomas23 authored May 14, 2024
1 parent 89fc8bb commit 9a4fadc
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 2 deletions.
1 change: 1 addition & 0 deletions cpp/arcticdb/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,7 @@ set(arcticdb_srcs
util/sparse_utils.hpp
util/storage_lock.hpp
util/string_utils.hpp
util/thread_cached_int.hpp
util/timeouts.hpp
util/timer.hpp
util/trace.hpp
Expand Down
4 changes: 2 additions & 2 deletions cpp/arcticdb/util/allocator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
#include <arcticdb/util/memory_tracing.hpp>
#include <arcticdb/util/clock.hpp>
#include <arcticdb/util/configs_map.hpp>
#include <arcticdb/util/thread_cached_int.hpp>
#include <folly/concurrency/ConcurrentHashMap.h>
#include <folly/ThreadCachedInt.h>

#include <fmt/std.h>

Expand Down Expand Up @@ -152,7 +152,7 @@ namespace arcticdb {
namespace {
template<typename TracingPolicy, typename ClockType>
auto& free_count_of(){
static folly::ThreadCachedInt<uint32_t> free_count;
static ThreadCachedInt<uint32_t> free_count;
return free_count;
};
}
Expand Down
78 changes: 78 additions & 0 deletions cpp/arcticdb/util/thread_cached_int.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
/* Copyright 2023 Man Group Operations Limited
*
* Use of this software is governed by the Business Source License 1.1 included in the file licenses/BSL.txt.
*
* As of the Change Date specified in that file, in accordance with the Business Source License, use of this software will be governed by the Apache License, version 2.0.
*
* The code in this file is derived from https://github.com/facebook/folly/blob/main/folly/ThreadCachedInt.h under the Apache 2.0 license which is available in full at https://github.com/facebook/folly/blob/main/LICENSE.
*/

#pragma once

#include <atomic>
#include <boost/thread/tss.hpp>

namespace arcticdb {

template <class IntT>
class ThreadCachedInt {
public:
explicit ThreadCachedInt(IntT initialVal = 0, uint32_t cacheSize = 1000)
: target_(initialVal), cacheSize_(cacheSize) {}

ThreadCachedInt(const ThreadCachedInt&) = delete;
ThreadCachedInt& operator=(const ThreadCachedInt&) = delete;

void increment(IntT inc) {
auto cache = cache_.get();
if (cache == nullptr) {
cache = new IntCache(*this);
cache_.reset(cache);
}
cache->increment(inc);
}

// Quickly grabs the current value which may not include some cached increments.
IntT readFast() const {
return target_.load(std::memory_order_relaxed);
}

// Quickly reads and resets current value (doesn't reset cached increments).
IntT readFastAndReset() {
return target_.exchange(0, std::memory_order_release);
}

private:
struct IntCache;

std::atomic<IntT> target_;
std::atomic<uint32_t> cacheSize_;
boost::thread_specific_ptr<IntCache> cache_; // Must be last for dtor ordering

struct IntCache {
ThreadCachedInt* parent_;
mutable IntT val_;
mutable uint32_t numUpdates_;

explicit IntCache(ThreadCachedInt& parent)
: parent_(&parent), val_(0), numUpdates_(0) {}

void increment(IntT inc) {
val_ += inc;
++numUpdates_;
if (numUpdates_ > parent_->cacheSize_.load(std::memory_order_acquire)) {
flush();
}
}

void flush() const {
parent_->target_.fetch_add(val_, std::memory_order_release);
val_ = 0;
numUpdates_ = 0;
}

~IntCache() { flush(); }
};
};

} //namespace arcticdb

0 comments on commit 9a4fadc

Please sign in to comment.