[logs] Fix a few log messages related to duration measurement #14358

pull romanz wants to merge 1 commits into bitcoin:master from romanz:log_fix changing 4 files +9 −9

romanz commented at 3:33 PM on September 30, 2018: contributor
- Remove fixed-width formatting for the duration value (%15d -> %d), for better consistency with other similar logging statements.
- Correct the logging statement that starts with a space (" block index %15dms\n") to start with a verb.
- Add " in " between the operation description and the millisecond duration.
DrahtBot commented at 3:38 PM on September 30, 2018: member


The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #14711 (Remove uses of chainActive and mapBlockIndex in wallet code by ryanofsky)
- #13582 (Extract AppInitLoadBlockIndex from AppInitMain by Empact)
- #10973 (Refactor: separate wallet from node by ryanofsky)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
romanz force-pushed on Sep 30, 2018
romanz force-pushed on Sep 30, 2018
romanz commented at 7:09 PM on September 30, 2018: contributor

Note: Travis and AppVeyor fail due to seemingly unrelated issues: https://ci.appveyor.com/project/DrahtBot/bitcoin/build/master.1848?fullLog=true https://travis-ci.org/bitcoin/bitcoin/jobs/435323308
promag commented at 12:26 AM on October 1, 2018: member

Not sure what this is fixing. NACK.
MarcoFalke added the label Docs on Oct 1, 2018
MarcoFalke added the label Utils/log/libs on Oct 1, 2018
MarcoFalke removed the label Docs on Oct 1, 2018
romanz commented at 5:08 AM on October 1, 2018: contributor

Updated the PR's description to contain more information about the issues it fixes. Should I split it into separate commits?
romanz commented at 8:39 AM on October 1, 2018: contributor

Also, I'm not sure how to test it (other than manually running bitcoind and inspecting the logs)...
promag commented at 9:42 AM on October 2, 2018: member

How about this one: https://github.com/bitcoin/bitcoin/blob/1e8f88e071019907785b260477bd359bef6f9a8f/src/validation.cpp#L2312

This only benefits the human eye as the durations can be extracted either way. Consistency is a plus though.

Retracted from NACK to -0.
romanz commented at 7:28 PM on October 2, 2018: contributor

Good suggestion! Will update the PR in the following days.
romanz commented at 7:53 AM on October 5, 2018: contributor

I have updated (most) the benchmarking-related code in this PR to be consistent with the format used at src/validation.cpp, except the code at src/wallet/wallet.cpp (since it uses CWallet::WalletLogPrintf()).
romanz commented at 7:01 AM on October 16, 2018: contributor

@promag Ping :)
in src/init.cpp:1511 in 7990bb5b61 outdated
```
1507 | @@ -1508,7 +1508,7 @@ bool AppInitMain()
1508 |              }
1509 |  
1510 |              fLoaded = true;
1511 | -            LogPrintf(" block index %15dms\n", GetTimeMillis() - load_block_index_start_time);
1512 | +            LogPrint(BCLog::BENCH, "- Loaded block index: %dms\n", GetTimeMillis() - load_block_index_start_time);
```
promag commented at 8:39 AM on October 16, 2018:

@romanz I didn't meant to change the log category. I think some of these messages must/should be present in the log regardless of the enabled logging categories.

romanz commented at 9:23 AM on October 16, 2018:

Sounds good, I'll switch back to using LogPrintf().

Just a small question: are the new log messages fine? If not, please let me know so I would fix them :)
romanz force-pushed on Oct 17, 2018
romanz commented at 2:58 PM on October 19, 2018: contributor

Updated the PR to use the original logging categories.

[logs] Fix a few log messages related to duration measurement

Also, remove fixed-width formatting for the duration value ('%15d' -> '%d')

036d36fd73

romanz force-pushed on Nov 10, 2018
romanz commented at 7:49 PM on November 10, 2018: contributor

Ping (rebased on latest master edc715240).
romanz commented at 12:45 PM on December 1, 2018: contributor

@promag Please let me know if there is a problem with this PR.
romanz closed this on Jan 12, 2019
DrahtBot locked this on Dec 16, 2021

Contributors

Labels

[logs] Fix a few log messages related to duration measurement #14358

Conflicts