perf: write the constructor header with a single store - #15010
Draft
Kha wants to merge 1 commit into
Draft
Conversation
This PR lets `lean_alloc_ctor` write all four header fields at once, replacing the two partial stores a constructor allocation needed before. `lean_alloc_small_object_core` initializes `m_cs_sz` itself, so a caller that goes on to set the remaining header fields cannot merge its stores with that one. A second entry point `lean_alloc_small_object_raw` leaves the header entirely to the caller, and `lean_alloc_ctor` then writes `m_rc`, `m_cs_sz`, `m_other` and `m_tag` with adjacent stores, which fold into one for the constant arguments of compiled code. Allocating a `List.cons`: ```diff mov edi, 0x18 -call lean_alloc_small_object_core ; stores m_cs_sz itself -mov DWORD PTR [rax], 0x1 ; m_rc -mov WORD PTR [rax+0x6], 0x102 ; m_other, m_tag; cannot merge across m_cs_sz +call lean_alloc_small_object_raw ; leaves the header alone +movabs rcx, 0x102001800000001 ; m_rc | m_cs_sz | m_other | m_tag +mov QWORD PTR [rax], rcx ; one store mov QWORD PTR [rax+0x8], r14 mov QWORD PTR [rax+0x10], rbx ``` What this removes is stores rather than instructions. The new entry point is the same length as `lean_alloc_small_object_core`, the register freed by dropping the `m_cs_sz` store being spent elsewhere, and the call site needs two instructions either way; what changes is that the three header stores a constructor allocation performed become one. Measured by swapping `libleanshared.so` between two full builds, single-threaded with `-DElab.async=false` on pinned cores: `tests/elab_bench/big_omega.lean` goes from 18.7822G to 18.7748G user instructions (-0.039%) and `import Lean` from 1.93575G to 1.93511G (-0.033%), each reproducible to within 0.02%. Whether the reduced store traffic is worth more than that in cycles was not resolvable on the machine used, where a same-binary control varied by 2.7% between rounds. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Member
Author
|
!bench |
|
Benchmark results for 55ee200 against 137a88d are in. There are significant results. @Kha
Large changes (3✅)
Medium changes (5✅, 3🟥)
Small changes (3✅, 4🟥)
|
|
Mathlib CI status (docs):
|
Collaborator
|
Reference manual CI status:
|
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.
This PR lets
lean_alloc_ctorwrite all four header fields at once, replacing the two partial stores a constructor allocation needed before.lean_alloc_small_object_coreinitializesm_cs_szitself, so a caller that goes on to set the remaining header fields cannot merge its stores with that one. A second entry pointlean_alloc_small_object_rawleaves the header entirely to the caller, andlean_alloc_ctorthen writesm_rc,m_cs_sz,m_otherandm_tagwith adjacent stores, which fold into one for the constant arguments of compiled code. Allocating aList.cons:What this removes is stores rather than instructions. The new entry point is the same length as
lean_alloc_small_object_core, the register freed by dropping them_cs_szstore being spent elsewhere, and the call site needs two instructions either way; what changes is that the three header stores a constructor allocation performed become one.