diff --git a/fdbserver/workloads/KVStoreTest.actor.cpp b/fdbserver/workloads/KVStoreTest.actor.cpp index 0aa34fdff3..bdaef4d6d2 100755 --- a/fdbserver/workloads/KVStoreTest.actor.cpp +++ b/fdbserver/workloads/KVStoreTest.actor.cpp @@ -153,7 +153,7 @@ struct KVTest { } }; -ACTOR Future testKVRead( KVTest* test, Key key, TestHistogram* latency, PerfIntCounter* count ) { +ACTOR Future testKVRead(KVTest* test, Key key, TestHistogram* latency, PerfIntCounter* count) { //state Version s1 = test->lastCommit; state Version s2 = test->lastDurable; @@ -171,7 +171,7 @@ ACTOR Future testKVRead( KVTest* test, Key key, TestHistogram* late return Void(); } -ACTOR Future testKVReadSaturation( KVTest* test, TestHistogram* latency, PerfIntCounter* count ) { +ACTOR Future testKVReadSaturation(KVTest* test, TestHistogram* latency, PerfIntCounter* count) { while (true) { state double begin = timer(); Optional val = wait( test->store->readValue(test->randomKey()) ); @@ -181,7 +181,7 @@ ACTOR Future testKVReadSaturation( KVTest* test, TestHistogram* lat } } -ACTOR Future testKVCommit( KVTest* test, TestHistogram* latency, PerfIntCounter* count ) { +ACTOR Future testKVCommit(KVTest* test, TestHistogram* latency, PerfIntCounter* count) { state Version v = test->lastSet; test->lastCommit = v; state double begin = timer(); @@ -232,7 +232,7 @@ struct KVStoreTestWorkload : TestWorkload { return Void(); } virtual Future check( Database const& cx ) { return true; } - void metricsFromHistogram(vector& m, std::string name, TestHistogram& h){ + void metricsFromHistogram(vector& m, std::string name, TestHistogram& h) { m.push_back( PerfMetric( "Min " + name, 1000.0 * h.min(), true) ); m.push_back( PerfMetric( "Average " + name, 1000.0 * h.mean(), true) ); m.push_back( PerfMetric( "Median " + name, 1000.0 * h.medianEstimate(), true) ); diff --git a/flow/Histogram.cpp b/flow/Histogram.cpp index 51aa647065..6fa0f14384 100644 --- a/flow/Histogram.cpp +++ b/flow/Histogram.cpp @@ -29,8 +29,8 @@ // metrics. #include -// pull in some global pointers too: These types are implemented in fdbrpc/sim2.actor.cpp, which is not available here. Yuck. -// If you're not using the simulator, these will remain null, and all should be well. +// pull in some global pointers too: These types are implemented in fdbrpc/sim2.actor.cpp, which is not available here. +// Yuck. If you're not using the simulator, these will remain null, and all should be well. // TODO: create a execution context abstraction that allows independent flow instances within a process. // The simulator would be the main user of it, and histogram would be the only other user (for now). @@ -39,133 +39,130 @@ thread_local ISimulator::ProcessInfo* ISimulator::currentProcess = nullptr; // Fallback registry when we're not in simulation -- if we had execution contexts we wouldn't need to check if // we have a simulated contex here; we'd just use the current context regardless. -static HistogramRegistry * globalHistograms = nullptr; +static HistogramRegistry* globalHistograms = nullptr; -HistogramRegistry & GetHistogramRegistry() { - ISimulator::ProcessInfo * info = g_simulator.getCurrentProcess(); +HistogramRegistry& GetHistogramRegistry() { + ISimulator::ProcessInfo* info = g_simulator.getCurrentProcess(); - if (info) { - // in simulator; scope histograms to simulated process - return info->histograms; - } - // avoid link order issues where the registry hasn't been initialized, but we're - // instantiating a histogram - if (globalHistograms == nullptr) { - globalHistograms = new HistogramRegistry(); - } - return *globalHistograms; + if (info) { + // in simulator; scope histograms to simulated process + return info->histograms; + } + // avoid link order issues where the registry hasn't been initialized, but we're + // instantiating a histogram + if (globalHistograms == nullptr) { + globalHistograms = new HistogramRegistry(); + } + return *globalHistograms; } -void HistogramRegistry::registerHistogram(Histogram * h) { - if (histograms.find(h->name()) != histograms.end()) { - TraceEvent(SevError, "HistogramDoubleRegistered") - .detail("group", h->group) - .detail("op", h->op); - ASSERT(false); - } - histograms.insert(std::pair(h->name(), h)); +void HistogramRegistry::registerHistogram(Histogram* h) { + if (histograms.find(h->name()) != histograms.end()) { + TraceEvent(SevError, "HistogramDoubleRegistered").detail("group", h->group).detail("op", h->op); + ASSERT(false); + } + histograms.insert(std::pair(h->name(), h)); } -void HistogramRegistry::unregisterHistogram(Histogram * h) { - if (histograms.find(h->name()) == histograms.end()) { - TraceEvent(SevError, "HistogramNotRegistered") - .detail("group", h->group) - .detail("op", h->op); - } - ASSERT(histograms.erase(h->name()) == 1); +void HistogramRegistry::unregisterHistogram(Histogram* h) { + if (histograms.find(h->name()) == histograms.end()) { + TraceEvent(SevError, "HistogramNotRegistered").detail("group", h->group).detail("op", h->op); + } + ASSERT(histograms.erase(h->name()) == 1); } -Histogram * HistogramRegistry::lookupHistogram(std::string name) { - auto h = histograms.find(name); - if (h == histograms.end()) { - return nullptr; - } - return h->second; +Histogram* HistogramRegistry::lookupHistogram(std::string name) { + auto h = histograms.find(name); + if (h == histograms.end()) { + return nullptr; + } + return h->second; } void HistogramRegistry::logReport() { - for (auto & i : histograms) { - i.second->writeToLog(); - i.second->clear(); - } + for (auto& i : histograms) { + i.second->writeToLog(); + i.second->clear(); + } } void Histogram::writeToLog() { - bool active = false; - for (uint32_t i = 0; i < 32; i++) { - if (buckets[i]) { - active = true; - break; - } - } - if (!active) { - return ; - } + bool active = false; + for (uint32_t i = 0; i < 32; i++) { + if (buckets[i]) { + active = true; + break; + } + } + if (!active) { + return; + } - TraceEvent e(SevInfo, "Histogram"); - e.detail("Group", group).detail("Op", op); - for (uint32_t i = 0; i < 32; i++) { - if (buckets[i]) { - switch(unit) { - case Unit::microseconds: - { - uint32_t usec = ((uint32_t)1)<<(i+1); - e.detail(format("LessThan%u.%03u", usec / 1000, usec % 1000), buckets[i]); - break; - } - case Unit::bytes: - e.detail(format("LessThan%u", ((uint32_t)1)<<(i+1)), buckets[i]); - break; - default: - ASSERT(false); - } - } - } + TraceEvent e(SevInfo, "Histogram"); + e.detail("Group", group).detail("Op", op); + for (uint32_t i = 0; i < 32; i++) { + if (buckets[i]) { + switch (unit) { + case Unit::microseconds: { + uint32_t usec = ((uint32_t)1) << (i + 1); + e.detail(format("LessThan%u.%03u", usec / 1000, usec % 1000), buckets[i]); + break; + } + case Unit::bytes: + e.detail(format("LessThan%u", ((uint32_t)1) << (i + 1)), buckets[i]); + break; + default: + ASSERT(false); + } + } + } } TEST_CASE("/flow/histogram/smoke_test") { - - { - Reference h = Histogram::getHistogram(LiteralStringRef("smoke_test"), LiteralStringRef("counts"), Histogram::Unit::bytes); - h->sample(0); - ASSERT(h->buckets[0] == 1); - h->sample(1); - ASSERT(h->buckets[0] == 2); + { + Reference h = + Histogram::getHistogram(LiteralStringRef("smoke_test"), LiteralStringRef("counts"), Histogram::Unit::bytes); - h->sample(2); - ASSERT(h->buckets[1] == 1); + h->sample(0); + ASSERT(h->buckets[0] == 1); + h->sample(1); + ASSERT(h->buckets[0] == 2); - GetHistogramRegistry().logReport(); + h->sample(2); + ASSERT(h->buckets[1] == 1); - ASSERT(h->buckets[0] == 0); - h->sample(0); - ASSERT(h->buckets[0] == 1); - h = Histogram::getHistogram(LiteralStringRef("smoke_test"), LiteralStringRef("counts2"), Histogram::Unit::bytes); - - // confirm that old h was deallocated. - h = Histogram::getHistogram(LiteralStringRef("smoke_test"), LiteralStringRef("counts"), Histogram::Unit::bytes); - ASSERT(h->buckets[0] == 0); + GetHistogramRegistry().logReport(); - h = Histogram::getHistogram(LiteralStringRef("smoke_test"), LiteralStringRef("times"), Histogram::Unit::microseconds); + ASSERT(h->buckets[0] == 0); + h->sample(0); + ASSERT(h->buckets[0] == 1); + h = Histogram::getHistogram(LiteralStringRef("smoke_test"), LiteralStringRef("counts2"), + Histogram::Unit::bytes); - h->sampleSeconds(0.000000); - h->sampleSeconds(0.0000019); - ASSERT(h->buckets[0] == 2); - h->sampleSeconds(0.0000021); - ASSERT(h->buckets[1] == 1); - h->sampleSeconds(0.000015); - ASSERT(h->buckets[3] == 1); + // confirm that old h was deallocated. + h = Histogram::getHistogram(LiteralStringRef("smoke_test"), LiteralStringRef("counts"), Histogram::Unit::bytes); + ASSERT(h->buckets[0] == 0); - h->sampleSeconds(4400.0); - ASSERT(h->buckets[31] == 1); + h = Histogram::getHistogram(LiteralStringRef("smoke_test"), LiteralStringRef("times"), + Histogram::Unit::microseconds); - GetHistogramRegistry().logReport(); + h->sampleSeconds(0.000000); + h->sampleSeconds(0.0000019); + ASSERT(h->buckets[0] == 2); + h->sampleSeconds(0.0000021); + ASSERT(h->buckets[1] == 1); + h->sampleSeconds(0.000015); + ASSERT(h->buckets[3] == 1); - } + h->sampleSeconds(4400.0); + ASSERT(h->buckets[31] == 1); - // h has been deallocated. Does this crash? - GetHistogramRegistry().logReport(); + GetHistogramRegistry().logReport(); + } - return Void(); + // h has been deallocated. Does this crash? + GetHistogramRegistry().logReport(); + + return Void(); } \ No newline at end of file diff --git a/flow/Histogram.h b/flow/Histogram.h index e9ba06f9f7..93dd508686 100644 --- a/flow/Histogram.h +++ b/flow/Histogram.h @@ -35,88 +35,80 @@ class Histogram; class HistogramRegistry { - public: - void registerHistogram(Histogram * h); - void unregisterHistogram(Histogram * h); - Histogram * lookupHistogram(std::string name); - void logReport(); - private: - std::map histograms; +public: + void registerHistogram(Histogram* h); + void unregisterHistogram(Histogram* h); + Histogram* lookupHistogram(std::string name); + void logReport(); + +private: + std::map histograms; }; -// TODO: This should be scoped properly for simulation (instead of just having all the "machines" share one histogram namespace) -HistogramRegistry & GetHistogramRegistry(); +// TODO: This should be scoped properly for simulation (instead of just having all the "machines" share one histogram +// namespace) +HistogramRegistry& GetHistogramRegistry(); class Histogram : public ReferenceCounted { public: - enum class Unit { - microseconds, - bytes - }; + enum class Unit { microseconds, bytes }; private: - Histogram(std::string group, std::string op, Unit unit) : group(group), op(op), unit(unit), registry(GetHistogramRegistry()) { - clear(); - registry.registerHistogram(this); - } + Histogram(std::string group, std::string op, Unit unit) + : group(group), op(op), unit(unit), registry(GetHistogramRegistry()) { + clear(); + registry.registerHistogram(this); + } - static std::string generateName(std::string group, std::string op) { - return group + ":" + op; - } + static std::string generateName(std::string group, std::string op) { return group + ":" + op; } public: + ~Histogram() { registry.unregisterHistogram(this); } - ~Histogram() { - registry.unregisterHistogram(this); - } + static Reference getHistogram(StringRef group, StringRef op, Unit unit) { + std::string group_str = group.toString(); + std::string op_str = op.toString(); + std::string name = generateName(group_str, op_str); + HistogramRegistry& registry = GetHistogramRegistry(); + Histogram* h = registry.lookupHistogram(name); + if (!h) { + h = new Histogram(group_str, op_str, unit); + } + return Reference(h); + } - - static Reference getHistogram(StringRef group, StringRef op, Unit unit) { - std::string group_str = group.toString(); - std::string op_str = op.toString(); - std::string name = generateName(group_str, op_str); - HistogramRegistry & registry = GetHistogramRegistry(); - Histogram * h = registry.lookupHistogram(name); - if (!h) { - h = new Histogram(group_str, op_str, unit); - } - return Reference(h); - } - - inline void sample(uint32_t sample) { + inline void sample(uint32_t sample) { #ifdef _WIN32 - unsigned long index; - buckets[_BitScanReverse(&index, sample) ? index : 0]++; + unsigned long index; + buckets[_BitScanReverse(&index, sample) ? index : 0]++; #else - buckets[sample ? (31 - __builtin_clz(sample)) : 0]++; + buckets[sample ? (31 - __builtin_clz(sample)) : 0]++; #endif - } + } - inline void sampleSeconds(double delta) { - uint64_t delta_usec = (delta * 1000000); - if (delta_usec > UINT32_MAX) { - sample(UINT32_MAX); - } else { - sample((uint32_t)(delta * 1000000)); // convert to microseconds and truncate to integer - } - } + inline void sampleSeconds(double delta) { + uint64_t delta_usec = (delta * 1000000); + if (delta_usec > UINT32_MAX) { + sample(UINT32_MAX); + } else { + sample((uint32_t)(delta * 1000000)); // convert to microseconds and truncate to integer + } + } - void clear() { - for (uint32_t & i : buckets) { - i = 0; - } - } - void writeToLog(); + void clear() { + for (uint32_t& i : buckets) { + i = 0; + } + } + void writeToLog(); - std::string name() { - return generateName(this->group, this->op); - } + std::string name() { return generateName(this->group, this->op); } - std::string const group; - std::string const op; - Unit const unit; - HistogramRegistry & registry; - uint32_t buckets[32]; + std::string const group; + std::string const op; + Unit const unit; + HistogramRegistry& registry; + uint32_t buckets[32]; }; #endif // FLOW_HISTOGRAM_H \ No newline at end of file diff --git a/flow/serialize.h b/flow/serialize.h index db51167c88..4028df16e7 100644 --- a/flow/serialize.h +++ b/flow/serialize.h @@ -666,7 +666,7 @@ private: }; struct PacketBuffer : SendBuffer { -private: +private: int reference_count; uint32_t const size_; double const enqueue_time;