[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-hotspot-runtime-dev
Subject: Re: RFR: 8331208: Memory stress test that checks OutOfMemoryError stack trace fails
From: Doug Simon <dnsimon () openjdk ! org>
Date: 2024-04-30 7:34:08
Message-ID: TD_o2kM5UsGhdnT_ExspbhknHnshN85lvH5kLMAh3cU=.d1cb2690-6baa-46e4-bfd2-d7d72e18a5f3 () github ! com
[Download RAW message or body]
On Tue, 30 Apr 2024 05:33:56 GMT, David Holmes <dholmes@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/3d5eeac3a38ec...
>
> 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.
I think it was a mistake to make it conditional when RetryableAllocationMark was \
first introduced. The purpose of RAM was to only to resolve a correctness issue wrt \
to JVMTI (it was seeing the "same" exception being reported twice). The -XX actions \
do not change the semantics of the exception throwing so can be done unconditionally.
> 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.
By "this part", do you mean the `else` branch? It exists for the `!activate` case of \
RetryableAllocationMark which is used when the `null_on_fail` parameter of \
`JVMCIRuntime::new_instance_common` is true. That is, the runtime call is from \
compiled code that does *not* want to trigger throwing of an OOME. Graal will deopt \
in such cases and let the interpreter throw the exception. This ensures the OOME is \
reported exactly once to JVMTI.
> 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
Same reasoning as for `MemAllocator::Allocation::check_out_of_memory`.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18925#discussion_r1584241473
PR Review Comment: https://git.openjdk.org/jdk/pull/18925#discussion_r1584261677
PR Review Comment: https://git.openjdk.org/jdk/pull/18925#discussion_r1584262845
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic