Skip to content

Commit e0b53ca

Browse files
uvdn7facebook-github-bot
authored andcommitted
Allow users to specifcy now
Summary: We are changing the underlying fb303 timeseries to have sub-second buckets. It requires a sub-second clock to reap the benefits. However, there is a small performance regression if we change `now()` when calling `addStatValue` for all ServiceData. This is because of this optimization in the MultilevelTimeseries https://fburl.com/code/904ox41z where it caches sum and count until the clock moves to the next tick. A more granular clock makes this optimization less useful. In this change, we allow users of ServiceData to specify `now` which defaults to a clock of second granularity to make this change perf neutral. #buildall Reviewed By: islamismailov Differential Revision: D82854336 fbshipit-source-id: 2c3d8ab2c0e91ac4c96ecbd54d7a574a6b6c7872
1 parent 186e60d commit e0b53ca

3 files changed

Lines changed: 36 additions & 23 deletions

File tree

fb303/BUCK

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,6 @@ cpp_library(
190190
srcs = ["ServiceData.cpp"],
191191
modular_headers = True,
192192
deps = [
193-
":legacy_clock",
194193
"//fb303/detail:regex_util",
195194
"//folly:conv",
196195
"//folly:indestructible",
@@ -202,6 +201,7 @@ cpp_library(
202201
":dynamic_counters",
203202
":exported_stat_map_impl",
204203
":histogram_exporter",
204+
":legacy_clock",
205205
"//fb303/detail:quantile_stat_map",
206206
"//folly:chrono",
207207
"//folly:optional",
@@ -212,8 +212,8 @@ cpp_library(
212212
"//folly/synchronization:relaxed_atomic",
213213
],
214214
external_deps = [
215-
"gflags",
216215
("boost", None, "boost_regex"),
216+
"gflags",
217217
],
218218
)
219219

fb303/ServiceData.cpp

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
#include <stdexcept>
2020

2121
#include <boost/regex.hpp>
22-
#include <fb303/LegacyClock.h>
2322
#include <fb303/detail/RegexUtil.h>
2423
#include <folly/Conv.h>
2524
#include <folly/Indestructible.h>
@@ -32,10 +31,6 @@ using folly::StringPiece;
3231

3332
namespace facebook::fb303 {
3433

35-
TimePoint get_current_time() {
36-
return TimePoint(std::chrono::seconds(get_legacy_stats_time()));
37-
}
38-
3934
template <typename T>
4035
static T& as_mutable(T const& t) {
4136
return const_cast<T&>(t);
@@ -156,29 +151,32 @@ void ServiceData::addStatExports(
156151
}
157152
}
158153

159-
void ServiceData::addStatValue(StringPiece key, int64_t value) {
160-
statsMap_.addValue(key, get_current_time(), value);
154+
void ServiceData::addStatValue(StringPiece key, int64_t value, TimePoint now) {
155+
statsMap_.addValue(key, now, value);
161156
}
162157

163158
void ServiceData::addStatValue(
164159
StringPiece key,
165160
int64_t value,
166-
ExportType exportType) {
167-
statsMap_.addValue(key, get_current_time(), value, exportType);
161+
ExportType exportType,
162+
TimePoint now) {
163+
statsMap_.addValue(key, now, value, exportType);
168164
}
169165

170166
void ServiceData::addStatValue(
171167
StringPiece key,
172168
int64_t value,
173-
folly::Range<const ExportType*> exportTypes) {
174-
statsMap_.addValue(key, get_current_time(), value, exportTypes);
169+
folly::Range<const ExportType*> exportTypes,
170+
TimePoint now) {
171+
statsMap_.addValue(key, now, value, exportTypes);
175172
}
176173

177174
void ServiceData::addStatValueAggregated(
178175
StringPiece key,
179176
int64_t sum,
180-
int64_t numSamples) {
181-
statsMap_.addValueAggregated(key, get_current_time(), sum, numSamples);
177+
int64_t numSamples,
178+
TimePoint now) {
179+
statsMap_.addValueAggregated(key, now, sum, numSamples);
182180
}
183181

184182
bool ServiceData::addHistogram(
@@ -267,8 +265,9 @@ void ServiceData::addHistogramValueMult(
267265
void ServiceData::addHistAndStatValue(
268266
StringPiece key,
269267
int64_t value,
270-
bool checkContains) {
271-
statsMap_.addValue(key, get_current_time(), value);
268+
bool checkContains,
269+
TimePoint now) {
270+
statsMap_.addValue(key, now, value);
272271

273272
if (!checkContains || histMap_.contains(key)) {
274273
histMap_.addValue(key, get_legacy_stats_time(), value);

fb303/ServiceData.h

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include <folly/container/RegexMatchCache.h>
3030
#include <folly/synchronization/RelaxedAtomic.h>
3131

32+
#include <fb303/LegacyClock.h>
3233
#include <atomic>
3334
#include <chrono>
3435
#include <cinttypes>
@@ -40,6 +41,10 @@
4041

4142
namespace facebook::fb303 {
4243

44+
inline TimePoint get_current_time() {
45+
return TimePoint(std::chrono::seconds(get_legacy_stats_time()));
46+
}
47+
4348
/**
4449
* ServiceData stores statistics and other information used by most
4550
* Facebook C++ services.
@@ -233,17 +238,25 @@ class ServiceData {
233238
* Note that the optional exportType parameter is only used if the
234239
* stat has not already been added.
235240
*/
236-
void addStatValue(folly::StringPiece key, int64_t value = 1);
237-
void
238-
addStatValue(folly::StringPiece key, int64_t value, ExportType exportType);
241+
void addStatValue(
242+
folly::StringPiece key,
243+
int64_t value = 1,
244+
TimePoint now = get_current_time());
245+
void addStatValue(
246+
folly::StringPiece key,
247+
int64_t value,
248+
ExportType exportType,
249+
TimePoint now = get_current_time());
239250
void addStatValue(
240251
folly::StringPiece key,
241252
int64_t value,
242-
folly::Range<const ExportType*> exportType);
253+
folly::Range<const ExportType*> exportType,
254+
TimePoint now = get_current_time());
243255
void addStatValueAggregated(
244256
folly::StringPiece key,
245257
int64_t sum,
246-
int64_t numSamples);
258+
int64_t numSamples,
259+
TimePoint now = get_current_time());
247260

248261
/**
249262
* Defines a histogram that can be used via calls to addHistogramValue() and
@@ -399,7 +412,8 @@ class ServiceData {
399412
[[deprecated]] void addHistAndStatValue(
400413
folly::StringPiece key,
401414
int64_t value,
402-
bool checkContains = false);
415+
bool checkContains = false,
416+
TimePoint now = get_current_time());
403417

404418
/**
405419
* Convenience function for adding the same value to stats and histograms.

0 commit comments

Comments
 (0)