Skip to content

refactor(once_map): build the table before locking it and simplify lookups - #355

Open
orthur2 wants to merge 5 commits into
apache:mainfrom
orthur2:refactor/once-map-exclusive-build
Open

orthur2 wants to merge 5 commits into
apache:mainfrom
orthur2:refactor/once-map-exclusive-build

Conversation

@orthur2

@orthur2 orthur2 commented Oct 10, 2026

Copy link
Copy Markdown
Contributor

Summary

FromIterator fills a local table and wraps it in the mutex once it is complete. insert(&mut self) was its only caller and locked a table nobody else could reach; a duplicate key now replaces its entry in place because no lock is held.

get_or_insert returns the entry and compute and try_compute check the cell themselves, replacing the Lookup enum and classify; the function now has the same shape as the singleflight group, including the note on where duplicate keys drop. A new test drops a duplicate key whose destructor discards another key, on both the pending and the ready path, so that order cannot regress silently. The entry is bound inside a block so that its scope ends before the await, which keeps the compute and try_compute futures at 192 and 176 bytes. get does its own lookup and remove_entry returns straight after the removal. The removed layers, the Entries alias, and a comment about a "write lock" were left from the sharded table, where the public map and the shard were different types. Every value is still cloned after the table lock is released, and public behavior is unchanged.

The unit tests use the imported pin! and Poll, pin futures where they are built, and check the release flag with a plain conditional; the lookup bench comment no longer describes a "ready index". Existing tests cover duplicate keys in collect(), cleanup of abandoned entries, and growth under concurrent compute. A new bench measures building a map from an iterator, which had no measurement before.

Benchmarks

main is 44fc2d8. Apple M5, macOS 26.4, Rust 1.96.0. cargo x bench --bench primitives -- --threads 1 with fixed sample counts, 100 samples of 64 iterations for the build rows and 200 samples of 256 for the lookup rows, both binaries alternating for ten rounds. Each cell is the median of the nine rounds after the first.

Benchmark main This branch Change
once_map::build::collect_distinct_keys/64 957.4 ns 762.1 ns -20.4%
once_map::build::collect_distinct_keys/1024 14.42 µs 11.40 µs -20.9%
once_map::build::collect_duplicate_keys/64 1.22 µs 0.99 µs -18.9%
once_map::build::collect_duplicate_keys/1024 18.87 µs 15.36 µs -18.6%
once_map::lookup::get_hit_same_key 6.45 ns 6.45 ns -0.1%
once_map::lookup::get_hit_distributed/1024 6.93 ns 6.94 ns +0.1%
once_map::lookup::get_miss_distributed/1024 6.45 ns 6.46 ns +0.2%
once_map::lookup::compute_hit_same_key 9.54 ns 9.54 ns 0.0%
once_map::lookup::compute_hit_distributed/1024 11.81 ns 11.66 ns -1.3%

Building from an iterator saves one uncontended lock and unlock per element plus a second table probe for duplicate keys. The lookup rows are within one or two 0.16 ns timer steps of main.

Validation

  • cargo x test (594 passed), cargo x check (26 feature configurations), and cargo x lint
  • Miri on the once_map unit tests

Follow-up to #345.

`from_iter` was the only caller of `insert(&mut self)`, which locked a
table nobody else could reach. Fill a local table and wrap it in the
mutex once it is complete; a duplicate key replaces its entry in place
because no lock is held.
`Lookup` and `classify` wrapped a single `cell.get()` check that
`compute` and `try_compute` only unpacked again. Returning the entry lets
each caller take its own fast path and gives `get_or_insert` the same
shape as the singleflight group, including the note on where duplicate
keys drop. A test drops a duplicate key whose destructor discards
another key, on the pending and on the ready path, so the unlock-before-
drop order cannot regress silently. The entry is bound inside a block so
that its scope ends before the await: a local that outlives the await
keeps a slot in the future and made `compute` 8 bytes larger.
`get` delegated to `get_value`, which delegated to `find_entry`, each
with one caller; the `Entries` alias had one use; `remove_entry`
unlocked explicitly right before returning; and a comment still said
"write lock" although the table is behind a mutex. All of them date
from the sharded table, where the public map and the shard were
different types. `get` still releases the lock before inspecting the
entry.
Use the imported `pin!` and `Poll`, pin futures where they are built,
write the release check as a plain conditional, and stop describing a
"ready index" the map does not have.
The build path had no measurement. The two cases cover distinct keys
and a 50% duplicate rate at the entry counts the lookup benches use.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant