Repository navigation
perf(set): single-probe ttak_set_add (and fix inverted presence check) - #99
Merged
Merged
Conversation
ttak_set_add previously performed a full get probe followed by a full
put probe per call. It also had an inverted presence check, so keys
were only (re)stored when already present and new keys were never
added; the set-container tutorial observed 'contains beta? no'
immediately after adding 'beta'.
Refactor the insert path in table.c into a shared table_insert core
with a replace flag, and expose ttak_table_put_if_absent() through a
new internal header. ttak_set_add now resolves every add, duplicate or
fresh, with a single probe while keeping put-if-absent semantics.
A/B benchmark (1M add attempts over a 100k-element set, duplicates
dominating, best of 5, gcc -O3 -march=native):
before: 29.8-30.4 ns/add
after: 22.0-22.1 ns/add (~26% faster)
Full test suite passes; tutorial 07 now prints the documented output.
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
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.
Summary
ttak_set_adddid a fullgetprobe and then a fullputprobe per call — two hash computations and two table walks for every insert. While reading the code I also found the presence check was inverted: keys were only re-stored when already present, so new keys were never added. This is a live bug —tutorials/07-set-containerprintscontains beta? noimmediately after adding"beta"on currentmain.Changes
src/ht/table.c: extract the insert logic into a sharedtable_insert()core with areplaceflag;ttak_table_put()keeps its exact old behavior, and a newttak_table_put_if_absent()performs a single probe and reports whether it inserted.internal/ttak/table_internal.h(new): declaration ofttak_table_put_if_absent, following the existing internal-header convention (e.g.mem_internal.h).src/container/set.c:ttak_set_addnow callsttak_table_put_if_absent— one probe per add, duplicates stay no-ops (existing value never replaced, no destructor invoked).bench/bench_set_add_dup.c(new): the A/B benchmark used for the numbers below.Benchmark (A/B against
mainbuilt in a clean worktree)Workload: prefill a set with 100k keys via
ttak_table_put(identical table state for both builds, since oldttak_set_addcannot insert), then 1Mttak_set_addattempts — 90% duplicates, 10% rotating fresh keys. gcc-O3 -march=native, best of 5 repeats, interleaved runs:* single outlier run; all other new-build runs cluster at 22.0–22.1. ~26% faster, stable across runs, well above the 5% bar. The win comes from halving probes on duplicate-dominant adds.
Verification
make test(gcc): all tests pass.contains beta? yes,contains alpha? noafter removal).Notes
ttak_table_putsemantics are unchanged (replace +val_freeon existing values), sottak_mapand all other table users are unaffected.