This repository was archived by the owner on Jan 23, 2023. It is now read-only.
Reduce time taken to test StackGuard and move to innerloop. #14444
Conversation
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
Contributor
Author
|
@bartdesmet changing your test of your functionality here, so PTAL |
Contributor
Author
|
@dotnet-bot test code coverage |
Contributor
Author
|
(And I just remembered that code coverage isn't going to show anything useful due to the existing issue there 😞 ) |
stephentoub
reviewed
Dec 13, 2016
| // This reduces the size of tree needed to risk a stack overflow. | ||
| Thread t = new Thread(() => f = Expression.Lambda<Func<int>>(e).Compile(useInterpreter), 1); | ||
| t.Start(); | ||
| t.Join(); |
Member
There was a problem hiding this comment.
Did the previous code overflow more than once? Presumably this new version won't, so maybe we still need the old test as outerloop in addition to the new one as inner?
We have a test that verifies that Expression compilation is protected against stack overflow in the case of compiling deep expression trees, but forcing a case that would cause such a problem by necessity requires a large tree that takes a long time to compiile, so the test is outerloop only. Forcing the compilation to happen on the smallest possible stack reduces the size of tree that would risk a stack overflow, and so can test the same functionality in less than a second. Do this, and move the test to innerloop.
JonHanna
force-pushed
the
test_stackguard_innerloop
branch
from
583e550 to
99975c9
Compare
Contributor
Author
|
Brought the old version back beside the new, to test the multiple-overflow case. |
Contributor
Author
|
Test Innerloop OSX Release Build and Test |
stephentoub
approved these changes
Dec 13, 2016
jakobbotsch
added a commit
to jakobbotsch/runtime
that referenced
this pull request
May 27, 2021
The manually assigned 128 KiB stack here is bordering on the amount of stack space that release JIT needs to JIT functions with deep trees. Since the JIT already has a limit on the depth of trees it creates https://github.com/dotnet/runtime/blob/44f050acc0f7791d6cd7ac772945444912bcf299/src/coreclr/jit/importer.cpp#L11234-L11252 it seems unnecessary to pressimize the JIT further to allow this test to pass. Also, the test was originally added as a faster equivalent to the test above it that could run in inner loop (dotnet/corefx#14444), but it was subsequently moved to outer loop (dotnet/corefx#19563), so now it does not seem like there's much point in retaining it. Fix dotnet#53309
picenka21
pushed a commit
to picenka21/runtime
that referenced
this pull request
Feb 18, 2022
…innerloop Reduce time taken to test StackGuard and move to innerloop. Commit migrated from dotnet/corefx@bdfe654
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 participants
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.
We have a test that verifies that
Expressioncompilation is protected against stack overflow in the case of compiling deep expression trees, but forcing a case that would cause such a problem by necessity requires a large tree that takes a long time to compile, so the test is outerloop only.Forcing the compilation to happen on the smallest possible stack reduces the size of tree that would risk a stack overflow, and so can test the same functionality in less than a second.
Do this, and move the test to innerloop.