Page MenuHomePhabricator

util: implement noexcept move assignment & move ctor for prevector
ClosedPublic

Authored by PiRK on Nov 11 2024, 10:56.

Details

Summary

prevector's move assignment and move constructor were not noexcept, which makes it inefficient to use inside STL containers like std::vector. That's the case e.g. for CScriptCheck. This PR adds noexcept, and also implements the move assignment & ctor, which makes it quite a bit more efficient to use prevector in an std::vector.

The PR also adds a benchmark which grows an std::vector by adding prevector objects to it.

Adds a few static_asserts so CScriptCheck stays is_nothrow_move_assignable, is_nothrow_move_constructible, and is_nothrow_destructible

As can be seen, mostly thanks to just noexcept the benchmark becomes about 2 times faster because std::vector can now use move operations instead of having to fall back to copy everything

Before:

|               ns/op |                op/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|            7,518.17 |          133,011.17 |    0.3% |      0.01 | `PrevectorFillVectorDirectNontrivial`
|              964.99 |        1,036,278.31 |    0.9% |      0.01 | `PrevectorFillVectorDirectTrivial`
|           21,728.31 |           46,022.91 |    0.1% |      0.01 | `PrevectorFillVectorIndirectNontrivial`
|           10,878.14 |           91,927.52 |    0.9% |      0.01 | `PrevectorFillVectorIndirectTrivial`

After:

|               ns/op |                op/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|            3,659.77 |          273,241.27 |    0.2% |      0.01 | `PrevectorFillVectorDirectNontrivial`
|              655.16 |        1,526,333.89 |    0.5% |      0.01 | `PrevectorFillVectorDirectTrivial`
|            7,934.76 |          126,027.76 |    0.3% |      0.01 | `PrevectorFillVectorIndirectNontrivial`
|            3,050.25 |          327,841.59 |    0.2% |      0.01 | `PrevectorFillVectorIndirectTrivial`

This is a backport of core#27334

Test Plan

ninja all check-all

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

PiRK requested review of this revision.Nov 11 2024, 10:56
Fabien added a subscriber: Fabien.
Fabien added inline comments.
src/validation.h
562 ↗(On Diff #50773)

should be in assumptions.h ? Not blocking but maybe a follow-up

This revision is now accepted and ready to land.Nov 11 2024, 13:15
src/validation.h
562 ↗(On Diff #50773)

assumptions.h currently only deals with external/standard types. We probably don't want to add a dependency to CScriptCheck to it