Hand the generated loop's own Maps on at the run's last use - #1437
Merged
Merged
Conversation
__bump read the run whole (__versionOf(run, owner)) inside the update that sets run.versions, and __park did the same with moved.now and __versionOf(moved, owner) inside the update that sets moved.slots, so on the VM neither record update could take its Map field out and every answer copied both Maps. perf-shared-update reports both when it is allowed to look at generated functions. Both now read everything else first and update the run at its last use. __take<Module> reads the state before it updates the run, so the update moves the run's other fields instead of leaving the old run holding them, and __seatFamily<P> lists the retired keys before it hands the retired Map to __unretire<P>. With the generated-function filter lifted, perf-shared-update reports nothing in the loop fixtures. On the VM the gain shows together with the consumed-tuple release (#1436), since the run reaches __serve<P><Kind> through the __take tuple: on run_owned_answer_state the writes refused because something else held the Map go from 403 to 4, and on its 2000-request version from 4003 to 4. The Maps are small, so wall time does not move. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
# Conflicts: # CHANGELOG.md
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.
__bump(run.versions) and__park(moved.slots) copied their Map on every answer on the VM. Each one read the run whole inside the same record update that sets the Map:__versionOf(run, owner)in__bump, andmoved.now/__versionOf(moved, owner)in__park. That meant the #1429 field take could never apply.What changes (generated Aver only)
__bumpand__park. Both bind the version (and in__parkthe clock) first, then update the run at its last use. The Map field is read once, inside the update that replaces it.__take<Module>. It now returns(run.<field>, __Run.update(run, <field> = Option.None)): the state is read first and the run is updated at its last use. The update then moves the run's other fields instead of leaving the dead run holdingslotsandversions. The tuple order flips, and__serve<P>matches(__held, __rest).__seatFamily<P>. Same class of problem, found by the warning: it passedkept.retired<P>andMap.keys(kept.retired<P>)to one call. It now lists the keys first.perf-shared-updateon the generated functionsThe warning skips
__functions, so I checked with that filter lifted locally (the change is not committed). I ranaver checkonrun_all_slice,run_owned_answer_state,run_schedule_casesandrun_families:__bump,__parkand__seatFamilyMember.Measurements
Map writes the runtime refused because something else held the Map, from
aver run --profile, release build without LTO:run_owned_answer_stateOn its own this PR does not change the VM numbers. The run reaches
__serve<P><Kind>through the__taketuple, and on main that tuple still holds the run (#1436 fixes that). With #1436 in, the per-answer copies of the loop's own Maps go away. These Maps are small, so wall time does not move (0.125 s vs 0.126 s at 2000 requests).Tests
run_all_spec: the dump test now pins the three new shapes.rust_work_specpinslet (__held, __rest) = __takeOwner(run);, and the Rust side still hands the state over by value.run_all_spec,rust_work_spec,yield_verify_spec(including the Lean check of the coordinator laws:__park law anErrKeepsTheInstancestays universal), andwasm_work_spec --features wasm.python3 tools/regenerate_self_host.py --check: the self-host does not change.cargo fmtis clean.