On 3/26/17 11:55 PM, Chris Plummer wrote:
Hi David,
On 3/26/17 5:06 PM, David Holmes wrote:
Hi Chris,
On 25/03/2017 6:12 AM, Chris Plummer wrote:
Hello,
Please review changes for the following:
http://cr.openjdk.java.net/~cjplummer/8177015/webrev.00/
https://bugs.openjdk.java.net/browse/JDK-8177015
Functional change seems okay, though I do have to wonder why we see
such variance in behaviour on different OS X systems
Newer version of OS X I believe. It worked fine for months. Seems 100%
reproducible on certain systems now.
The CR description explains the problem. I'm increasing the minimum
allowed -Xss stack size from 32k to 64k. There are also a couple of
tests that were testing for -Xss32k that I updated to 64k. I also added
hotspot/test/runtime/Thread/TooSmallStackSize.java
Don't understand why you bumped the 32K to 64K in this test as they
seem unrelated to -Xss flag ??
I considered not doing this change, but wanted to keep the test in
sync with the jdk test it was originally based on. There is some
relation to -Xss here in that -Xss impacts ThreadStackSize, although
I don't believe the other way around (if you set ThreadStackSize, I
don't think it impacts the main thread).
Correct. Once you get to the point where the VM is up enough to
process the ThreadStackSize option, the main thread is already
up and running with the stack size set by the launcher.
Dan
In any case, the 32k for all the thread stack size testing done here
(including VM and Compiler) was replicated from the JDK test. I'm not
sure there's that much importance to this size, other than to set it
small enough so an error message with the minimum allowed stack size
will be generated (although that could also be done with the 16k size
that is already being tested for).
a new test case for 253k. This is something I intended to do for
I see no new test for 253K. I see one for 513K in the launcher test.
Sorry, that was a typo in my email. Should have said 513k.
thanks,
Chris
Thanks,
David
JDK-8176768, but forgot to include the change in the review. It's not
really related to this CR, but I figured I'd drop it in since I'm
making
other changes to the file.
thanks,
Chris