A header only class for C++ time measurement
$begingroup$
I wrote a header only class (actually there is a second one for data bookkeeping and dumping) to measure the execution time of a C++ scope without worrying too much about boilerplates. The idea being to be able to simply instantiate a class at the begining of the scope you want to measure, and do a single call at the end to dump the measure.
I rely on the fact that the class instantiation is at the beginning of the scope, and its destruction at the very end. So my main worry is about compile time optimizations that could change the order of execution and bias the measure. Also I'm not satisfy on how I retrieve the type for ScopeTimer::Duration
but I couldn't find the type properly :/
Here is the code:
scope_timer.hpp
#ifndef SCOPE_TIMER
#define SCOPE_TIMER
#include <chrono>
#include <string>
#include <vector>
#include <map>
#include <fstream>
class ScopeTimer {
public:
using ScopeSignature = std::string;
using DurationType = std::chrono::microseconds;
using Duration = decltype(std::chrono::duration_cast<DurationType>(std::chrono::high_resolution_clock::now() - std::chrono::high_resolution_clock::now()).count());
ScopeTimer(const ScopeSignature& scopeName);
~ScopeTimer();
Duration getDurationFromStart() const;
private:
ScopeTimer();
const ScopeSignature scopeName;
const std::chrono::high_resolution_clock::time_point start;
};
class ScopeTimerStaticCore {
public:
static void addTimingToNamedScope(const ScopeTimer::ScopeSignature& scopeName, const ScopeTimer::Duration& duration);
static void dumpTimingToFile(const std::string& path);
static void clearAllTiming();
static void clearTimingForNamedScope(const ScopeTimer::ScopeSignature& scopeName);
private:
using TimingVector = std::vector<ScopeTimer::Duration>;
using ScopesTiming = std::map<ScopeTimer::ScopeSignature, TimingVector>;
static ScopesTiming& getScopesTimingStaticInstance() {
static ScopesTiming scopesTimingContainer;
return (scopesTimingContainer);
};
};
/*******************************************************Implementations*******************************************************/
inline ScopeTimer::ScopeTimer(const ScopeSignature& scopeName) : scopeName(scopeName), start(std::chrono::high_resolution_clock::now()) {};
inline ScopeTimer::~ScopeTimer() {
const Duration scopeTimerLifetimeDuration = this->getDurationFromStart();
ScopeTimerStaticCore::addTimingToNamedScope(this->scopeName, scopeTimerLifetimeDuration);
return ;
};
inline ScopeTimer::Duration ScopeTimer::getDurationFromStart() const {
using std::chrono::duration_cast;
const std::chrono::high_resolution_clock::time_point now = std::chrono::high_resolution_clock::now();
return (duration_cast<DurationType>(now - this->start).count());
};
inline void ScopeTimerStaticCore::addTimingToNamedScope(const ScopeTimer::ScopeSignature& scopeName, const ScopeTimer::Duration& duration) {
ScopesTiming& scopesTimingContainer = ScopeTimerStaticCore::getScopesTimingStaticInstance();
scopesTimingContainer[scopeName].push_back(duration);
return ;
};
inline void ScopeTimerStaticCore::dumpTimingToFile(const std::string& path) {
const ScopesTiming& scopesTimingContainer = ScopeTimerStaticCore::getScopesTimingStaticInstance();
std::ofstream dumpfile;
dumpfile.open(path, std::ios::out | std::ios::trunc);
for (ScopesTiming::const_iterator it_scopes = scopesTimingContainer.begin(); it_scopes != scopesTimingContainer.end(); ++it_scopes) {
const ScopeTimer::ScopeSignature& currentScope = it_scopes->first;
const TimingVector& timings = it_scopes->second;
for (TimingVector::const_iterator it_timings = timings.begin(); it_timings != timings.end(); ++it_timings)
dumpfile << currentScope << "," << *it_timings << std::endl;
}
dumpfile.close();
return ;
};
inline void ScopeTimerStaticCore::clearAllTiming() {
ScopesTiming& scopesTimingContainer = ScopeTimerStaticCore::getScopesTimingStaticInstance();
scopesTimingContainer.clear();
return ;
};
inline void ScopeTimerStaticCore::clearTimingForNamedScope(const ScopeTimer::ScopeSignature& scopeName) {
ScopesTiming& scopesTimingContainer = ScopeTimerStaticCore::getScopesTimingStaticInstance();
ScopesTiming::iterator it_scopes = scopesTimingContainer.find(scopeName);
if (it_scopes != scopesTimingContainer.end())
it_scopes->second.clear();
return ;
};
#endif /* SCOPE_TIMER */
And an dummy program that use it
main.cpp
#include "../include/scope_timer.hpp"
void functionA();
void functionB();
int main() {
for (size_t i = 0; i < 3; ++i) {
functionA();
functionB();
}
ScopeTimerStaticCore::dumpTimingToFile("/tmp/scope-timer_dump-dummy-test.csv");
return (0);
};
dumb_functions.cpp
#include <thread>
#include <chrono>
#include "../include/scope_timer.hpp"
void functionA() {
ScopeTimer scopeTimer("functionA");
std::this_thread::sleep_for (std::chrono::milliseconds(500));
return ;
};
void functionB() {
ScopeTimer scopeTimer("functionB");
std::this_thread::sleep_for (std::chrono::seconds(1));
return ;
};
if you want to retrieve the code quickly here is the repo link
c++ performance c++11 benchmarking
New contributor
$endgroup$
add a comment |
$begingroup$
I wrote a header only class (actually there is a second one for data bookkeeping and dumping) to measure the execution time of a C++ scope without worrying too much about boilerplates. The idea being to be able to simply instantiate a class at the begining of the scope you want to measure, and do a single call at the end to dump the measure.
I rely on the fact that the class instantiation is at the beginning of the scope, and its destruction at the very end. So my main worry is about compile time optimizations that could change the order of execution and bias the measure. Also I'm not satisfy on how I retrieve the type for ScopeTimer::Duration
but I couldn't find the type properly :/
Here is the code:
scope_timer.hpp
#ifndef SCOPE_TIMER
#define SCOPE_TIMER
#include <chrono>
#include <string>
#include <vector>
#include <map>
#include <fstream>
class ScopeTimer {
public:
using ScopeSignature = std::string;
using DurationType = std::chrono::microseconds;
using Duration = decltype(std::chrono::duration_cast<DurationType>(std::chrono::high_resolution_clock::now() - std::chrono::high_resolution_clock::now()).count());
ScopeTimer(const ScopeSignature& scopeName);
~ScopeTimer();
Duration getDurationFromStart() const;
private:
ScopeTimer();
const ScopeSignature scopeName;
const std::chrono::high_resolution_clock::time_point start;
};
class ScopeTimerStaticCore {
public:
static void addTimingToNamedScope(const ScopeTimer::ScopeSignature& scopeName, const ScopeTimer::Duration& duration);
static void dumpTimingToFile(const std::string& path);
static void clearAllTiming();
static void clearTimingForNamedScope(const ScopeTimer::ScopeSignature& scopeName);
private:
using TimingVector = std::vector<ScopeTimer::Duration>;
using ScopesTiming = std::map<ScopeTimer::ScopeSignature, TimingVector>;
static ScopesTiming& getScopesTimingStaticInstance() {
static ScopesTiming scopesTimingContainer;
return (scopesTimingContainer);
};
};
/*******************************************************Implementations*******************************************************/
inline ScopeTimer::ScopeTimer(const ScopeSignature& scopeName) : scopeName(scopeName), start(std::chrono::high_resolution_clock::now()) {};
inline ScopeTimer::~ScopeTimer() {
const Duration scopeTimerLifetimeDuration = this->getDurationFromStart();
ScopeTimerStaticCore::addTimingToNamedScope(this->scopeName, scopeTimerLifetimeDuration);
return ;
};
inline ScopeTimer::Duration ScopeTimer::getDurationFromStart() const {
using std::chrono::duration_cast;
const std::chrono::high_resolution_clock::time_point now = std::chrono::high_resolution_clock::now();
return (duration_cast<DurationType>(now - this->start).count());
};
inline void ScopeTimerStaticCore::addTimingToNamedScope(const ScopeTimer::ScopeSignature& scopeName, const ScopeTimer::Duration& duration) {
ScopesTiming& scopesTimingContainer = ScopeTimerStaticCore::getScopesTimingStaticInstance();
scopesTimingContainer[scopeName].push_back(duration);
return ;
};
inline void ScopeTimerStaticCore::dumpTimingToFile(const std::string& path) {
const ScopesTiming& scopesTimingContainer = ScopeTimerStaticCore::getScopesTimingStaticInstance();
std::ofstream dumpfile;
dumpfile.open(path, std::ios::out | std::ios::trunc);
for (ScopesTiming::const_iterator it_scopes = scopesTimingContainer.begin(); it_scopes != scopesTimingContainer.end(); ++it_scopes) {
const ScopeTimer::ScopeSignature& currentScope = it_scopes->first;
const TimingVector& timings = it_scopes->second;
for (TimingVector::const_iterator it_timings = timings.begin(); it_timings != timings.end(); ++it_timings)
dumpfile << currentScope << "," << *it_timings << std::endl;
}
dumpfile.close();
return ;
};
inline void ScopeTimerStaticCore::clearAllTiming() {
ScopesTiming& scopesTimingContainer = ScopeTimerStaticCore::getScopesTimingStaticInstance();
scopesTimingContainer.clear();
return ;
};
inline void ScopeTimerStaticCore::clearTimingForNamedScope(const ScopeTimer::ScopeSignature& scopeName) {
ScopesTiming& scopesTimingContainer = ScopeTimerStaticCore::getScopesTimingStaticInstance();
ScopesTiming::iterator it_scopes = scopesTimingContainer.find(scopeName);
if (it_scopes != scopesTimingContainer.end())
it_scopes->second.clear();
return ;
};
#endif /* SCOPE_TIMER */
And an dummy program that use it
main.cpp
#include "../include/scope_timer.hpp"
void functionA();
void functionB();
int main() {
for (size_t i = 0; i < 3; ++i) {
functionA();
functionB();
}
ScopeTimerStaticCore::dumpTimingToFile("/tmp/scope-timer_dump-dummy-test.csv");
return (0);
};
dumb_functions.cpp
#include <thread>
#include <chrono>
#include "../include/scope_timer.hpp"
void functionA() {
ScopeTimer scopeTimer("functionA");
std::this_thread::sleep_for (std::chrono::milliseconds(500));
return ;
};
void functionB() {
ScopeTimer scopeTimer("functionB");
std::this_thread::sleep_for (std::chrono::seconds(1));
return ;
};
if you want to retrieve the code quickly here is the repo link
c++ performance c++11 benchmarking
New contributor
$endgroup$
add a comment |
$begingroup$
I wrote a header only class (actually there is a second one for data bookkeeping and dumping) to measure the execution time of a C++ scope without worrying too much about boilerplates. The idea being to be able to simply instantiate a class at the begining of the scope you want to measure, and do a single call at the end to dump the measure.
I rely on the fact that the class instantiation is at the beginning of the scope, and its destruction at the very end. So my main worry is about compile time optimizations that could change the order of execution and bias the measure. Also I'm not satisfy on how I retrieve the type for ScopeTimer::Duration
but I couldn't find the type properly :/
Here is the code:
scope_timer.hpp
#ifndef SCOPE_TIMER
#define SCOPE_TIMER
#include <chrono>
#include <string>
#include <vector>
#include <map>
#include <fstream>
class ScopeTimer {
public:
using ScopeSignature = std::string;
using DurationType = std::chrono::microseconds;
using Duration = decltype(std::chrono::duration_cast<DurationType>(std::chrono::high_resolution_clock::now() - std::chrono::high_resolution_clock::now()).count());
ScopeTimer(const ScopeSignature& scopeName);
~ScopeTimer();
Duration getDurationFromStart() const;
private:
ScopeTimer();
const ScopeSignature scopeName;
const std::chrono::high_resolution_clock::time_point start;
};
class ScopeTimerStaticCore {
public:
static void addTimingToNamedScope(const ScopeTimer::ScopeSignature& scopeName, const ScopeTimer::Duration& duration);
static void dumpTimingToFile(const std::string& path);
static void clearAllTiming();
static void clearTimingForNamedScope(const ScopeTimer::ScopeSignature& scopeName);
private:
using TimingVector = std::vector<ScopeTimer::Duration>;
using ScopesTiming = std::map<ScopeTimer::ScopeSignature, TimingVector>;
static ScopesTiming& getScopesTimingStaticInstance() {
static ScopesTiming scopesTimingContainer;
return (scopesTimingContainer);
};
};
/*******************************************************Implementations*******************************************************/
inline ScopeTimer::ScopeTimer(const ScopeSignature& scopeName) : scopeName(scopeName), start(std::chrono::high_resolution_clock::now()) {};
inline ScopeTimer::~ScopeTimer() {
const Duration scopeTimerLifetimeDuration = this->getDurationFromStart();
ScopeTimerStaticCore::addTimingToNamedScope(this->scopeName, scopeTimerLifetimeDuration);
return ;
};
inline ScopeTimer::Duration ScopeTimer::getDurationFromStart() const {
using std::chrono::duration_cast;
const std::chrono::high_resolution_clock::time_point now = std::chrono::high_resolution_clock::now();
return (duration_cast<DurationType>(now - this->start).count());
};
inline void ScopeTimerStaticCore::addTimingToNamedScope(const ScopeTimer::ScopeSignature& scopeName, const ScopeTimer::Duration& duration) {
ScopesTiming& scopesTimingContainer = ScopeTimerStaticCore::getScopesTimingStaticInstance();
scopesTimingContainer[scopeName].push_back(duration);
return ;
};
inline void ScopeTimerStaticCore::dumpTimingToFile(const std::string& path) {
const ScopesTiming& scopesTimingContainer = ScopeTimerStaticCore::getScopesTimingStaticInstance();
std::ofstream dumpfile;
dumpfile.open(path, std::ios::out | std::ios::trunc);
for (ScopesTiming::const_iterator it_scopes = scopesTimingContainer.begin(); it_scopes != scopesTimingContainer.end(); ++it_scopes) {
const ScopeTimer::ScopeSignature& currentScope = it_scopes->first;
const TimingVector& timings = it_scopes->second;
for (TimingVector::const_iterator it_timings = timings.begin(); it_timings != timings.end(); ++it_timings)
dumpfile << currentScope << "," << *it_timings << std::endl;
}
dumpfile.close();
return ;
};
inline void ScopeTimerStaticCore::clearAllTiming() {
ScopesTiming& scopesTimingContainer = ScopeTimerStaticCore::getScopesTimingStaticInstance();
scopesTimingContainer.clear();
return ;
};
inline void ScopeTimerStaticCore::clearTimingForNamedScope(const ScopeTimer::ScopeSignature& scopeName) {
ScopesTiming& scopesTimingContainer = ScopeTimerStaticCore::getScopesTimingStaticInstance();
ScopesTiming::iterator it_scopes = scopesTimingContainer.find(scopeName);
if (it_scopes != scopesTimingContainer.end())
it_scopes->second.clear();
return ;
};
#endif /* SCOPE_TIMER */
And an dummy program that use it
main.cpp
#include "../include/scope_timer.hpp"
void functionA();
void functionB();
int main() {
for (size_t i = 0; i < 3; ++i) {
functionA();
functionB();
}
ScopeTimerStaticCore::dumpTimingToFile("/tmp/scope-timer_dump-dummy-test.csv");
return (0);
};
dumb_functions.cpp
#include <thread>
#include <chrono>
#include "../include/scope_timer.hpp"
void functionA() {
ScopeTimer scopeTimer("functionA");
std::this_thread::sleep_for (std::chrono::milliseconds(500));
return ;
};
void functionB() {
ScopeTimer scopeTimer("functionB");
std::this_thread::sleep_for (std::chrono::seconds(1));
return ;
};
if you want to retrieve the code quickly here is the repo link
c++ performance c++11 benchmarking
New contributor
$endgroup$
I wrote a header only class (actually there is a second one for data bookkeeping and dumping) to measure the execution time of a C++ scope without worrying too much about boilerplates. The idea being to be able to simply instantiate a class at the begining of the scope you want to measure, and do a single call at the end to dump the measure.
I rely on the fact that the class instantiation is at the beginning of the scope, and its destruction at the very end. So my main worry is about compile time optimizations that could change the order of execution and bias the measure. Also I'm not satisfy on how I retrieve the type for ScopeTimer::Duration
but I couldn't find the type properly :/
Here is the code:
scope_timer.hpp
#ifndef SCOPE_TIMER
#define SCOPE_TIMER
#include <chrono>
#include <string>
#include <vector>
#include <map>
#include <fstream>
class ScopeTimer {
public:
using ScopeSignature = std::string;
using DurationType = std::chrono::microseconds;
using Duration = decltype(std::chrono::duration_cast<DurationType>(std::chrono::high_resolution_clock::now() - std::chrono::high_resolution_clock::now()).count());
ScopeTimer(const ScopeSignature& scopeName);
~ScopeTimer();
Duration getDurationFromStart() const;
private:
ScopeTimer();
const ScopeSignature scopeName;
const std::chrono::high_resolution_clock::time_point start;
};
class ScopeTimerStaticCore {
public:
static void addTimingToNamedScope(const ScopeTimer::ScopeSignature& scopeName, const ScopeTimer::Duration& duration);
static void dumpTimingToFile(const std::string& path);
static void clearAllTiming();
static void clearTimingForNamedScope(const ScopeTimer::ScopeSignature& scopeName);
private:
using TimingVector = std::vector<ScopeTimer::Duration>;
using ScopesTiming = std::map<ScopeTimer::ScopeSignature, TimingVector>;
static ScopesTiming& getScopesTimingStaticInstance() {
static ScopesTiming scopesTimingContainer;
return (scopesTimingContainer);
};
};
/*******************************************************Implementations*******************************************************/
inline ScopeTimer::ScopeTimer(const ScopeSignature& scopeName) : scopeName(scopeName), start(std::chrono::high_resolution_clock::now()) {};
inline ScopeTimer::~ScopeTimer() {
const Duration scopeTimerLifetimeDuration = this->getDurationFromStart();
ScopeTimerStaticCore::addTimingToNamedScope(this->scopeName, scopeTimerLifetimeDuration);
return ;
};
inline ScopeTimer::Duration ScopeTimer::getDurationFromStart() const {
using std::chrono::duration_cast;
const std::chrono::high_resolution_clock::time_point now = std::chrono::high_resolution_clock::now();
return (duration_cast<DurationType>(now - this->start).count());
};
inline void ScopeTimerStaticCore::addTimingToNamedScope(const ScopeTimer::ScopeSignature& scopeName, const ScopeTimer::Duration& duration) {
ScopesTiming& scopesTimingContainer = ScopeTimerStaticCore::getScopesTimingStaticInstance();
scopesTimingContainer[scopeName].push_back(duration);
return ;
};
inline void ScopeTimerStaticCore::dumpTimingToFile(const std::string& path) {
const ScopesTiming& scopesTimingContainer = ScopeTimerStaticCore::getScopesTimingStaticInstance();
std::ofstream dumpfile;
dumpfile.open(path, std::ios::out | std::ios::trunc);
for (ScopesTiming::const_iterator it_scopes = scopesTimingContainer.begin(); it_scopes != scopesTimingContainer.end(); ++it_scopes) {
const ScopeTimer::ScopeSignature& currentScope = it_scopes->first;
const TimingVector& timings = it_scopes->second;
for (TimingVector::const_iterator it_timings = timings.begin(); it_timings != timings.end(); ++it_timings)
dumpfile << currentScope << "," << *it_timings << std::endl;
}
dumpfile.close();
return ;
};
inline void ScopeTimerStaticCore::clearAllTiming() {
ScopesTiming& scopesTimingContainer = ScopeTimerStaticCore::getScopesTimingStaticInstance();
scopesTimingContainer.clear();
return ;
};
inline void ScopeTimerStaticCore::clearTimingForNamedScope(const ScopeTimer::ScopeSignature& scopeName) {
ScopesTiming& scopesTimingContainer = ScopeTimerStaticCore::getScopesTimingStaticInstance();
ScopesTiming::iterator it_scopes = scopesTimingContainer.find(scopeName);
if (it_scopes != scopesTimingContainer.end())
it_scopes->second.clear();
return ;
};
#endif /* SCOPE_TIMER */
And an dummy program that use it
main.cpp
#include "../include/scope_timer.hpp"
void functionA();
void functionB();
int main() {
for (size_t i = 0; i < 3; ++i) {
functionA();
functionB();
}
ScopeTimerStaticCore::dumpTimingToFile("/tmp/scope-timer_dump-dummy-test.csv");
return (0);
};
dumb_functions.cpp
#include <thread>
#include <chrono>
#include "../include/scope_timer.hpp"
void functionA() {
ScopeTimer scopeTimer("functionA");
std::this_thread::sleep_for (std::chrono::milliseconds(500));
return ;
};
void functionB() {
ScopeTimer scopeTimer("functionB");
std::this_thread::sleep_for (std::chrono::seconds(1));
return ;
};
if you want to retrieve the code quickly here is the repo link
c++ performance c++11 benchmarking
c++ performance c++11 benchmarking
New contributor
New contributor
edited 3 hours ago
shorty_ponton
New contributor
asked 4 hours ago
shorty_pontonshorty_ponton
112
112
New contributor
New contributor
add a comment |
add a comment |
1 Answer
1
active
oldest
votes
$begingroup$
using Duration = decltype(std::chrono::duration_cast<DurationType>(std::chrono::high_resolution_clock::now() - std::chrono::high_resolution_clock::now()).count());
I'm confused by this line. decltype(std::chrono::duration_cast<DurationType>(expr))
is invariably DurationType
, isn't it? That's why it's a "cast"? So this simplifies down to decltype(std::declval<DurationType&>().count())
, which I'm pretty sure can be spelled as DurationType::rep
unless you're really eager to support non-standard duration types that might not have a rep
member. So:
using Duration = typename DurationType::rep;
And now it appears that maybe Duration
is the wrong name for this typedef, eh?
static ScopesTiming& getScopesTimingStaticInstance() {
static ScopesTiming scopesTimingContainer;
return (scopesTimingContainer);
};
Minor nits on whitespace and naming and parentheses and trailing semicolons:
static ScopesTiming& getScopesTimingStaticInstance() {
static ScopesTiming instance;
return instance;
}
The defining quality of instance
is that it's a static instance of ScopesTiming
. If you want to convey the additional information that ScopesTiming
is actually a container type, then that information belongs in the name of the type. Personally I'd call it something like TimingVectorMap
, because it's a map of TimingVector
s.
Since the static map is not guarded by any mutex, your function addTimingToNamedScope
(which mutates the map) is not safe to call from multiple threads concurrently. This could be a problem for real-world use.
ScopeTimer
has two const
-qualified fields. This doesn't do anything except pessimize its implicitly generated move-constructor into a copy-constructor. I recommend removing the const
.
ScopeTimer
also has an implicit conversion from ScopeSignature
a.k.a. std::string
, so that for example
void f(ScopeTimer timer);
std::string hello = "hello world";
f(hello); // compiles without any diagnostic
I very strongly suggest that you never enable any implicit conversion unless you have a very good reason for it. This means putting explicit
on every constructor and conversion operator.
explicit ScopeTimer(const ScopeSignature& scopeName);
dumpfile.open(path, std::ios::out | std::ios::trunc);
Should you check to see if the open succeeded?
inline void ScopeTimerStaticCore::clearTimingForNamedScope(const ScopeTimer::ScopeSignature& scopeName) {
ScopesTiming& scopesTimingContainer = ScopeTimerStaticCore::getScopesTimingStaticInstance();
ScopesTiming::iterator it_scopes = scopesTimingContainer.find(scopeName);
if (it_scopes != scopesTimingContainer.end())
it_scopes->second.clear();
return ;
};
This would be a good place to use C++11 auto
:
inline void ScopeTimerStaticCore::clearTimingForNamedScope(const ScopeTimer::ScopeSignature& scopeName) {
ScopesTiming& instance = getScopesTimingStaticInstance();
auto it = instance.find(scopeName);
if (it != instance.end()) {
it->second.clear();
}
}
Or, if you don't mind removing the element from the map completely, you could just use erase
:
inline void ScopeTimerStaticCore::clearTimingForNamedScope(const ScopeTimer::ScopeSignature& scopeName) {
ScopesTiming& instance = getScopesTimingStaticInstance();
instance.erase(scopeName);
}
I also notice that these functions would get a lot shorter and simpler to read if you put their definitions in-line into the class body of ScopeTimerStaticCore
. In this case you could omit the keyword inline
and the qualification of the parameter type:
void clearTimingForNamedScope(const ScopeSignature& scopeName) {
ScopesTiming& instance = getScopesTimingStaticInstance();
instance.erase(scopeName);
}
(Assuming that ScopeTimerStaticCore
contains a member typedef using ScopeSignature = ScopeTimer::ScopeSignature;
, I guess. It probably should — or vice versa.)
$endgroup$
add a comment |
Your Answer
StackExchange.ifUsing("editor", function () {
return StackExchange.using("mathjaxEditing", function () {
StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix) {
StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
});
});
}, "mathjax-editing");
StackExchange.ifUsing("editor", function () {
StackExchange.using("externalEditor", function () {
StackExchange.using("snippets", function () {
StackExchange.snippets.init();
});
});
}, "code-snippets");
StackExchange.ready(function() {
var channelOptions = {
tags: "".split(" "),
id: "196"
};
initTagRenderer("".split(" "), "".split(" "), channelOptions);
StackExchange.using("externalEditor", function() {
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled) {
StackExchange.using("snippets", function() {
createEditor();
});
}
else {
createEditor();
}
});
function createEditor() {
StackExchange.prepareEditor({
heartbeatType: 'answer',
autoActivateHeartbeat: false,
convertImagesToLinks: false,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
imageUploader: {
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
},
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
});
}
});
shorty_ponton is a new contributor. Be nice, and check out our Code of Conduct.
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f211857%2fa-header-only-class-for-c-time-measurement%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
1 Answer
1
active
oldest
votes
1 Answer
1
active
oldest
votes
active
oldest
votes
active
oldest
votes
$begingroup$
using Duration = decltype(std::chrono::duration_cast<DurationType>(std::chrono::high_resolution_clock::now() - std::chrono::high_resolution_clock::now()).count());
I'm confused by this line. decltype(std::chrono::duration_cast<DurationType>(expr))
is invariably DurationType
, isn't it? That's why it's a "cast"? So this simplifies down to decltype(std::declval<DurationType&>().count())
, which I'm pretty sure can be spelled as DurationType::rep
unless you're really eager to support non-standard duration types that might not have a rep
member. So:
using Duration = typename DurationType::rep;
And now it appears that maybe Duration
is the wrong name for this typedef, eh?
static ScopesTiming& getScopesTimingStaticInstance() {
static ScopesTiming scopesTimingContainer;
return (scopesTimingContainer);
};
Minor nits on whitespace and naming and parentheses and trailing semicolons:
static ScopesTiming& getScopesTimingStaticInstance() {
static ScopesTiming instance;
return instance;
}
The defining quality of instance
is that it's a static instance of ScopesTiming
. If you want to convey the additional information that ScopesTiming
is actually a container type, then that information belongs in the name of the type. Personally I'd call it something like TimingVectorMap
, because it's a map of TimingVector
s.
Since the static map is not guarded by any mutex, your function addTimingToNamedScope
(which mutates the map) is not safe to call from multiple threads concurrently. This could be a problem for real-world use.
ScopeTimer
has two const
-qualified fields. This doesn't do anything except pessimize its implicitly generated move-constructor into a copy-constructor. I recommend removing the const
.
ScopeTimer
also has an implicit conversion from ScopeSignature
a.k.a. std::string
, so that for example
void f(ScopeTimer timer);
std::string hello = "hello world";
f(hello); // compiles without any diagnostic
I very strongly suggest that you never enable any implicit conversion unless you have a very good reason for it. This means putting explicit
on every constructor and conversion operator.
explicit ScopeTimer(const ScopeSignature& scopeName);
dumpfile.open(path, std::ios::out | std::ios::trunc);
Should you check to see if the open succeeded?
inline void ScopeTimerStaticCore::clearTimingForNamedScope(const ScopeTimer::ScopeSignature& scopeName) {
ScopesTiming& scopesTimingContainer = ScopeTimerStaticCore::getScopesTimingStaticInstance();
ScopesTiming::iterator it_scopes = scopesTimingContainer.find(scopeName);
if (it_scopes != scopesTimingContainer.end())
it_scopes->second.clear();
return ;
};
This would be a good place to use C++11 auto
:
inline void ScopeTimerStaticCore::clearTimingForNamedScope(const ScopeTimer::ScopeSignature& scopeName) {
ScopesTiming& instance = getScopesTimingStaticInstance();
auto it = instance.find(scopeName);
if (it != instance.end()) {
it->second.clear();
}
}
Or, if you don't mind removing the element from the map completely, you could just use erase
:
inline void ScopeTimerStaticCore::clearTimingForNamedScope(const ScopeTimer::ScopeSignature& scopeName) {
ScopesTiming& instance = getScopesTimingStaticInstance();
instance.erase(scopeName);
}
I also notice that these functions would get a lot shorter and simpler to read if you put their definitions in-line into the class body of ScopeTimerStaticCore
. In this case you could omit the keyword inline
and the qualification of the parameter type:
void clearTimingForNamedScope(const ScopeSignature& scopeName) {
ScopesTiming& instance = getScopesTimingStaticInstance();
instance.erase(scopeName);
}
(Assuming that ScopeTimerStaticCore
contains a member typedef using ScopeSignature = ScopeTimer::ScopeSignature;
, I guess. It probably should — or vice versa.)
$endgroup$
add a comment |
$begingroup$
using Duration = decltype(std::chrono::duration_cast<DurationType>(std::chrono::high_resolution_clock::now() - std::chrono::high_resolution_clock::now()).count());
I'm confused by this line. decltype(std::chrono::duration_cast<DurationType>(expr))
is invariably DurationType
, isn't it? That's why it's a "cast"? So this simplifies down to decltype(std::declval<DurationType&>().count())
, which I'm pretty sure can be spelled as DurationType::rep
unless you're really eager to support non-standard duration types that might not have a rep
member. So:
using Duration = typename DurationType::rep;
And now it appears that maybe Duration
is the wrong name for this typedef, eh?
static ScopesTiming& getScopesTimingStaticInstance() {
static ScopesTiming scopesTimingContainer;
return (scopesTimingContainer);
};
Minor nits on whitespace and naming and parentheses and trailing semicolons:
static ScopesTiming& getScopesTimingStaticInstance() {
static ScopesTiming instance;
return instance;
}
The defining quality of instance
is that it's a static instance of ScopesTiming
. If you want to convey the additional information that ScopesTiming
is actually a container type, then that information belongs in the name of the type. Personally I'd call it something like TimingVectorMap
, because it's a map of TimingVector
s.
Since the static map is not guarded by any mutex, your function addTimingToNamedScope
(which mutates the map) is not safe to call from multiple threads concurrently. This could be a problem for real-world use.
ScopeTimer
has two const
-qualified fields. This doesn't do anything except pessimize its implicitly generated move-constructor into a copy-constructor. I recommend removing the const
.
ScopeTimer
also has an implicit conversion from ScopeSignature
a.k.a. std::string
, so that for example
void f(ScopeTimer timer);
std::string hello = "hello world";
f(hello); // compiles without any diagnostic
I very strongly suggest that you never enable any implicit conversion unless you have a very good reason for it. This means putting explicit
on every constructor and conversion operator.
explicit ScopeTimer(const ScopeSignature& scopeName);
dumpfile.open(path, std::ios::out | std::ios::trunc);
Should you check to see if the open succeeded?
inline void ScopeTimerStaticCore::clearTimingForNamedScope(const ScopeTimer::ScopeSignature& scopeName) {
ScopesTiming& scopesTimingContainer = ScopeTimerStaticCore::getScopesTimingStaticInstance();
ScopesTiming::iterator it_scopes = scopesTimingContainer.find(scopeName);
if (it_scopes != scopesTimingContainer.end())
it_scopes->second.clear();
return ;
};
This would be a good place to use C++11 auto
:
inline void ScopeTimerStaticCore::clearTimingForNamedScope(const ScopeTimer::ScopeSignature& scopeName) {
ScopesTiming& instance = getScopesTimingStaticInstance();
auto it = instance.find(scopeName);
if (it != instance.end()) {
it->second.clear();
}
}
Or, if you don't mind removing the element from the map completely, you could just use erase
:
inline void ScopeTimerStaticCore::clearTimingForNamedScope(const ScopeTimer::ScopeSignature& scopeName) {
ScopesTiming& instance = getScopesTimingStaticInstance();
instance.erase(scopeName);
}
I also notice that these functions would get a lot shorter and simpler to read if you put their definitions in-line into the class body of ScopeTimerStaticCore
. In this case you could omit the keyword inline
and the qualification of the parameter type:
void clearTimingForNamedScope(const ScopeSignature& scopeName) {
ScopesTiming& instance = getScopesTimingStaticInstance();
instance.erase(scopeName);
}
(Assuming that ScopeTimerStaticCore
contains a member typedef using ScopeSignature = ScopeTimer::ScopeSignature;
, I guess. It probably should — or vice versa.)
$endgroup$
add a comment |
$begingroup$
using Duration = decltype(std::chrono::duration_cast<DurationType>(std::chrono::high_resolution_clock::now() - std::chrono::high_resolution_clock::now()).count());
I'm confused by this line. decltype(std::chrono::duration_cast<DurationType>(expr))
is invariably DurationType
, isn't it? That's why it's a "cast"? So this simplifies down to decltype(std::declval<DurationType&>().count())
, which I'm pretty sure can be spelled as DurationType::rep
unless you're really eager to support non-standard duration types that might not have a rep
member. So:
using Duration = typename DurationType::rep;
And now it appears that maybe Duration
is the wrong name for this typedef, eh?
static ScopesTiming& getScopesTimingStaticInstance() {
static ScopesTiming scopesTimingContainer;
return (scopesTimingContainer);
};
Minor nits on whitespace and naming and parentheses and trailing semicolons:
static ScopesTiming& getScopesTimingStaticInstance() {
static ScopesTiming instance;
return instance;
}
The defining quality of instance
is that it's a static instance of ScopesTiming
. If you want to convey the additional information that ScopesTiming
is actually a container type, then that information belongs in the name of the type. Personally I'd call it something like TimingVectorMap
, because it's a map of TimingVector
s.
Since the static map is not guarded by any mutex, your function addTimingToNamedScope
(which mutates the map) is not safe to call from multiple threads concurrently. This could be a problem for real-world use.
ScopeTimer
has two const
-qualified fields. This doesn't do anything except pessimize its implicitly generated move-constructor into a copy-constructor. I recommend removing the const
.
ScopeTimer
also has an implicit conversion from ScopeSignature
a.k.a. std::string
, so that for example
void f(ScopeTimer timer);
std::string hello = "hello world";
f(hello); // compiles without any diagnostic
I very strongly suggest that you never enable any implicit conversion unless you have a very good reason for it. This means putting explicit
on every constructor and conversion operator.
explicit ScopeTimer(const ScopeSignature& scopeName);
dumpfile.open(path, std::ios::out | std::ios::trunc);
Should you check to see if the open succeeded?
inline void ScopeTimerStaticCore::clearTimingForNamedScope(const ScopeTimer::ScopeSignature& scopeName) {
ScopesTiming& scopesTimingContainer = ScopeTimerStaticCore::getScopesTimingStaticInstance();
ScopesTiming::iterator it_scopes = scopesTimingContainer.find(scopeName);
if (it_scopes != scopesTimingContainer.end())
it_scopes->second.clear();
return ;
};
This would be a good place to use C++11 auto
:
inline void ScopeTimerStaticCore::clearTimingForNamedScope(const ScopeTimer::ScopeSignature& scopeName) {
ScopesTiming& instance = getScopesTimingStaticInstance();
auto it = instance.find(scopeName);
if (it != instance.end()) {
it->second.clear();
}
}
Or, if you don't mind removing the element from the map completely, you could just use erase
:
inline void ScopeTimerStaticCore::clearTimingForNamedScope(const ScopeTimer::ScopeSignature& scopeName) {
ScopesTiming& instance = getScopesTimingStaticInstance();
instance.erase(scopeName);
}
I also notice that these functions would get a lot shorter and simpler to read if you put their definitions in-line into the class body of ScopeTimerStaticCore
. In this case you could omit the keyword inline
and the qualification of the parameter type:
void clearTimingForNamedScope(const ScopeSignature& scopeName) {
ScopesTiming& instance = getScopesTimingStaticInstance();
instance.erase(scopeName);
}
(Assuming that ScopeTimerStaticCore
contains a member typedef using ScopeSignature = ScopeTimer::ScopeSignature;
, I guess. It probably should — or vice versa.)
$endgroup$
using Duration = decltype(std::chrono::duration_cast<DurationType>(std::chrono::high_resolution_clock::now() - std::chrono::high_resolution_clock::now()).count());
I'm confused by this line. decltype(std::chrono::duration_cast<DurationType>(expr))
is invariably DurationType
, isn't it? That's why it's a "cast"? So this simplifies down to decltype(std::declval<DurationType&>().count())
, which I'm pretty sure can be spelled as DurationType::rep
unless you're really eager to support non-standard duration types that might not have a rep
member. So:
using Duration = typename DurationType::rep;
And now it appears that maybe Duration
is the wrong name for this typedef, eh?
static ScopesTiming& getScopesTimingStaticInstance() {
static ScopesTiming scopesTimingContainer;
return (scopesTimingContainer);
};
Minor nits on whitespace and naming and parentheses and trailing semicolons:
static ScopesTiming& getScopesTimingStaticInstance() {
static ScopesTiming instance;
return instance;
}
The defining quality of instance
is that it's a static instance of ScopesTiming
. If you want to convey the additional information that ScopesTiming
is actually a container type, then that information belongs in the name of the type. Personally I'd call it something like TimingVectorMap
, because it's a map of TimingVector
s.
Since the static map is not guarded by any mutex, your function addTimingToNamedScope
(which mutates the map) is not safe to call from multiple threads concurrently. This could be a problem for real-world use.
ScopeTimer
has two const
-qualified fields. This doesn't do anything except pessimize its implicitly generated move-constructor into a copy-constructor. I recommend removing the const
.
ScopeTimer
also has an implicit conversion from ScopeSignature
a.k.a. std::string
, so that for example
void f(ScopeTimer timer);
std::string hello = "hello world";
f(hello); // compiles without any diagnostic
I very strongly suggest that you never enable any implicit conversion unless you have a very good reason for it. This means putting explicit
on every constructor and conversion operator.
explicit ScopeTimer(const ScopeSignature& scopeName);
dumpfile.open(path, std::ios::out | std::ios::trunc);
Should you check to see if the open succeeded?
inline void ScopeTimerStaticCore::clearTimingForNamedScope(const ScopeTimer::ScopeSignature& scopeName) {
ScopesTiming& scopesTimingContainer = ScopeTimerStaticCore::getScopesTimingStaticInstance();
ScopesTiming::iterator it_scopes = scopesTimingContainer.find(scopeName);
if (it_scopes != scopesTimingContainer.end())
it_scopes->second.clear();
return ;
};
This would be a good place to use C++11 auto
:
inline void ScopeTimerStaticCore::clearTimingForNamedScope(const ScopeTimer::ScopeSignature& scopeName) {
ScopesTiming& instance = getScopesTimingStaticInstance();
auto it = instance.find(scopeName);
if (it != instance.end()) {
it->second.clear();
}
}
Or, if you don't mind removing the element from the map completely, you could just use erase
:
inline void ScopeTimerStaticCore::clearTimingForNamedScope(const ScopeTimer::ScopeSignature& scopeName) {
ScopesTiming& instance = getScopesTimingStaticInstance();
instance.erase(scopeName);
}
I also notice that these functions would get a lot shorter and simpler to read if you put their definitions in-line into the class body of ScopeTimerStaticCore
. In this case you could omit the keyword inline
and the qualification of the parameter type:
void clearTimingForNamedScope(const ScopeSignature& scopeName) {
ScopesTiming& instance = getScopesTimingStaticInstance();
instance.erase(scopeName);
}
(Assuming that ScopeTimerStaticCore
contains a member typedef using ScopeSignature = ScopeTimer::ScopeSignature;
, I guess. It probably should — or vice versa.)
answered 3 hours ago
QuuxplusoneQuuxplusone
11.9k12061
11.9k12061
add a comment |
add a comment |
shorty_ponton is a new contributor. Be nice, and check out our Code of Conduct.
shorty_ponton is a new contributor. Be nice, and check out our Code of Conduct.
shorty_ponton is a new contributor. Be nice, and check out our Code of Conduct.
shorty_ponton is a new contributor. Be nice, and check out our Code of Conduct.
Thanks for contributing an answer to Code Review Stack Exchange!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
Use MathJax to format equations. MathJax reference.
To learn more, see our tips on writing great answers.
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f211857%2fa-header-only-class-for-c-time-measurement%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown