stacks-core icon indicating copy to clipboard operation
stacks-core copied to clipboard

chore(stacks-common): Improvements to `Vec` allocation and operations

Open jbencin opened this issue 1 year ago • 1 comments

Description

Similar to #4408, but for ./stacks-common. This PR also removes unnecessary intermediate Vecs used by Vec::append() by using Vec::extend_from_slice() instead

This gives a significant improvement of around 5%

❯ hyperfine -w 3 -r 10 "stacks-core/target/release/stacks-inspect.next replay-block /home/jbencin/data/next/ range 99990 100000" "stacks-core/target/release/stacks-inspect.vec-allocation-stacks-common replay-block /home/jbencin/data/next/ range 99990 100000"
Benchmark 1: stacks-core/target/release/stacks-inspect.next replay-block /home/jbencin/data/next/ range 99990 100000
  Time (mean ± σ):      9.044 s ±  0.020 s    [User: 8.556 s, System: 0.442 s]
  Range (min … max):    9.022 s …  9.074 s    10 runs
 
Benchmark 2: stacks-core/target/release/stacks-inspect.vec-allocation-stacks-common replay-block /home/jbencin/data/next/ range 99990 100000
  Time (mean ± σ):      8.649 s ±  0.040 s    [User: 8.157 s, System: 0.441 s]
  Range (min … max):    8.597 s …  8.724 s    10 runs
 
Summary
  stacks-core/target/release/stacks-inspect.vec-allocation-stacks-common replay-block /home/jbencin/data/next/ range 99990 100000 ran
    1.05 ± 0.01 times faster than stacks-core/target/release/stacks-inspect.next replay-block /home/jbencin/data/next/ range 99990 100000

Applicable issues

  • #4316

Additional info (benefits, drawbacks, caveats)

Same as #4408

Checklist

  • [ ] Test coverage for new or modified code paths
  • [ ] Changelog is updated
  • [ ] Required documentation changes (e.g., docs/rpc/openapi.yaml and rpc-endpoints.md for v2 endpoints, event-dispatcher.md for new events)
  • [ ] New clarity functions have corresponding PR in clarity-benchmarking repo
  • [ ] New integration test(s) added to bitcoin-tests.yml

jbencin avatar Feb 23 '24 21:02 jbencin

Codecov Report

Attention: Patch coverage is 78.33333% with 13 lines in your changes are missing coverage. Please review.

Project coverage is 78.48%. Comparing base (d72ad44) to head (16bd17c).

Additional details and impacted files
@@            Coverage Diff             @@
##             next    #4425      +/-   ##
==========================================
- Coverage   83.12%   78.48%   -4.64%     
==========================================
  Files         448      448              
  Lines      323925   323930       +5     
==========================================
- Hits       269255   254239   -15016     
- Misses      54670    69691   +15021     
Files Coverage Δ
stacks-common/src/address/c32.rs 97.88% <100.00%> (+0.04%) :arrow_up:
stacks-common/src/deps_common/bech32/mod.rs 80.74% <100.00%> (-12.29%) :arrow_down:
...common/src/deps_common/bitcoin/blockdata/script.rs 78.58% <100.00%> (-1.87%) :arrow_down:
stacks-common/src/deps_common/bitcoin/util/hash.rs 73.15% <100.00%> (-13.38%) :arrow_down:
stacks-common/src/util/hash.rs 81.16% <100.00%> (ø)
stacks-common/src/types/chainstate.rs 82.18% <88.88%> (-0.56%) :arrow_down:
...n/src/deps_common/bitcoin/blockdata/transaction.rs 91.35% <58.62%> (-3.21%) :arrow_down:

... and 174 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d72ad44...16bd17c. Read the comment docs.

codecov[bot] avatar Feb 23 '24 22:02 codecov[bot]

Benchmark Reference showcasing the increase performance of for_loop_iter with append instead of map and flat_map related to https://github.com/stacks-network/stacks-core/pull/4425/commits/fe4be97ad3911370916620cd3ecefd222535c8a6 .

ASuciuX avatar Feb 27 '24 13:02 ASuciuX