Repository navigation
test(allocation): set the InsertBatch ceiling from runner measurements (915 B/row) - #504
Merged
Merged
Conversation
…s (915 B/row) The per-OS ceiling was still wrong: the GitHub windows-latest runner measures 902 B/row while this developer box measures 894 on the same commit, and both Unix runners measure 903. So the spread is the environment's GC/JIT accounting, not the OS and not a code path - an absolute-bytes canary set on a developer machine fails every runner. One ceiling now covers all four environments: 915 B/row, above the highest runner figure (903) and below the pre-hoist 919 (about 927 on a runner), so reintroducing the allocation this canary was written to catch still fails everywhere. Measurements recorded in the test: dev box win-x64 894 (1.787.608 bytes), runner win-x64 902 (1.803.560), runner linux-x64 903 (1.806.872), runner osx-arm64 903 (1.806.856). Verified locally: the canary passes on win-x64 with the new ceiling.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
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.
mastermerged the promotion at4c078b8a, which carried the per-OS allocation ceiling —OperatingSystem.IsWindows() ? 900 : 915. The Windows figure was recorded on a developer machine (894 B/row), but the GitHubwindows-latestrunner measures 902 B/row for the same commit, so the Windows job ofBuild and Testfails onmasterand thepack→publishjobs are skipped (needs: build), which blocks the 2.1.3 publication.This PR replaces that with the ceiling measured on the runners, which was pushed five minutes after the merge (
26fdcfde/ the single commit here):915 is above the highest runner figure and still below the pre-hoist 919 B/row (~927 on a runner), so reintroducing the allocation this canary was written to catch still fails on every platform. The test comment records all four measurements and the guardrail: absolute-bytes canaries must be set from the runner measurement, never a developer machine.
Verified locally: the canary passes on win-x64 with 915 (894 measured). Ubuntu and macOS already passed at 915 in the previous run of the promotion PR — only Windows was above the old 900 ceiling.