-
Notifications
You must be signed in to change notification settings - Fork 4.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add hot/cold splitting test job to jit-runtime-experimental #69922
Conversation
Tagging subscribers to this area: @JulieLeeMSFT Issue DetailsAdding a new rolling test job to
This environment forces the JIT to fake-split every method after its first basic block, assuming the method consists of more than one block. In my local testing, all of the x64 tests in
|
Looks good. However, this isn't related to OSR, so remove "osr" from the name. Use |
671c81a
to
c2765b9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
You should trigger jit-runtime-experimental
on this PR to verify.
/azp run runtime-jit-experimental |
Azure Pipelines successfully started running 1 pipeline(s). |
I ran and re-ran |
You hit #69854, which I believe is now fixed. Try triggering the job again.
No -- It's the "Send to Helix" step that actually runs tests, and since it failed, no tests were run. |
You also hit the "remainingSize == 4" assert which was fixed with #69905 Also, some wasm failures which I don't see issues for, but which are obviously unrelated. You can always try "Re-run" on those. |
/azp run runtime-jit-experimental |
Azure Pipelines successfully started running 1 pipeline(s). |
I pulled from |
The scheduled run on Sunday did not have those issues (it had other issues, but not those: https://dev.azure.com/dnceng/public/_build/results?buildId=1795838&view=ms.vss-test-web.build-test-results-tab). My suggestion:
It shouldn't be necessary, but if that doesn't work, try closing and re-opening the PR, or create an entirely new PR. |
9569921
to
00ba968
Compare
/azp run runtime-jit-experimental |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-jit-experimental |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-jit-experimental |
Azure Pipelines successfully started running 1 pipeline(s). |
Fake-splitting currently breaks stack walks by not generating unwind info for cold code. This commit implements unwind info on x86/64 when fake-splitting by generating unwind info for the combined hot/cold section just once, rather than generating unwind info for the separate sections. For reasons to be investigated, this implementation does not work when the code sections are separated by an arbitrary buffer (such as the 4KB buffer previously used). Thus, the buffer has been removed from the fake-splitting implementation: Now, the hot and cold sections are placed contiguously in memory, but the JIT continues to behave as if they are arbitrarily far away (for example, by using long branches between sections). Following this fix, fake-splitting no longer requires the GC to be suppressed by setting `COMPlus_GCgen0size=1000000`. A test job has been added to `runtime-jit-experimental` to ensure fake/stress-splitting does not regress.
00ba968
to
e727fba
Compare
/azp run runtime-jit-experimental |
Azure Pipelines successfully started running 1 pipeline(s). |
I have pushed a commit with a fix for unwind info generation when fake-splitting: When calling I've tested the x64 implementation locally, and will adjust x86 implementation/add ARM implementation as needed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. If the tests pass, you're good to merge.
Other than |
Adding a new rolling test job to
JIT-runtime-experimental
can help ensure the new fake- and stress-splitting modes in the JIT do not regress. The proposed test, titledjitosr_stress_splitting
, runs a Checked build of the JIT on OS- and platform-specific test suites with the following environment:This environment forces the JIT to fake-split every method after its first basic block, assuming the method consists of more than one block.
COMPlus_GCgen0size=1000000
sets a large memory allocation threshold before the GC runs; recall thatCOMPlus_JitFakeProcedureSplitting
currently doesn't generate unwind information for cold code, thus breaking the GC's stack walks.COMPlus_GCgen0size
provides a patchwork fix for this limitation in cases where memory usage is not excessive (i.e.>= 0x1000000 B
).In my local testing, all of the x64 tests in
src/tests/
pass with this environment.