optimization: Preallocate addresses in GetAddr based on nNodes #29608

pull l0rinc wants to merge 1 commits into bitcoin:master from l0rinc:paplorinc/addrman-get-address-preallocate changing 1 files +1 −0
  1. l0rinc commented at 7:58 pm on March 9, 2024: contributor

    The upper bound ensures efficient memory usage based on the input constraints.

    before:

    0|               ns/op |                op/s |    err% |     total | benchmark
1|--------------------:|--------------------:|--------:|----------:|:----------
2|           76,852.79 |           13,011.89 |    0.4% |      1.07 | `AddrManGetAddr`
3|           76,598.21 |           13,055.14 |    0.2% |      1.07 | `AddrManGetAddr`
4|           76,296.32 |           13,106.79 |    0.1% |      1.07 | `AddrManGetAddr`

    after:

    0|               ns/op |                op/s |    err% |     total | benchmark
1|--------------------:|--------------------:|--------:|----------:|:----------
2|           65,966.97 |           15,159.10 |    0.3% |      1.07 | `AddrManGetAddr`
3|           66,075.40 |           15,134.23 |    0.2% |      1.06 | `AddrManGetAddr`
4|           66,306.34 |           15,081.51 |    0.3% |      1.06 | `AddrManGetAddr`
  2. DrahtBot commented at 7:58 pm on March 9, 2024: contributor

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

    Code Coverage

    For detailed information about the code coverage, see the test coverage report.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK stickies-v, vasild

    If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

  3. l0rinc renamed this:
    refactorÉ Preallocate addresses in GetAddr_ based on nNodes
    refactor: Preallocate addresses in GetAddr_ based on nNodes
    on Mar 9, 2024
  4. DrahtBot added the label Refactoring on Mar 9, 2024
  5. l0rinc force-pushed on Mar 9, 2024
  6. l0rinc renamed this:
    refactor: Preallocate addresses in GetAddr_ based on nNodes
    refactor: Preallocate addresses in GetAddr based on nNodes
    on Mar 9, 2024
  7. l0rinc force-pushed on Mar 9, 2024
  8. l0rinc marked this as ready for review on Mar 9, 2024
  9. brunoerg commented at 10:11 pm on March 9, 2024: contributor
    Perhaps It would be better to cherry-pick this change into #29578 since that touches it?
  10. l0rinc commented at 10:16 pm on March 9, 2024: contributor
    @brunoerg, feel free to do that, I’ll close this if you cherry-pick it to the other PR (please provide before/after measurements)
  11. vasild approved
  12. vasild commented at 2:13 pm on March 11, 2024: contributor

    ACK 278a1ee27bc760d3eb00069327d4b8f49950f6b6

    Note that this may overshoot and allocate more space than is going to be needed. Less so with #29578.

  13. brunoerg commented at 2:42 pm on March 11, 2024: contributor

    @brunoerg, feel free to do that, I’ll close this if you cherry-pick it to the other PR (please provide before/after measurements)

    Done.

  14. l0rinc commented at 2:46 pm on March 11, 2024: contributor
    Thanks for the review @vasild, we’ve moved it over to the other PR
  15. l0rinc closed this on Mar 11, 2024
  16. l0rinc deleted the branch on Mar 11, 2024
  17. l0rinc restored the branch on Jun 23, 2024
  18. l0rinc reopened this on Jun 23, 2024
  19. l0rinc commented at 4:25 pm on June 23, 2024: contributor
    Reopening since #29578 was closed without merge
  20. l0rinc force-pushed on Jun 23, 2024
  21. l0rinc renamed this:
    refactor: Preallocate addresses in GetAddr based on nNodes
    optimization: Preallocate addresses in GetAddr based on nNodes
    on Jun 23, 2024
  22. l0rinc requested review from vasild on Jul 1, 2024
  23. Preallocate addresses in GetAddr based on nNodes 
    > make && ./src/bench/bench_bitcoin --filter=AddrManGetAddr --min-time=1000

Before:
```
|               ns/op |                op/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|           76,852.79 |           13,011.89 |    0.4% |      1.07 | `AddrManGetAddr`
|           76,598.21 |           13,055.14 |    0.2% |      1.07 | `AddrManGetAddr`
|           76,296.32 |           13,106.79 |    0.1% |      1.07 | `AddrManGetAddr`
```
After:
```
|               ns/op |                op/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|           65,966.97 |           15,159.10 |    0.3% |      1.07 | `AddrManGetAddr`
|           66,075.40 |           15,134.23 |    0.2% |      1.06 | `AddrManGetAddr`
|           66,306.34 |           15,081.51 |    0.3% |      1.06 | `AddrManGetAddr`
```
    66082ca348
  24. l0rinc force-pushed on Jul 1, 2024
  25. achow101 requested review from sipa on Oct 15, 2024
  26. stickies-v approved
  27. stickies-v commented at 3:13 pm on October 15, 2024: contributor

    ACK 66082ca3488e7ad78149e05631dccd09be03c961

    I don’t think this is a particularly impactful change but it’s very small and straightforward to review.

  28. vasild approved
  29. vasild commented at 12:00 pm on October 25, 2024: contributor
    ACK 66082ca3488e7ad78149e05631dccd09be03c961
  30. fanquake commented at 1:40 pm on October 25, 2024: member

    I don’t think this is a particularly impactful change but it’s very small and straightforward to review.

    I agree, and I think we are probably overdue for a review of what is in src/bench, and cleaning out benchmarks that aren’t (very) meaningful. IIRC clang-tidy has a check for exactly this kind of change, so I think if any more were going to be made, they could be done using that.

  31. fanquake merged this on Oct 25, 2024
  32. fanquake closed this on Oct 25, 2024
  33. l0rinc deleted the branch on Oct 25, 2024
  34. fanquake commented at 1:55 pm on October 25, 2024: member
    The clang-tidy check I was thinking of was https://clang.llvm.org/extra/clang-tidy/checks/performance/inefficient-vector-operation.html.


l0rinc DrahtBot brunoerg vasild stickies-v fanquake


sipa

Labels
Refactoring

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-12-22 09:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me