On Mon, 10 Apr 2023 18:46:42 GMT, Serguei Spitsyn <sspit...@openjdk.org> wrote:
>> Updated VM internal object allocation C2 intrinsic to post jvmti events when >> needed. > > src/hotspot/share/opto/library_call.cpp line 2856: > >> 2854: set_result(ideal.value(result)); >> 2855: return true; >> 2856: #else > > Nit: It is better to replace #else at 2856 with #endif. Then #endif at 2859 > is not needed. In this case the code is: ``` set_result(ideal.value(result)); return true; set_result(obj); return true; Which might cause compiler warnings and complains from static analyzers. > src/hotspot/share/prims/jvmtiEventController.cpp line 727: > >> 725: JvmtiExport::set_should_post_on_exceptions((any_env_thread_enabled >> & SHOULD_POST_ON_EXCEPTIONS_BITS) != 0); >> 726: >> 727: JvmtiExport::_should_post_allocation_notifications = >> JvmtiExport::should_post_vm_object_alloc(); > > I'm not sure why this flag is needed. It looks like a dup of > `JvmtiExport::should_post_vm_object_alloc()`. > Can we just replace it with `JvmtiExport::should_post_vm_object_alloc()`? I don't think we could replace it by function. Also, I think that it is needed later to add SampledObjectAlloc event here. It should consider VM internal object allocations along with all allocations. > test/hotspot/jtreg/ProblemList-Xcomp.txt line 41: > >> 39: serviceability/sa/TestJhsdbJstackMixed.java 8248675 linux-aarch64 >> 40: >> 41: serviceability/jvmti/VMObjectAlloc/VMObjectAllocTest.java 8288430 >> generic-all > > If the 8288430 is a dup of 8277573 then should we close it as such? Done. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/13312#discussion_r1162123421 PR Review Comment: https://git.openjdk.org/jdk/pull/13312#discussion_r1162124409 PR Review Comment: https://git.openjdk.org/jdk/pull/13312#discussion_r1162124580
