8247615: Initialize the bytes left for the heap sampler

Jean Christophe Beyler · Jean Christophe Beyler · commit 7339cf8f5454 · 2020-07-16T16:58:55.000-07:00
Initialize the heap sampler correctly before usage

Reviewed-by: sspitsyn, martin, manc
diff --git a/src/hotspot/share/gc/shared/threadLocalAllocBuffer.hpp b/src/hotspot/share/gc/shared/threadLocalAllocBuffer.hpp
@@ -111,7 +111,8 @@ class ThreadLocalAllocBuffer: public CHeapObj<mtThread> {
   int slow_allocations() const  { return _slow_allocations; }
 
 public:
-  ThreadLocalAllocBuffer() : _allocated_before_last_gc(0), _allocation_fraction(TLABAllocationWeight) {
+  ThreadLocalAllocBuffer() : _allocated_before_last_gc(0), _bytes_since_last_sample_point(0),
+    _allocation_fraction(TLABAllocationWeight) {
     // do nothing.  tlabs must be inited by initialize() calls
   }
 
diff --git a/src/hotspot/share/runtime/threadHeapSampler.hpp b/src/hotspot/share/runtime/threadHeapSampler.hpp
@@ -50,11 +50,14 @@ class ThreadHeapSampler {
   uint64_t next_random(uint64_t rnd);
 
  public:
-  ThreadHeapSampler() : _bytes_until_sample(0) {
+  ThreadHeapSampler() {
     _rnd = static_cast<uint32_t>(reinterpret_cast<uintptr_t>(this));
     if (_rnd == 0) {
       _rnd = 1;
     }
+
+    // Call this after _rnd is initialized to initialize _bytes_until_sample.
+    pick_next_sample();
   }
 
   size_t bytes_until_sample()                    { return _bytes_until_sample;   }
diff --git a/test/hotspot/jtreg/serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitorInitialAllocationTest.java b/test/hotspot/jtreg/serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitorInitialAllocationTest.java
@@ -0,0 +1,73 @@
+/*
+ * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2020, Google and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+package MyPackage;
+
+/**
+ * @test
+ * @summary Verifies the JVMTI Heap Monitor does not always sample the first object.
+ * @build Frame HeapMonitor
+ * @compile HeapMonitorInitialAllocationTest.java
+ * @run main/othervm/native -agentlib:HeapMonitorTest MyPackage.HeapMonitorInitialAllocationTest
+ */
+public class HeapMonitorInitialAllocationTest {
+  public static void main(String[] args) throws Exception {
+    final int numThreads = 500;
+
+    // Allocate the threads first to not risk sampling.
+    Thread threads[] = new Thread[numThreads];
+    for (int i = 0; i < numThreads ; i++) {
+      threads[i] = new Thread(new Task(), "Task " + i);
+    }
+
+    HeapMonitor.setSamplingInterval(1024 * 1024);
+    HeapMonitor.enableSamplingEvents();
+
+    for (int i = 0; i < numThreads ; i++) {
+      threads[i].start();
+    }
+
+    for (int i = 0; i < numThreads ; i++) {
+      threads[i].join();
+    }
+
+    HeapMonitor.disableSamplingEvents();
+
+    int sampledEvents = HeapMonitor.sampledEvents();
+    if (sampledEvents > numThreads / 2) {
+      throw new RuntimeException(
+          "Sampling the initial allocation too many times: " + sampledEvents);
+    }
+  }
+
+  private static class Task implements Runnable {
+    @Override
+    public void run() {
+      new EmptyObject();
+    }
+  }
+
+  private static class EmptyObject {
+  }
+}

Original file line number	Diff line number	Diff line change
`@@ -111,7 +111,8 @@ class ThreadLocalAllocBuffer: public CHeapObj<mtThread> {`
`111`	`111`	`int slow_allocations() const { return _slow_allocations; }`
`112`	`112`
`113`	`113`	`public:`
`114`		`- ThreadLocalAllocBuffer() : _allocated_before_last_gc(0), _allocation_fraction(TLABAllocationWeight) {`
	`114`	`+ ThreadLocalAllocBuffer() : _allocated_before_last_gc(0), _bytes_since_last_sample_point(0),`
	`115`	`+ _allocation_fraction(TLABAllocationWeight) {`
`115`	`116`	`// do nothing. tlabs must be inited by initialize() calls`
`116`	`117`	`}`
`117`	`118`
Original file line number	Diff line number	Diff line change
`@@ -50,11 +50,14 @@ class ThreadHeapSampler {`
`50`	`50`	`uint64_t next_random(uint64_t rnd);`
`51`	`51`
`52`	`52`	`public:`
`53`		`- ThreadHeapSampler() : _bytes_until_sample(0) {`
	`53`	`+ ThreadHeapSampler() {`
`54`	`54`	`_rnd = static_cast<uint32_t>(reinterpret_cast<uintptr_t>(this));`
`55`	`55`	`if (_rnd == 0) {`
`56`	`56`	`_rnd = 1;`
`57`	`57`	`}`
	`58`	`+`
	`59`	`+ // Call this after _rnd is initialized to initialize _bytes_until_sample.`
	`60`	`+ pick_next_sample();`
`58`	`61`	`}`
`59`	`62`
`60`	`63`	`size_t bytes_until_sample() { return _bytes_until_sample; }`