Repository navigation
perf: plain counts on structs only one task can reach - #1230
Merged
Merged
Conversation
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.
A struct's strong and weak counts are now plain loads and stores while only the task that built it can reach it, and atomic once it is marked shared. On struct graphs the locked read-modify-write was most of the count cost (an unsafe all-plain prototype took cyclic_graph from 384 to 232 ms); everywhere else the counts were not the bottleneck.
How it works (cranelift/runtime/src/share.rs):
pith_spawn), a task's result (the spawn wrapper), a channel send and aselectsend arm, a store into a module global and the global's initializer, and a store into a struct, list or map that is already shared (an inline test of the holder's mark at field stores, a check of the container's mark in the list and map store paths). The barrier runs before the store.PITH_LOCAL_COUNTS=0keeps every count atomic for comparison.PITH_RC_CHECK=1records the task that built each struct and aborts when another task makes a plain count update on it.make rc-check-onlyruns the green corpus and the green-thread tests that way, and CI runs it. It found three gaps while this was being built: the field barrier ran after the store, tracers were registered after global init, and function-typed globals were not marked.Interleaved against main 6a0132b (5 rounds, 9 for the noisy rows): cyclic_graph 401 to 282 ms, cyclic_graph_strong 780 to 724 ms, http_head_read 711 to 683 ms, lock_sweep/labels 61 to 57 ms. task_churn reads 4 to 6% slower; it does not change with PITH_LOCAL_COUNTS, so it is the added checks on the alloc and free paths or placement. Other rows within noise.
New case
test_shared_markingcrosses tasks through every path above. It is not in the memcheck list: its channel-receive loop shows two invalid reads under valgrind on main too, a separate bug.