Skip to content

Commit

Permalink
Track setting access count
Browse files Browse the repository at this point in the history
Summary:
Setting access information can be used to:
*  Flag unused settings and dead code paths that are eligible for cleanup
* Implement change safety tooling which can leverage this granular (per-setting) consumption information
* Figure out which settings are only accessed once during startup and should probably be marked `Immutable` instead of `Mutable`
* Figure out which settings are "hot"
* `DistributedCounter` benchmarks (could consider committing although they're somewhat redundant with SettingsBenchmarks) https://www.internalfb.com/intern/diff/view-version/248548896/

Reviewed By: yfeldblum

Differential Revision: D64188812

fbshipit-source-id: 3d5f25ce6334c6baf1cc7a7f2a44dc8f188e05f9
  • Loading branch information
Kevin Doherty authored and facebook-github-bot committed Oct 18, 2024
1 parent 02fb5ed commit 467af72
Show file tree
Hide file tree
Showing 6 changed files with 209 additions and 62 deletions.
1 change: 1 addition & 0 deletions folly/settings/BUCK
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ cpp_library(
"//folly:shared_mutex",
"//folly:thread_local",
"//folly:utility",
"//folly/concurrency:singleton_relaxed_counter",
"//folly/container:f14_hash",
"//folly/lang:aligned",
],
Expand Down
92 changes: 58 additions & 34 deletions folly/settings/Settings.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include <folly/Function.h>
#include <folly/Likely.h>
#include <folly/Range.h>
#include <folly/concurrency/SingletonRelaxedCounter.h>
#include <folly/settings/Types.h>
#include <folly/settings/detail/SettingsImpl.h>

Expand All @@ -35,10 +36,12 @@ namespace detail {
* @param TrivialPtr location of the small type storage. Optimization
* for better inlining.
*/
template <class T, std::atomic<uint64_t>* TrivialPtr>
template <class T, std::atomic<uint64_t>* TrivialPtr, typename Tag>
class SettingWrapper {
using AccessCounter = typename SettingCore<T, Tag>::AccessCounter;

public:
using CallbackHandle = typename SettingCore<T>::CallbackHandle;
using CallbackHandle = typename SettingCore<T, Tag>::CallbackHandle;

/**
* Returns the setting's current value.
Expand All @@ -50,9 +53,13 @@ class SettingWrapper {
* here after some amount of time (on the order of minutes).
*/
std::conditional_t<IsSmallPOD<T>, T, const T&> operator*() const {
AccessCounter::add(1);
return core_.getWithHint(*TrivialPtr);
}
const T* operator->() const { return &core_.getSlow().value; }
const T* operator->() const {
AccessCounter::add(1);
return &core_.getSlow().value;
}

/**
* Returns the setting's current value. Equivalent to dereference operator
Expand Down Expand Up @@ -94,7 +101,8 @@ class SettingWrapper {
* @returns a handle object which automatically removes the callback from
* processing once destroyd
*/
CallbackHandle addCallback(typename SettingCore<T>::UpdateCallback callback) {
CallbackHandle addCallback(
typename SettingCore<T, Tag>::UpdateCallback callback) {
return core_.addCallback(std::move(callback));
}

Expand All @@ -111,15 +119,20 @@ class SettingWrapper {
*/
StringPiece updateReason() const { return core_.getSlow().updateReason; }

/**
* Returns the number of times this setting has been accessed.
*/
uint64_t accessCount() const { return AccessCounter::count(); }

/**
* Returns the setting's update reason in the snapshot.
*/
StringPiece updateReason(const Snapshot& snapshot) const;

explicit SettingWrapper(SettingCore<T>& core) : core_(core) {}
explicit SettingWrapper(SettingCore<T, Tag>& core) : core_(core) {}

private:
SettingCore<T>& core_;
SettingCore<T, Tag>& core_;
friend class folly::settings::Snapshot;
};

Expand All @@ -140,11 +153,15 @@ class SettingWrapper {
*/
#define FOLLY_DETAIL_SETTINGS_DEFINE_LOCAL_FUNC__( \
_project, _name, _Type, _overloadType) \
extern ::std::atomic<::folly::settings::detail::SettingCore<_Type>*> \
struct FOLLY_SETTINGS_TAG__##_project##_##_name; \
extern ::std::atomic<::folly::settings::detail::SettingCore< \
_Type, \
FOLLY_SETTINGS_TAG__##_project##_##_name>*> \
FOLLY_SETTINGS_CACHE__##_project##_##_name; \
extern ::std::atomic<uint64_t> FOLLY_SETTINGS_TRIVIAL__##_project##_##_name; \
::folly::settings::detail::SettingCore<_Type>& \
FOLLY_SETTINGS_FUNC__##_project##_##_name(); \
::folly::settings::detail:: \
SettingCore<_Type, FOLLY_SETTINGS_TAG__##_project##_##_name>& \
FOLLY_SETTINGS_FUNC__##_project##_##_name(); \
FOLLY_ALWAYS_INLINE auto FOLLY_SETTINGS_LOCAL_FUNC__##_project##_##_name( \
_overloadType) { \
auto* folly_detail_settings_value = \
Expand All @@ -156,13 +173,16 @@ class SettingWrapper {
FOLLY_SETTINGS_CACHE__##_project##_##_name.store( \
folly_detail_settings_value, ::std::memory_order_release); \
} \
return ::folly::settings::detail:: \
SettingWrapper<_Type, &FOLLY_SETTINGS_TRIVIAL__##_project##_##_name>( \
*folly_detail_settings_value); \
return ::folly::settings::detail::SettingWrapper< \
_Type, \
&FOLLY_SETTINGS_TRIVIAL__##_project##_##_name, \
FOLLY_SETTINGS_TAG__##_project##_##_name>( \
*folly_detail_settings_value); \
} \
/* This is here just to force a semicolon */ \
::folly::settings::detail::SettingCore<_Type>& \
FOLLY_SETTINGS_FUNC__##_project##_##_name()
::folly::settings::detail:: \
SettingCore<_Type, FOLLY_SETTINGS_TAG__##_project##_##_name>& \
FOLLY_SETTINGS_FUNC__##_project##_##_name()

} // namespace detail

Expand Down Expand Up @@ -193,11 +213,13 @@ class SettingWrapper {
* @param _desc setting documentation
*/
#define FOLLY_SETTING_DEFINE(_project, _name, _Type, _def, _mut, _cli, _desc) \
struct FOLLY_SETTINGS_TAG__##_project##_##_name {}; \
/* Fastpath optimization, see notes in FOLLY_SETTINGS_DEFINE_LOCAL_FUNC__. \
Aggregate all off these together in a single section for better TLB \
and cache locality. */ \
__attribute__((__section__(".folly.settings.cache"))) ::std::atomic< \
::folly::settings::detail::SettingCore<_Type>*> \
::folly::settings::detail:: \
SettingCore<_Type, FOLLY_SETTINGS_TAG__##_project##_##_name>*> \
FOLLY_SETTINGS_CACHE__##_project##_##_name; \
/* Location for the small value cache (if _Type is small and trivial). \
Intentionally located right after the pointer cache above to take \
Expand All @@ -206,10 +228,12 @@ class SettingWrapper {
__section__(".folly.settings.cache"))) ::std::atomic<uint64_t> \
FOLLY_SETTINGS_TRIVIAL__##_project##_##_name; \
/* Meyers singleton to avoid SIOF */ \
FOLLY_NOINLINE ::folly::settings::detail::SettingCore<_Type>& \
FOLLY_SETTINGS_FUNC__##_project##_##_name() { \
static ::folly::Indestructible< \
::folly::settings::detail::SettingCore<_Type>> \
FOLLY_NOINLINE ::folly::settings::detail:: \
SettingCore<_Type, FOLLY_SETTINGS_TAG__##_project##_##_name>& \
FOLLY_SETTINGS_FUNC__##_project##_##_name() { \
static ::folly::Indestructible<::folly::settings::detail::SettingCore< \
_Type, \
FOLLY_SETTINGS_TAG__##_project##_##_name>> \
setting( \
::folly::settings::SettingMetadata{ \
#_project, \
Expand Down Expand Up @@ -264,7 +288,7 @@ namespace detail {
* Like SettingWrapper, but checks against any values saved/updated in a
* snapshot.
*/
template <class T>
template <class T, typename Tag>
class SnapshotSettingWrapper {
public:
/**
Expand All @@ -286,10 +310,10 @@ class SnapshotSettingWrapper {

private:
Snapshot& snapshot_;
SettingCore<T>& core_;
SettingCore<T, Tag>& core_;
friend class folly::settings::Snapshot;

SnapshotSettingWrapper(Snapshot& snapshot, SettingCore<T>& core)
SnapshotSettingWrapper(Snapshot& snapshot, SettingCore<T, Tag>& core)
: snapshot_(snapshot), core_(core) {}
};

Expand Down Expand Up @@ -328,10 +352,10 @@ class Snapshot final : public detail::SnapshotBase {
/**
* Wraps a global FOLLY_SETTING(a, b) and returns a snapshot-local wrapper.
*/
template <class T, std::atomic<uint64_t>* P>
detail::SnapshotSettingWrapper<T> operator()(
detail::SettingWrapper<T, P>&& setting) {
return detail::SnapshotSettingWrapper<T>(*this, setting.core_);
template <class T, std::atomic<uint64_t>* P, typename Tag>
detail::SnapshotSettingWrapper<T, Tag> operator()(
detail::SettingWrapper<T, P, Tag>&& setting) {
return detail::SnapshotSettingWrapper<T, Tag>(*this, setting.core_);
}

/**
Expand Down Expand Up @@ -397,27 +421,27 @@ class Snapshot final : public detail::SnapshotBase {
FunctionRef<void(const SettingVisitorInfo&)> func) const override;

private:
template <typename T>
template <typename T, typename Tag>
friend class detail::SnapshotSettingWrapper;

template <typename T, std::atomic<uint64_t>* TrivialPtr>
template <typename T, std::atomic<uint64_t>* TrivialPtr, typename Tag>
friend class detail::SettingWrapper;
};

namespace detail {
template <class T>
inline const T& SnapshotSettingWrapper<T>::operator*() const {
template <class T, typename Tag>
inline const T& SnapshotSettingWrapper<T, Tag>::operator*() const {
return snapshot_.get(core_).value;
}

template <class T, std::atomic<uint64_t>* TrivialPtr>
template <class T, std::atomic<uint64_t>* TrivialPtr, typename Tag>
inline std::conditional_t<IsSmallPOD<T>, T, const T&>
SettingWrapper<T, TrivialPtr>::value(const Snapshot& snapshot) const {
SettingWrapper<T, TrivialPtr, Tag>::value(const Snapshot& snapshot) const {
return snapshot.get(core_).value;
}

template <class T, std::atomic<uint64_t>* TrivialPtr>
StringPiece SettingWrapper<T, TrivialPtr>::updateReason(
template <class T, std::atomic<uint64_t>* TrivialPtr, typename Tag>
StringPiece SettingWrapper<T, TrivialPtr, Tag>::updateReason(
const Snapshot& snapshot) const {
return snapshot.get(core_).updateReason;
}
Expand Down
34 changes: 20 additions & 14 deletions folly/settings/detail/SettingsImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include <folly/SharedMutex.h>
#include <folly/ThreadLocal.h>
#include <folly/Utility.h>
#include <folly/concurrency/SingletonRelaxedCounter.h>
#include <folly/container/F14Set.h>
#include <folly/lang/Aligned.h>
#include <folly/settings/Immutables.h>
Expand Down Expand Up @@ -72,6 +73,7 @@ class SettingCoreBase {
virtual SetResult resetToDefault(SnapshotBase* snapshot) = 0;
virtual void forceResetToDefault(SnapshotBase* snapshot) = 0;
virtual const SettingMetadata& meta() const = 0;
virtual uint64_t accessCount() const = 0;
virtual ~SettingCoreBase() {}

/**
Expand All @@ -87,7 +89,7 @@ void registerSetting(SettingCoreBase& core);
*/
SettingCoreBase::Version nextGlobalVersion();

template <class T>
template <class T, typename Tag>
class SettingCore;

/**
Expand All @@ -108,11 +110,11 @@ class BoxedValue {
* Stores a value that can be both retrieved later and optionally
* applied globally
*/
template <class T>
BoxedValue(const T& value, StringPiece reason, SettingCore<T>& core)
template <class T, typename Tag>
BoxedValue(const T& value, StringPiece reason, SettingCore<T, Tag>& core)
: value_(std::make_shared<SettingContents<T>>(reason.str(), value)),
core_{&core},
publish_{doPublish<T>} {}
publish_{doPublish<T, Tag>} {}

/**
* Returns the reference to the stored value
Expand All @@ -134,10 +136,10 @@ class BoxedValue {
private:
using PublishFun = void(BoxedValue&, const FrozenSettingProjects&);

template <typename T>
template <typename T, typename Tag>
static void doPublish(
BoxedValue& boxed, const FrozenSettingProjects& frozenProjects) {
auto& core = *static_cast<SettingCore<T>*>(boxed.core_);
auto& core = *static_cast<SettingCore<T, Tag>*>(boxed.core_);
if (core.meta().mutability == Mutability::Immutable &&
frozenProjects.contains(core.meta().project)) {
return;
Expand Down Expand Up @@ -189,6 +191,7 @@ class SnapshotBase {
const SettingMetadata& meta() const { return core_.meta(); }
std::pair<std::string, std::string> valueAndReason() const;
const std::string& fullName() const { return fullName_; }
uint64_t accessCount() const { return core_.accessCount(); }

private:
const std::string& fullName_;
Expand Down Expand Up @@ -254,7 +257,7 @@ class SnapshotBase {
std::unordered_map<detail::SettingCoreBase::Key, detail::BoxedValue>
snapshotValues_;

template <typename T>
template <typename T, typename Tag>
friend class SettingCore;

SnapshotBase();
Expand All @@ -264,8 +267,8 @@ class SnapshotBase {
SnapshotBase(SnapshotBase&&) = delete;
SnapshotBase& operator=(SnapshotBase&&) = delete;

template <class T>
const SettingContents<T>& get(const detail::SettingCore<T>& core) const {
template <class T, typename Tag>
const SettingContents<T>& get(const detail::SettingCore<T, Tag>& core) const {
auto it = snapshotValues_.find(core.getKey());
if (it != snapshotValues_.end()) {
return it->second.template unbox<T>();
Expand All @@ -277,16 +280,17 @@ class SnapshotBase {
return core.getSlow();
}

template <class T>
void set(detail::SettingCore<T>& core, const T& t, StringPiece reason) {
template <class T, typename Tag>
void set(detail::SettingCore<T, Tag>& core, const T& t, StringPiece reason) {
snapshotValues_[core.getKey()] = detail::BoxedValue(t, reason, core);
}
};

template <class T>
template <class T, typename Tag>
class SettingCore : public SettingCoreBase {
public:
using Contents = SettingContents<T>;
using AccessCounter = SingletonRelaxedCounter<uint64_t, Tag>;

SetResult setFromString(
StringPiece newValue,
Expand Down Expand Up @@ -329,6 +333,8 @@ class SettingCore : public SettingCoreBase {

const SettingMetadata& meta() const override { return meta_; }

uint64_t accessCount() const override { return AccessCounter::count(); }

/**
* @param trivialStorage must refer to the same location
* as the internal trivialStorage_. This hint will
Expand Down Expand Up @@ -363,7 +369,7 @@ class SettingCore : public SettingCoreBase {
class CallbackHandle {
public:
CallbackHandle(
std::shared_ptr<UpdateCallback> callback, SettingCore<T>& setting)
std::shared_ptr<UpdateCallback> callback, SettingCore<T, Tag>& setting)
: callback_(std::move(callback)), setting_(setting) {}
~CallbackHandle() {
if (callback_) {
Expand All @@ -378,7 +384,7 @@ class SettingCore : public SettingCoreBase {

private:
std::shared_ptr<UpdateCallback> callback_;
SettingCore<T>& setting_;
SettingCore<T, Tag>& setting_;
};
CallbackHandle addCallback(UpdateCallback callback) {
auto callbackPtr = copy_to_shared_ptr(std::move(callback));
Expand Down
6 changes: 4 additions & 2 deletions folly/settings/test/BUCK
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,15 @@ cpp_unittest(
"SettingsTest.cpp",
],
deps = [
"fbsource//third-party/fmt:fmt",
":a",
":b",
"//folly:format",
"//folly:string",
"//folly/portability:gmock",
"//folly/portability:gtest",
"//folly/settings:settings",
"//folly/settings/test:a",
"//folly/settings/test:b",
"//folly/synchronization/test:barrier",
],
)

Expand Down
Loading

0 comments on commit 467af72

Please sign in to comment.