diff --git a/fb303/ThreadCachedServiceData.cpp b/fb303/ThreadCachedServiceData.cpp index dc01785e4..79b624d3f 100644 --- a/fb303/ThreadCachedServiceData.cpp +++ b/fb303/ThreadCachedServiceData.cpp @@ -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 diff --git a/fb303/ThreadCachedServiceData.h b/fb303/ThreadCachedServiceData.h index 49ec05be0..538ac0dd2 100644 --- a/fb303/ThreadCachedServiceData.h +++ b/fb303/ThreadCachedServiceData.h @@ -135,28 +135,6 @@ class ThreadCachedServiceData { getThreadStats()->addStatValue(key, value); } - /** Small LRU lookup set class, for exported keys. */ - struct ExportKeyCache - : public SimpleLRUMap { - /** - * 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); @@ -361,9 +339,6 @@ class ThreadCachedServiceData { ServiceData* serviceData_; StatsThreadLocal* threadLocalStats_; - using KeyCacheTable = - std::array; - folly::ThreadLocal keyCacheTable_; std::atomic interval_{ std::chrono::milliseconds(0)}; diff --git a/fb303/ThreadLocalStatsMap-inl.h b/fb303/ThreadLocalStatsMap-inl.h index 9828b49ea..ec6264e6b 100644 --- a/fb303/ThreadLocalStatsMap-inl.h +++ b/fb303/ThreadLocalStatsMap-inl.h @@ -40,6 +40,27 @@ void ThreadLocalStatsMapT::addStatValueAggregated( getTimeseriesLocked(*state, name)->addValueAggregated(sum, numSamples); } +template +void ThreadLocalStatsMapT::addStatValue( + folly::StringPiece name, + int64_t value, + ExportType exportType) { + auto state = state_.lock(); + getTimeseriesLocked(*state, name, exportType)->addValue(value); +} + +template +void ThreadLocalStatsMapT::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 void ThreadLocalStatsMapT::addHistogramValue( folly::StringPiece name, @@ -64,10 +85,10 @@ std::shared_ptr::TLTimeseries> ThreadLocalStatsMapT::getTimeseriesSafe(folly::StringPiece name) { auto state = state_.lock(); auto& entry = state->namedTimeseries_[name]; - if (!entry) { - entry = std::make_shared(this, name); + if (!entry.ptr) { + entry.ptr = std::make_shared(this, name); } - return entry; + return entry.ptr; } template @@ -79,21 +100,39 @@ ThreadLocalStatsMapT::getTimeseriesSafe( const int levelDurations[]) { auto state = state_.lock(); auto& entry = state->namedTimeseries_[name]; - if (!entry) { - entry = std::make_shared( + if (!entry.ptr) { + entry.ptr = std::make_shared( this, name, numBuckets, numLevels, levelDurations); } - return entry; + return entry.ptr; } template typename ThreadLocalStatsMapT::TLTimeseries* ThreadLocalStatsMapT< LockTraits>::getTimeseriesLocked(State& state, folly::StringPiece name) { auto& entry = state.namedTimeseries_[name]; - if (!entry) { - entry = std::make_shared(this, name); + if (!entry.ptr) { + entry.ptr = std::make_shared(this, name); + } + return entry.ptr.get(); +} + +template +typename ThreadLocalStatsMapT::TLTimeseries* +ThreadLocalStatsMapT::getTimeseriesLocked( + State& state, + folly::StringPiece name, + ExportType exportType) { + auto& entry = state.namedTimeseries_[name]; + if (!entry.ptr) { + entry.ptr = std::make_shared(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 @@ -113,11 +152,11 @@ template typename ThreadLocalStatsMapT::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 @@ -146,11 +185,11 @@ ThreadLocalStatsMapT::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 @@ -158,20 +197,20 @@ std::shared_ptr::TLCounter> ThreadLocalStatsMapT::getCounterSafe(folly::StringPiece name) { auto state = state_.lock(); auto& entry = state->namedCounters_[name]; - if (!entry) { - entry = std::make_shared(this, name); + if (!entry.ptr) { + entry.ptr = std::make_shared(this, name); } - return entry; + return entry.ptr; } template typename ThreadLocalStatsMapT::TLCounter* ThreadLocalStatsMapT< LockTraits>::getCounterLocked(State& state, folly::StringPiece name) { auto& entry = state.namedCounters_[name]; - if (!entry) { - entry = std::make_shared(this, name); + if (!entry.ptr) { + entry.ptr = std::make_shared(this, name); } - return entry.get(); + return entry.ptr.get(); } template diff --git a/fb303/ThreadLocalStatsMap.h b/fb303/ThreadLocalStatsMap.h index 9bd826e18..80dabb69e 100644 --- a/fb303/ThreadLocalStatsMap.h +++ b/fb303/ThreadLocalStatsMap.h @@ -65,6 +65,10 @@ class ThreadLocalStatsMapT : public ThreadLocalStatsT { 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. * @@ -124,7 +128,13 @@ class ThreadLocalStatsMapT : public ThreadLocalStatsT { using NamedMapLock = typename TLStatsNoLocking::RegistryLock; template - using StatMap = folly::F14FastMap>; + struct StatPtr { + std::shared_ptr ptr; + uintptr_t exports{}; + }; + + template + using StatMap = folly::F14FastMap>; struct State; @@ -136,6 +146,10 @@ class ThreadLocalStatsMapT : public ThreadLocalStatsT { * 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.