Decide the mode once per operation, and swap without three container moves - #81
Merged
Conversation
…moves Two changes, both about doing the same bookkeeping fewer times. The single byte of overhead in direct mode is untouched: svector<uint8_t, 1> is still 8 bytes holding 7. emplace_back asked which mode it was in three times. Constructing the element writes through a T*, which the compiler cannot prove does not alias our own bytes, so the is_direct() behind set_size() was a fresh load and the indirect() behind it a fresh pointer chase. The generated loop reloaded the tag byte and the storage pointer after every single append. Deciding once and carrying the answer costs 15.3 instructions per push_back where it used to cost 19.3. swap went through std::swap(*this, other), which is three whole container moves, and in direct mode a container move is every element moved and the original destroyed. Two svector<std::string, 7> cost 607 instructions to exchange. Now two indirect svectors exchange their pointers and touch no element at all, two direct ones exchange elements in place, and a mixed pair relocates only the inline side. 285 instructions for the same swap, and the int case gets faster too by keeping the whole-buffer path that #54 measured. There was also no free swap(), so std::swap and everything built on it got the generic three move version rather than the member. There is one now. The noexcept condition on swap widens from is_nothrow_move_constructible_v<T> to that and is_nothrow_swappable_v<T>, because exchanging elements in place is a swap and no longer only a move construction. Both traits hold for every type where the old one did; noexcept_contract.cpp still passes unchanged. Measured on gcc 16 -O3 -DNDEBUG, one workload per process, medians: push_back uint8_t 19.32 -> 15.33 ins 0.68 -> 0.65 ns swap direct std::string 607 -> 285 ins 23.65 -> 15.71 ns swap direct int 21 -> 27 ins 2.52 -> 1.79 ns swap indirect std::string 27 -> 23 ins 1.37 -> 1.11 ns emplace_back std::string 88.83 -> 82.83 ins insert at random index 11554 -> 11048 ins 767 -> 720 ns Nothing else moved. shuffle_sort's instruction count is identical to the byte, so the 2% on its clock is where the linker happened to put things -- which is also why every number above comes from a binary containing one workload. In a binary holding four containers over nine workloads, touching emplace_back changes 145 of 163 functions, std::sort's own instantiation among them, and the noise swamps the signal. Tests: swap now has a case per mode combination rather than one path, so swap.cpp walks every combination in both size orders, swaps a vector with itself, and covers the one hazard the old suite could not see -- an element type whose alignment is below sizeof(void*), whose inline storage therefore starts inside the bytes the indirect pointer occupies. Every existing element type is 8 aligned, which hides whether swap reads that pointer before or after moving elements onto it. Reordering those two lines now hangs the suite; before, it passed. Benchmarks: bench-solo gains swap_string, swap_int and build_inline, the last being the case the library exists for and the one nothing measured. It reads the elements back on purpose: with a trivially destructible element and nothing reading it, the optimizer deletes an inline vector outright, and the benchmark measures how deletable each implementation is rather than how fast. It says svector builds a 7 element inline vector in a third of what std::vector needs, and about twice what absl needs -- worth a look later. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Cleanup pass over the two optimizations, plus the benchmarks and README the user asked for. No behaviour change beyond the one noted below. svector.h: * the both-indirect swap called set_indirect twice, which re-reads the byte it just wrote to check the low bit. Both pointers came straight off live indirect svectors, so that bit was already known. 29 -> 25 instructions, 1.34 -> 1.22 ns on the fastest path swap has * swap()'s doc quoted 607 -> 285 instructions as though it held generally. It was measured on strings that own a buffer. Short strings go the other way and the comment now says so, with the numbers for both * a note on why swap_direct does not call uninitialized_move_and_destroy, which is the obvious cleanup and was suggested twice. Its memcpy specialisation becomes an out of line call: on svector<uint64_t, 40>, which lands exactly there, 70 instructions against 157 and 8.93 ns against 5.69 Tests: the three mode-combination cases were one body three times, so they are one helper called three times, and the file shrank while the assertion count went up. That refactor also introduced and then caught a real hole: the helper had switched to std::swap(a, b), which is qualified, so argument dependent lookup never runs and it exercised the generic three move template instead of svector::swap. The mutation that used to hang the suite passed. It calls the member again, and a new case pins the free function by counting move assignments -- three container moves only ever move construct, so a non-zero moveAssign is proof the ADL overload ran. Benchmarks: swap_string_long joins swap_string, because the two string lengths are swapped by completely different code in libstdc++ and svector lands on opposite sides of the comparison for them. Publishing only the short one would have shown a regression as the headline. README: every number re-measured on the current code, one workload per process, medians of 9 with the noisy ones re-checked over 15. Two new charts. The section now opens with the case the library exists for -- a vector that never allocates -- rather than with push_back, where a tagged pointer can only lose. That ordering was not a decision anyone made; it fell out of the last rewrite. What the numbers say now: svector is 2.2x behind std::vector on push_back of a trivial type, 18% on a subscript, and 2.6x behind absl at filling inline storage. It is the fastest of the four at emplace_back of strings and at inserting them at the front, the fastest of the three inline containers at swapping, and no worse than any of them at sorting. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two optimizations, both about doing the same bookkeeping fewer times. The single byte of overhead in direct mode is untouched —
svector<uint8_t, 1>is still 8 bytes holding 7, checked bystatic_assert.emplace_backasked which mode it was in three timesConstructing the element writes through a
T*, which the compiler cannot prove does not alias our own bytes. So theis_direct()behindset_size()was a fresh load, and theindirect()behind it a fresh pointer chase — the generated loop reloaded the tag byte and the storage pointer after every append. Deciding once: 19.32 → 15.33 instructions perpush_back.swapdid three whole container movesstd::swap(*this, other)is a move construction and two move assignments, and in direct mode a container move is every element moved and the original destroyed. Now each mode combination does the least it can: two indirect svectors exchange their two pointers and touch no element, two direct ones exchange elements in place, a mixed pair relocates only the inline side. A trivially copyable element keeps the whole-buffer path #54 measured.There was also no free
swap(), so unqualifiedswap(a, b)— the form every standard algorithm uses internally — got the generic three-move version. There is one now.Measured against the three-move version, ns per swap, inline capacity 7:
std::stringuint64_tstd::stringOne case is worse and it is worth stating plainly. For strings short enough to live inside themselves,
std::swap_rangesfindsstd::string::swap, and that is three copies of the internal buffer where a longer string would have exchanged one pointer. That is libstdc++'s trade, not one this code can pick around, and it buys the other five.swap_string_longwas added alongsideswap_stringso both are visible — publishing only the short-string case would have shown a regression as the headline.Also in here
set_indirecttwice, which re-reads the byte it just wrote to check the low bit. Both pointers came off live indirect svectors, so that bit was known: 29 → 25 instructions, 1.34 → 1.22 ns.swap_directdoes not call the existinguninitialized_move_and_destroy. It is the obvious cleanup and was suggested twice in review; itsmemcpyspecialization becomes an out-of-line call, and onsvector<uint64_t, 40>— which lands exactly there — it is 70 instructions against 157 and 8.93 ns against 5.69.A hole in the tests, introduced and caught
Deduplicating the three mode-combination cases into one helper, I switched from
a.swap(b)tostd::swap(a, b). That is a qualified call, so ADL never runs and it tested the generic three-move template rather thansvector::swap. The mutation that used to hang the suite started passing.Fixed to call the member, plus a new case that pins the free function by counting move-assignments: three container moves only ever move-construct, so a non-zero
moveAssignproves the ADL overload ran. The suite also covers the one hazard structurally invisible before — an element type whose alignment is belowsizeof(void*), whose inline storage therefore starts inside the bytes the indirect pointer occupies. Reordering those two lines hangs the suite again.113 cases / 634,643 assertions green on gcc, clang, C++20, ASAN+UBSAN and the strict release build, plus the fuzzing corpus.
README
Every number re-measured on the current code, one workload per process, medians of 9 with the noisy rows re-checked over 15. Two new charts.
The section now opens with a vector that never allocates rather than with
push_back. That ordering was not a decision anyone made — it fell out of the previous rewrite, and it led with the two benchmarks where a tagged pointer can only lose while never charting the case the library exists for.What the numbers say: svector is 2.2x behind
std::vectoronpush_backof a trivial type, 18% on a subscript, and 2.6x behind absl at filling inline storage. It is the fastest of the four atemplace_backof strings and at inserting them at the front, the fastest of the three inline containers at swapping, and no worse than any of them at sorting — at a third the size of the next smallest object.