Skip to content

bench: add logging benchmarks for LogPrintLevel and LogPrintfCategory#25586

Closed
jonatack wants to merge 4 commits intobitcoin:masterfrom
jonatack:add-logging-benchmarks
Closed

bench: add logging benchmarks for LogPrintLevel and LogPrintfCategory#25586
jonatack wants to merge 4 commits intobitcoin:masterfrom
jonatack:add-logging-benchmarks

Conversation

@jonatack
Copy link
Member

@jonatack jonatack commented Jul 11, 2022 •
edited
Loading

LogPrintLevel and LogPrintfCategory were recently added to the logging macros in #24464 and #25306, respectively. Our logging is expected to primarily use these methods, so it makes sense to benchmark them.

This pull adds benchmarks for both of them to the logging benchmarks, improves the existing benchmark naming to be clearer and with a more coherent output order, and sorts the benchmark code in that order.

$ NANOBENCH_SUPPRESS_WARNINGS=1 ./src/bench/bench_bitcoin -filter=Log*.*

before

|               ns/op |                op/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|               75.82 |       13,189,445.28 |    5.3% |      0.01 | `LoggingNoCategory`
|            1,344.86 |          743,571.82 |   19.8% |      0.01 | `LoggingNoFile`
|           15,341.11 |           65,184.34 |   19.6% |      0.01 | `LoggingNoThreadNames`
|           22,790.48 |           43,877.96 |   36.3% |      0.01 | `LoggingYoCategory`
|           22,972.32 |           43,530.65 |    7.4% |      0.01 | `LoggingYoThreadNames`

after

$ NANOBENCH_SUPPRESS_WARNINGS=1 ./src/bench/bench_bitcoin -filter=Log*.*

|               ns/op |                op/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|            2,497.81 |          400,349.92 |   25.1% |      0.01 | `LogNoDebugLogFile`
|           30,758.83 |           32,510.99 |   18.8% |      0.01 | `LogPrintLevelWithThreadNames`
|           25,559.50 |           39,124.40 |    7.1% |      0.01 | `LogPrintLevelWithoutThreadNames`
|           22,128.45 |           45,190.70 |    2.2% |      0.01 | `LogPrintWithCategory`
|           21,888.91 |           45,685.23 |    6.3% |      0.01 | `LogPrintWithoutCategory`
|           28,293.11 |           35,344.29 |    4.9% |      0.01 | `LogPrintfCategoryWithThreadNames`
|           24,329.70 |           41,102.03 |    7.3% |      0.01 | `LogPrintfCategoryWithoutThreadNames`
|           26,814.71 |           37,292.97 |    6.9% |      0.01 | `LogPrintfWithThreadNames`
|           18,139.46 |           55,128.43 |   20.9% |      0.01 | `LogPrintfWithoutThreadNames`

@jonatack jonatack force-pushed the add-logging-benchmarks branch from 478272d to f896ef9 Compare July 11, 2022 11:48
@jonatack jonatack changed the title bench: add logging benchmarks for LogPrintLevel and LogPrintfCategory bench: add benchmarks for LogPrintLevel, LogPrintfCategory, and lock contention logging Jul 11, 2022
@jonatack jonatack force-pushed the add-logging-benchmarks branch from f896ef9 to 147f479 Compare July 11, 2022 11:58
@jonatack jonatack changed the title bench: add benchmarks for LogPrintLevel, LogPrintfCategory, and lock contention logging bench: add logging benchmarks for LogPrintLevel and LogPrintfCategory Jul 11, 2022
@DrahtBot DrahtBot added the Tests label Jul 11, 2022
@fanquake
Copy link
Member

fanquake commented Jul 20, 2022

~0 - I'm not really convinced we need more benchmarks for logging code.

@jonatack
Copy link
Member Author

jonatack commented Aug 5, 2022

This change makes sense given that the codebase is migrating to these methods but closing for now, will try again later.

@jonatack jonatack closed this Aug 5, 2022
@jonatack jonatack mentioned this pull request Aug 22, 2022
Merged
@bitcoin bitcoin locked and limited conversation to collaborators Aug 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

Tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jonatack @fanquake @DrahtBot