RFR: 8331208: Memory stress test that checks OutOfMemoryError stack trace fails

[David Holmes]

On Tue, 23 Apr 2024 21:11:53 GMT, Doug Simon <dnsimon at openjdk.org> wrote:

> This pull request mitigates failures in memory stress tests that check the stack trace of an `OutOfMemoryError` for certain expected entries.
> 
> The stack trace of an OOME will [not be allocated once all preallocated OOMEs are used up](https://github.com/openjdk/jdk/blob/3d5eeac3a38ece4a23ea6da2dfe5939d64e81cea/src/hotspot/share/memory/universe.cpp#L722). If the only heap allocations performed in stressful conditions are those of the stress test, then the [4 preallocated OOMEs](https://github.com/openjdk/jdk/blob/f1d0e715b67e2ca47b525069d8153abbb33f75b9/src/hotspot/share/runtime/globals.hpp#L800) would be sufficient. However, it's possible for VM internal allocations to also occur during stressful conditions, especially in `-Xcomp` mode. For example, [CompileBroker::compile_method](https://github.com/openjdk/jdk/blob/3d5eeac3a38ece4a23ea6da2dfe5939d64e81cea/src/hotspot/share/compiler/compileBroker.cpp#L1399) will try to resolve the string constants in the constant pool of the method about to be compiled. This can fail as shown here:
> 
> V  [jvm.dll+0x62c23a]  Exceptions::_throw+0x11a  (exceptions.cpp:168)
> V  [jvm.dll+0x62d85b]  Exceptions::_throw_oop+0xab  (exceptions.cpp:140)
> V  [jvm.dll+0xbbce78]  MemAllocator::Allocation::check_out_of_memory+0x208  (memAllocator.cpp:138)
> V  [jvm.dll+0xbbcac8]  MemAllocator::allocate+0x158  (memAllocator.cpp:377)
> V  [jvm.dll+0x79bd05]  InstanceKlass::allocate_instance+0x95  (instanceKlass.cpp:1509)
> V  [jvm.dll+0x7ddeed]  java_lang_String::basic_create+0x9d  (javaClasses.cpp:273)
> V  [jvm.dll+0x7e43c0]  java_lang_String::create_from_unicode+0x60  (javaClasses.cpp:291)
> V  [jvm.dll+0xdb91a5]  StringTable::do_intern+0xb5  (stringTable.cpp:379)
> V  [jvm.dll+0xdba9f2]  StringTable::intern+0x1b2  (stringTable.cpp:368)
> V  [jvm.dll+0xdbaaa6]  StringTable::intern+0x86  (stringTable.cpp:328)
> V  [jvm.dll+0x51c8b1]  ConstantPool::string_at_impl+0x1d1  (constantPool.cpp:1251)
> V  [jvm.dll+0x51b95b]  ConstantPool::resolve_string_constants_impl+0xeb  (constantPool.cpp:800)
> V  [jvm.dll+0x4f2f8d]  CompileBroker::compile_method+0x31d  (compileBroker.cpp:1395)
> V  [jvm.dll+0x4f3474]  CompileBroker::compile_method+0xc4  (compileBroker.cpp:1348)
> 
> These internal allocations can occur before the allocations of the test and thus use up the pre-allocated OOMEs. As a result, the OOMEs triggered by the stress test may end up throwing the [default, shared OOME instance](https://github.com/openjdk/jdk/blob/3d5eeac3a38ece4a23ea6da2dfe5939d64e81cea/src/hotspot/...

So you are generalising (and seemingly simplifying) the notion of a "retryable allocation" so that internally an OOME can be ignored for a range of reasons. It seems a rather elaborate response to the test failure (especially when generating a stacktrace under OOM conditions could itself fail anyway), but I can see the general utility of expanding things this way. I really dislike the name `SandboxedOOMEMark` though - sorry - suggestions: `InternalOOMEMark`, `ScopedOOMEMark`, `ConfinedOOMEMark` ?

My main concerns relate to me not understanding the details of the existing retryable allocation, so some of the new code seems a little odd. Comments below.

Thanks

src/hotspot/share/gc/shared/memAllocator.cpp line 127:

> 125:   const char* message = _overhead_limit_exceeded ? "GC overhead limit exceeded" : "Java heap space";
> 126:   // -XX:+HeapDumpOnOutOfMemoryError and -XX:OnOutOfMemoryError support
> 127:   report_java_out_of_memory(message);

Not obvious we now need this to be unconditional.

src/hotspot/share/gc/shared/memAllocator.hpp line 139:

> 137:       _outer = false;
> 138:       _thread = nullptr;
> 139:     }

It isn't obvious to me how this part is intended to be used. I see it ties back to the retryable allocation "activate" mode, but I'm unclear what that means as well.

src/hotspot/share/oops/klass.cpp line 876:

> 874: void Klass::check_array_allocation_length(int length, int max_length, TRAPS) {
> 875:   if (length > max_length) {
> 876:     report_java_out_of_memory("Requested array size exceeds VM limit");

Again not obvious this should now be unconditional

-------------

PR Review: https://git.openjdk.org/jdk/pull/18925#pullrequestreview-2030233468
PR Review Comment: https://git.openjdk.org/jdk/pull/18925#discussion_r1584154877
PR Review Comment: https://git.openjdk.org/jdk/pull/18925#discussion_r1584160406
PR Review Comment: https://git.openjdk.org/jdk/pull/18925#discussion_r1584164575