RFR: 8308231: Faster MemAllocator::Allocation checks for verify/notification [v2]

Aleksey Shipilev shade at openjdk.org
Fri May 19 07:33:53 UTC 2023


On Wed, 17 May 2023 15:45:59 GMT, Leonid Mesnik <lmesnik at openjdk.org> wrote:

>> Aleksey Shipilev has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains six additional commits since the last revision:
>> 
>>  - Touch up comment
>>  - Merge branch 'master' into JDK-8308231-memalloc-check-faster
>>  - Hide more stuff
>>  - Touchups
>>  - Branch
>>  - Fix
>
> src/hotspot/share/gc/shared/memAllocator.cpp line 75:
> 
>> 73:   //   - (optionally) the enabled JVMTI event that wants to capture all allocations;
>> 74: 
>> 75:   bool should_notify_allocation_no_jvmti_vmobjalloc() {
> 
> As I understand this check is specific for SampledObjectAlloc event. Might be better to name it like: should_notify_jvmti_sampled_object_alloc? 
> 
> Does it make sense to check should_post_sampled_object_alloc also here? It is usually disabled.

No, this check verifies everything, _but_ `JvmtiExport::should_post_vm_object_alloc()`, see the full method below. So in `MemAllocator::Allocation::notify_allocation_jvmti_sampler` can actually be called for two reasons: sampling event is required, or VMObjectAlloc is required. The `should_notify_allocation_no_jvmti_vmobjalloc` disambiguates the case where we don't need to proceed with sampling event gathering. 

Honestly, I can just revert this hunk, but I think it is cleaner to expose the helper:


@@ -187,9 +206,8 @@ void MemAllocator::Allocation::notify_allocation_jvmti_sampler() {
     return;
   }
 
-  if (!_allocated_outside_tlab && _allocated_tlab_size == 0 && !_tlab_end_reset_for_sample) {
-    // Sample if it's a non-TLAB allocation, or a TLAB allocation that either refills the TLAB
-    // or expands it due to taking a sampler induced slow path.
+  if (!should_notify_allocation_no_jvmti_vmobjalloc()) {
+    // Called here only for JVMTI VMObjectAlloc event
     return;
   }

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14019#discussion_r1198639901


More information about the hotspot-gc-dev mailing list