logging: bypass timestamp formatting when not logging#12970
Merged
sipa merged 1 commit intobitcoin:masterfrom Apr 13, 2018
Merged
logging: bypass timestamp formatting when not logging#12970sipa merged 1 commit intobitcoin:masterfrom
sipa merged 1 commit intobitcoin:masterfrom
Conversation
Member
|
utACK 8ca1f6ecba5efc278927fd34eb04f6a4254dfe03 |
Member
Author
|
Wine i686 build down to 11 min :) |
Member
|
utACK 8ca1f6e (Run time seems to be about the same or slightly faster than with the wine-hq) |
Member
Author
|
While this is an easy win for tests, we should still track down why LogTimestampStr() is so painful. |
Member
It looks inefficient too: std::string DateTimeStrFormat(const char* pszFormat, int64_t nTime)
{
static std::locale classic(std::locale::classic());
// std::locale takes ownership of the pointer
std::locale loc(classic, new boost::posix_time::time_facet(pszFormat));
std::stringstream ss;
ss.imbue(loc);
ss << boost::posix_time::from_time_t(nTime);
return ss.str();
} |
laanwj
reviewed
Apr 12, 2018
src/util.cpp
Outdated
Member
There was a problem hiding this comment.
nit: might be even more efficient to do this check in util.h already, before calling tfm::format.
This leads to massive speedups under Wine.
Contributor
|
utACK 339730a |
Member
|
utACK 339730a |
Member
|
utACK 339730a |
sipa
added a commit
that referenced
this pull request
Apr 13, 2018
339730a logging: bypass timestamp formatting when not logging (Cory Fields) Pull request description: As suggested by @laanwj on IRC: ``` <cfields> whoa <cfields> Leaving test case "knapsack_solver_test"; testing time: 358694ms <cfields> i386 + old wine ^^ <cfields> Leaving test case "knapsack_solver_test"; testing time: 6781ms <cfields> ^^ same, but with the LogPrint commented out ... <wumpus> if both log-to-file and log-to-console is disabled, it should probably bypass all logging ``` Edit: The painful line commented out being the LogPrintf in CWallet::AddToWallet. Tree-SHA512: bc6da67dcdf05e9164fff7a7e9980de897e6f1b0d3f6e1ebde2162cbcba7d54a6ec94283534eb5a1ebde7134533d7fe7e496aa35ea3128c567ed6483eae5212c
Contributor
|
utACK 339730a Impressive! Very nice find! |
laanwj
added a commit
that referenced
this pull request
Apr 14, 2018
1527015 Avoid std::locale/imbue in DateTimeStrFormat (Pieter Wuille) Pull request description: And replace them with just hardcoded ISO8601 strings and `gmtime_r`. Pointed out by @laanwj here: #12970 (comment) Tree-SHA512: a459758b42ca56f8462115aefe8e6377c1319fce509ea64dbb767f3f087c9b848335954cb684e5896c38008847684045505a3e1559fb3e83b8e80e10b003d1e7
maflcko
pushed a commit
to maflcko/bitcoin-core
that referenced
this pull request
May 29, 2018
This leads to massive speedups under Wine. Github-Pull: bitcoin#12970 Rebased-From: 339730a
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Jun 27, 2020
…gging 339730a logging: bypass timestamp formatting when not logging (Cory Fields) Pull request description: As suggested by @laanwj on IRC: ``` <cfields> whoa <cfields> Leaving test case "knapsack_solver_test"; testing time: 358694ms <cfields> i386 + old wine ^^ <cfields> Leaving test case "knapsack_solver_test"; testing time: 6781ms <cfields> ^^ same, but with the LogPrint commented out ... <wumpus> if both log-to-file and log-to-console is disabled, it should probably bypass all logging ``` Edit: The painful line commented out being the LogPrintf in CWallet::AddToWallet. Tree-SHA512: bc6da67dcdf05e9164fff7a7e9980de897e6f1b0d3f6e1ebde2162cbcba7d54a6ec94283534eb5a1ebde7134533d7fe7e496aa35ea3128c567ed6483eae5212c
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Apr 13, 2021
…rmat 1527015 Avoid std::locale/imbue in DateTimeStrFormat (Pieter Wuille) Pull request description: And replace them with just hardcoded ISO8601 strings and `gmtime_r`. Pointed out by @laanwj here: bitcoin#12970 (comment) Tree-SHA512: a459758b42ca56f8462115aefe8e6377c1319fce509ea64dbb767f3f087c9b848335954cb684e5896c38008847684045505a3e1559fb3e83b8e80e10b003d1e7
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Apr 18, 2021
…rmat 1527015 Avoid std::locale/imbue in DateTimeStrFormat (Pieter Wuille) Pull request description: And replace them with just hardcoded ISO8601 strings and `gmtime_r`. Pointed out by @laanwj here: bitcoin#12970 (comment) Tree-SHA512: a459758b42ca56f8462115aefe8e6377c1319fce509ea64dbb767f3f087c9b848335954cb684e5896c38008847684045505a3e1559fb3e83b8e80e10b003d1e7
gades
pushed a commit
to cosanta/cosanta-core
that referenced
this pull request
Jun 30, 2021
…gging 339730a logging: bypass timestamp formatting when not logging (Cory Fields) Pull request description: As suggested by @laanwj on IRC: ``` <cfields> whoa <cfields> Leaving test case "knapsack_solver_test"; testing time: 358694ms <cfields> i386 + old wine ^^ <cfields> Leaving test case "knapsack_solver_test"; testing time: 6781ms <cfields> ^^ same, but with the LogPrint commented out ... <wumpus> if both log-to-file and log-to-console is disabled, it should probably bypass all logging ``` Edit: The painful line commented out being the LogPrintf in CWallet::AddToWallet. Tree-SHA512: bc6da67dcdf05e9164fff7a7e9980de897e6f1b0d3f6e1ebde2162cbcba7d54a6ec94283534eb5a1ebde7134533d7fe7e496aa35ea3128c567ed6483eae5212c
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
As suggested by @laanwj on IRC:
Edit: The painful line commented out being the LogPrintf in CWallet::AddToWallet.