Skip to content

Commit

Permalink
revise ThreadCachedServiceData per-key export caching
Browse files Browse the repository at this point in the history
Summary:
Cut `ThreadCachedServiceData::ExportCacheMap` etc. It's an extra map that every call to `ThreadCachedServiceData::addStatValue` has to traverse, and the map lookup to find the export status can be just as costly as the map lookup to find the stat.

Instead, just cache the export status in the thread-local stat-map value.

Some aspects of the `ExportType` overloads of `addStatValue` and `clearStat` have seemingly strange behavior. But, for now, we attempt to preserve the current behavior.

Differential Revision: D68659181

fbshipit-source-id: f9028696f9007d0e2bafeddd35551f1663ff6149
  • Loading branch information
yfeldblum authored and facebook-github-bot committed Jan 25, 2025
1 parent 283e843 commit df955b8
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 58 deletions.
13 changes: 2 additions & 11 deletions fb303/ThreadCachedServiceData.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -259,22 +259,13 @@ void ThreadCachedServiceData::addHistAndStatValues(
void ThreadCachedServiceData::clearStat(
folly::StringPiece key,
ExportType exportType) {
(*keyCacheTable_)[exportType].erase(key);
ServiceData::get()->addStatExportType(key, exportType, nullptr);
getThreadStats()->clearStat(key, exportType);
}

void ThreadCachedServiceData::addStatValue(
folly::StringPiece key,
int64_t value,
ExportType exportType) {
if (UNLIKELY(!(*keyCacheTable_)[exportType].has(key))) {
// This is not present in the threadlocal export set; possible it was
// not yet registered with the underlying ServiceData impl.
// This time around, pass it to the ServiceData so the type is exported
getServiceData()->addStatExportType(key, exportType);
(*keyCacheTable_)[exportType].add(key);
}
// now we know the export was done; finally bump the counter
addStatValue(key, value);
getThreadStats()->addStatValue(key, value, exportType);
}
} // namespace facebook::fb303
25 changes: 0 additions & 25 deletions fb303/ThreadCachedServiceData.h
Original file line number Diff line number Diff line change
Expand Up @@ -135,28 +135,6 @@ class ThreadCachedServiceData {
getThreadStats()->addStatValue(key, value);
}

/** Small LRU lookup set class, for exported keys. */
struct ExportKeyCache
: public SimpleLRUMap<std::string, bool, folly::F14FastMap> {
/**
* It would be best if this can be overridden by gflag; but it's often the
* case that the stats classes are used pre-startup / post-shutdown and thus
* trying to access gflags might stir up an initialization/shutdown order
* fiasco. So this constant is hardcoded here.
*/
static constexpr size_t kLRUMaxSize = 1000;

ExportKeyCache() : SimpleLRUMap(kLRUMaxSize) {}

void add(folly::StringPiece key) {
set(key, true);
}

bool has(folly::StringPiece key) {
return find(key, /*moveToFront*/ true) != end();
}
};

void
addStatValue(folly::StringPiece key, int64_t value, ExportType exportType);
void clearStat(folly::StringPiece key, ExportType exportType);
Expand Down Expand Up @@ -361,9 +339,6 @@ class ThreadCachedServiceData {

ServiceData* serviceData_;
StatsThreadLocal* threadLocalStats_;
using KeyCacheTable =
std::array<ExportKeyCache, ExportTypeMeta::kNumExportTypes>;
folly::ThreadLocal<KeyCacheTable> keyCacheTable_;

std::atomic<std::chrono::milliseconds> interval_{
std::chrono::milliseconds(0)};
Expand Down
81 changes: 60 additions & 21 deletions fb303/ThreadLocalStatsMap-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,27 @@ void ThreadLocalStatsMapT<LockTraits>::addStatValueAggregated(
getTimeseriesLocked(*state, name)->addValueAggregated(sum, numSamples);
}

template <class LockTraits>
void ThreadLocalStatsMapT<LockTraits>::addStatValue(
folly::StringPiece name,
int64_t value,
ExportType exportType) {
auto state = state_.lock();
getTimeseriesLocked(*state, name, exportType)->addValue(value);
}

template <class LockTraits>
void ThreadLocalStatsMapT<LockTraits>::clearStat(
folly::StringPiece name,
ExportType exportType) {
auto state = state_.lock();
if (auto ptr = folly::get_ptr(state->namedTimeseries_, name)) {
auto mask = uintptr_t(1) << size_t(exportType);
ptr->exports = ptr->exports & ~mask;
}
this->getServiceData()->addStatExportType(name, exportType, nullptr);
}

template <class LockTraits>
void ThreadLocalStatsMapT<LockTraits>::addHistogramValue(
folly::StringPiece name,
Expand All @@ -64,10 +85,10 @@ std::shared_ptr<typename ThreadLocalStatsMapT<LockTraits>::TLTimeseries>
ThreadLocalStatsMapT<LockTraits>::getTimeseriesSafe(folly::StringPiece name) {
auto state = state_.lock();
auto& entry = state->namedTimeseries_[name];
if (!entry) {
entry = std::make_shared<TLTimeseries>(this, name);
if (!entry.ptr) {
entry.ptr = std::make_shared<TLTimeseries>(this, name);
}
return entry;
return entry.ptr;
}

template <class LockTraits>
Expand All @@ -79,21 +100,39 @@ ThreadLocalStatsMapT<LockTraits>::getTimeseriesSafe(
const int levelDurations[]) {
auto state = state_.lock();
auto& entry = state->namedTimeseries_[name];
if (!entry) {
entry = std::make_shared<TLTimeseries>(
if (!entry.ptr) {
entry.ptr = std::make_shared<TLTimeseries>(
this, name, numBuckets, numLevels, levelDurations);
}
return entry;
return entry.ptr;
}

template <class LockTraits>
typename ThreadLocalStatsMapT<LockTraits>::TLTimeseries* ThreadLocalStatsMapT<
LockTraits>::getTimeseriesLocked(State& state, folly::StringPiece name) {
auto& entry = state.namedTimeseries_[name];
if (!entry) {
entry = std::make_shared<TLTimeseries>(this, name);
if (!entry.ptr) {
entry.ptr = std::make_shared<TLTimeseries>(this, name);
}
return entry.ptr.get();
}

template <class LockTraits>
typename ThreadLocalStatsMapT<LockTraits>::TLTimeseries*
ThreadLocalStatsMapT<LockTraits>::getTimeseriesLocked(
State& state,
folly::StringPiece name,
ExportType exportType) {
auto& entry = state.namedTimeseries_[name];
if (!entry.ptr) {
entry.ptr = std::make_shared<TLTimeseries>(this, name);
}
auto mask = uintptr_t(1) << size_t(exportType);
if (!(entry.exports & mask)) {
this->getServiceData()->addStatExportType(name, exportType);
entry.exports = entry.exports | mask;
}
return entry.get();
return entry.ptr.get();
}

template <class LockTraits>
Expand All @@ -113,11 +152,11 @@ template <class LockTraits>
typename ThreadLocalStatsMapT<LockTraits>::TLHistogram* ThreadLocalStatsMapT<
LockTraits>::getHistogramLockedPtr(State& state, folly::StringPiece name) {
auto& entry = state.namedHistograms_[name];
if (!entry) {
entry = this->createHistogramLocked(state, name);
if (!entry.ptr) {
entry.ptr = this->createHistogramLocked(state, name);
}

return entry.get();
return entry.ptr.get();
}

template <class LockTraits>
Expand Down Expand Up @@ -146,32 +185,32 @@ ThreadLocalStatsMapT<LockTraits>::getHistogramLocked(
State& state,
folly::StringPiece name) {
auto& entry = state.namedHistograms_[name];
if (!entry) {
entry = this->createHistogramLocked(state, name);
if (!entry.ptr) {
entry.ptr = this->createHistogramLocked(state, name);
}

return entry;
return entry.ptr;
}

template <class LockTraits>
std::shared_ptr<typename ThreadLocalStatsMapT<LockTraits>::TLCounter>
ThreadLocalStatsMapT<LockTraits>::getCounterSafe(folly::StringPiece name) {
auto state = state_.lock();
auto& entry = state->namedCounters_[name];
if (!entry) {
entry = std::make_shared<TLCounter>(this, name);
if (!entry.ptr) {
entry.ptr = std::make_shared<TLCounter>(this, name);
}
return entry;
return entry.ptr;
}

template <class LockTraits>
typename ThreadLocalStatsMapT<LockTraits>::TLCounter* ThreadLocalStatsMapT<
LockTraits>::getCounterLocked(State& state, folly::StringPiece name) {
auto& entry = state.namedCounters_[name];
if (!entry) {
entry = std::make_shared<TLCounter>(this, name);
if (!entry.ptr) {
entry.ptr = std::make_shared<TLCounter>(this, name);
}
return entry.get();
return entry.ptr.get();
}

template <class LockTraits>
Expand Down
16 changes: 15 additions & 1 deletion fb303/ThreadLocalStatsMap.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@ class ThreadLocalStatsMapT : public ThreadLocalStatsT<LockTraits> {
int64_t sum,
int64_t numSamples);

void
addStatValue(folly::StringPiece name, int64_t value, ExportType exportType);
void clearStat(folly::StringPiece name, ExportType exportType);

/**
* Add a value to the histogram with the specified name.
*
Expand Down Expand Up @@ -124,7 +128,13 @@ class ThreadLocalStatsMapT : public ThreadLocalStatsT<LockTraits> {
using NamedMapLock = typename TLStatsNoLocking::RegistryLock;

template <class StatType>
using StatMap = folly::F14FastMap<std::string, std::shared_ptr<StatType>>;
struct StatPtr {
std::shared_ptr<StatType> ptr;
uintptr_t exports{};
};

template <class StatType>
using StatMap = folly::F14FastMap<std::string, StatPtr<StatType>>;

struct State;

Expand All @@ -136,6 +146,10 @@ class ThreadLocalStatsMapT : public ThreadLocalStatsT<LockTraits> {
* Never returns NULL.
*/
TLTimeseries* getTimeseriesLocked(State& state, folly::StringPiece name);
TLTimeseries* getTimeseriesLocked(
State& state,
folly::StringPiece name,
ExportType exportType);

/*
* Get the TLHistogram with the given name.
Expand Down

0 comments on commit df955b8

Please sign in to comment.