-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
[release/8.0] JIT: Fix exponential blowup of memory dependency arrays in VNForMapSelectWork #93388
Conversation
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsBackport of #93344 to release/8.0 /cc @jakobbotsch Customer ImpactTestingRiskIMPORTANT: If this backport is for a servicing release, please verify that:
|
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.
approved. please get a code review. we will take for consideration in 8.0.x
Backport of #93344 to release/8.0
/cc @jakobbotsch
Customer Impact
Code of a particular shape may hit exponential blow-up when the JIT processes it as part of one of its optimizations. This can cause extreme memory usage (many gigabytes) and many orders of magnitude of slowdown, potentially showing up as a hang during JIT.
The affected code shape requires a long sequence of
if
statements (or other control flow constructs) where eachif
statement modifies memory in a way that the JIT is able to fully reason about it, for example by writing a particular class field and avoiding any calls to other functions.The exponential behavior occurs if the code after this long sequence of
if
statements then accesses a field that was written before all theif
statements. The PR includes a test case showing an example. The code shape may also be exposed after inlining performed by the JIT.Reported by a customer with an issue opened on their behalf in #93342. Also hit by an internal customer.
Testing
Regression test is included. Also tested by internal customer and confirmed as fixing the issue. Stress testing also done as part of the original PR.
Risk
Low. The fix effectively replaces a dynamic array with a set.