Low-Overhead Heap Profiling

JC Beyler jcbeyler at google.com
Wed Oct 25 17:03:18 UTC 2017


Clearly a last minute clean-up gone awry... Fixed for next webrev :)

On Wed, Oct 25, 2017 at 12:30 AM, Robbin Ehn <robbin.ehn at oracle.com> wrote:

> Hi,
>
> 325     HeapWord *tlab_old_end = thread->tlab().return end();
>
> Should be something like:
>
> 325     HeapWord *tlab_old_end = thread->tlab().end();
>
> Thanks, Robbin
>
> On 2017-10-23 17:27, JC Beyler wrote:
>
>> Dear all,
>>
>> Small update this week with this new webrev:
>>    - http://cr.openjdk.java.net/~rasbold/8171119/webrev.13/
>>    - Incremental is here: http://cr.openjdk.java.net/~ra
>> sbold/8171119/webrev.12_13/
>>
>> I patched the code changes showed by Robbin last week and I refactored
>> collectedHeap.cpp:
>> http://cr.openjdk.java.net/~rasbold/8171119/webrev.12_13/src
>> /hotspot/share/gc/shared/collectedHeap.cpp.patch
>>
>> The original code became a bit too complex in my opinion with the
>> handle_heap_sampling handling too many things. So I subdivided the logic
>> into two smaller methods and moved out a bit of the logic to make it more
>> clear. Hopefully it is :)
>>
>> Let me know if you have any questions/comments :)
>> Jc
>>
>> On Mon, Oct 16, 2017 at 9:34 AM, JC Beyler <jcbeyler at google.com <mailto:
>> jcbeyler at google.com>> wrote:
>>
>>     Hi Robbin,
>>
>>     That is because version 11 to 12 was only a test change. I was going
>> to
>>     write about it and say here are the webrev links:
>>     Incremental:
>>     http://cr.openjdk.java.net/~rasbold/8171119/webrev.11_12/
>>     <http://cr.openjdk.java.net/~rasbold/8171119/webrev.11_12/>
>>
>>     Full webrev:
>>     http://cr.openjdk.java.net/~rasbold/8171119/webrev.12/
>>     <http://cr.openjdk.java.net/~rasbold/8171119/webrev.12/>
>>
>>     This change focused only on refactoring the tests to be more
>> manageable,
>>     readable, maintainable. As all tests are looking at allocations, I
>> moved
>>     common code to a java class:
>>     http://cr.openjdk.java.net/~rasbold/8171119/webrev.11_12/tes
>> t/hotspot/jtreg/serviceability/jvmti/HeapMonitor/MyPackage/H
>> eapMonitor.java.patch
>>     <http://cr.openjdk.java.net/~rasbold/8171119/webrev.11_12/te
>> st/hotspot/jtreg/serviceability/jvmti/HeapMonitor/MyPackage/
>> HeapMonitor.java.patch>
>>
>>     And then most tests call into that class to turn on/off the sampling,
>>     allocate, etc. This has removed almost 500 lines of test code so I'm
>> happy
>>     about that.
>>
>>     Thanks for your changes, a bit of relics of previous versions :). I've
>>     already integrated them into my code and will make a new webrev end
>> of this
>>     week with a bit of refactor of the code handling the tlab slow path.
>> I find
>>     it could use a bit of refactoring to make it easier to follow so I'm
>> going
>>     to take a stab at it this week.
>>
>>     Any other issues/comments?
>>
>>     Thanks!
>>     Jc
>>
>>
>>     On Mon, Oct 16, 2017 at 8:46 AM, Robbin Ehn <robbin.ehn at oracle.com
>>     <mailto:robbin.ehn at oracle.com>> wrote:
>>
>>         Hi JC,
>>
>>         I saw a webrev.12 in the directory, with only test
>> changes(11->12), so I
>>         took that version.
>>         I had a look and tested the tests, worked fine!
>>
>>         First glance at the code (looking at full v12) some minor things
>> below,
>>         mostly unused stuff.
>>
>>         Thanks, Robbin
>>
>>         diff -r 9047e0d726d6 src/hotspot/share/runtime/heapMonitoring.cpp
>>         --- a/src/hotspot/share/runtime/heapMonitoring.cpp      Mon Oct
>> 16
>>         16:54:06 2017 +0200
>>         +++ b/src/hotspot/share/runtime/heapMonitoring.cpp      Mon Oct
>> 16
>>         17:42:42 2017 +0200
>>         @@ -211,2 +211,3 @@
>>             void initialize(int max_storage) {
>>         +    // validate max_storage to sane value ? What would 0 mean ?
>>               MutexLocker mu(HeapMonitor_lock);
>>         @@ -227,8 +228,4 @@
>>             bool initialized() { return _initialized; }
>>         -  volatile bool *initialized_address() { return &_initialized; }
>>
>>            private:
>>         -  // Protects the traces currently sampled (below).
>>         -  volatile intptr_t _stack_storage_lock[1];
>>         -
>>             // The traces currently sampled.
>>         @@ -313,3 +310,2 @@
>>             _initialized(false) {
>>         -    _stack_storage_lock[0] = 0;
>>           }
>>         @@ -532,13 +528,2 @@
>>
>>         -// Delegate the initialization question to the underlying
>> storage system.
>>         -bool HeapMonitoring::initialized() {
>>         -  return StackTraceStorage::storage()->initialized();
>>         -}
>>         -
>>         -// Delegate the initialization question to the underlying
>> storage system.
>>         -bool *HeapMonitoring::initialized_address() {
>>         -  return
>>         -             const_cast<bool*>(StackTraceS
>> torage::storage()->initialized_address());
>>         -}
>>         -
>>           void HeapMonitoring::get_live_traces(jvmtiStackTraces *traces)
>> {
>>         diff -r 9047e0d726d6 src/hotspot/share/runtime/heapMonitoring.hpp
>>         --- a/src/hotspot/share/runtime/heapMonitoring.hpp      Mon Oct
>> 16
>>         16:54:06 2017 +0200
>>         +++ b/src/hotspot/share/runtime/heapMonitoring.hpp      Mon Oct
>> 16
>>         17:42:42 2017 +0200
>>         @@ -35,3 +35,2 @@
>>             static uint64_t _rnd;
>>         -  static bool _initialized;
>>             static jint _monitoring_rate;
>>         @@ -92,7 +91,2 @@
>>
>>         -  // Is the profiler initialized and where is the address to the
>>         initialized
>>         -  // boolean.
>>         -  static bool initialized();
>>         -  static bool *initialized_address();
>>         -
>>             // Called when o is to be sampled from a given thread and a
>> given size.
>>
>>
>>
>>         On 10/10/2017 12:57 AM, JC Beyler wrote:
>>
>>             Dear all,
>>
>>             Thread-safety is back!! Here is the update webrev:
>>             http://cr.openjdk.java.net/~rasbold/8171119/webrev.10_11/
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.10_11/>
>>
>>             Full webrev is here:
>>             http://cr.openjdk.java.net/~rasbold/8171119/webrev.11/
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.11/>
>>
>>             In order to really test this, I needed to add this so thought
>> now
>>             was a good time. It required a few changes here for the
>> creation to
>>             ensure correctness and safety. Now we keep the static pointer
>> but
>>             clear the data internally so on re-initialize, it will be a
>> bit more
>>             costly than before. I don't think this is a huge use-case so
>> I did
>>             not think it was a problem. I used the internal MutexLocker,
>> I think
>>             I used it well, let me know.
>>
>>             I also added three tests:
>>
>>             1) Stack depth test:
>>             http://cr.openjdk.java.net/~rasbold/8171119/webrev.10_11/tes
>> t/hotspot/jtreg/serviceability/jvmti/HeapMonitor/MyPackage/H
>> eapMonitorStackDepthTest.java.patch
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.10_11/te
>> st/hotspot/jtreg/serviceability/jvmti/HeapMonitor/MyPackage/
>> HeapMonitorStackDepthTest.java.patch>
>>
>>             This test shows that the maximum stack depth system is
>> working.
>>
>>             2) Thread safety:
>>             http://cr.openjdk.java.net/~rasbold/8171119/webrev.10_11/tes
>> t/hotspot/jtreg/serviceability/jvmti/HeapMonitor/MyPackage/H
>> eapMonitorThreadTest.java.patch
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.10_11/te
>> st/hotspot/jtreg/serviceability/jvmti/HeapMonitor/MyPackage/
>> HeapMonitorThreadTest.java.patch>
>>
>>             The test creates 24 threads and they all allocate at the same
>> time.
>>             The test then checks it does find samples from all the
>> threads.
>>
>>             3) Thread on/off safety
>>             http://cr.openjdk.java.net/~rasbold/8171119/webrev.10_11/tes
>> t/hotspot/jtreg/serviceability/jvmti/HeapMonitor/MyPackage/H
>> eapMonitorThreadOnOffTest.java.patch
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.10_11/te
>> st/hotspot/jtreg/serviceability/jvmti/HeapMonitor/MyPackage/
>> HeapMonitorThreadOnOffTest.java.patch>
>>
>>             The test creates 24 threads that all allocate a bunch of
>> memory.
>>             Then another thread turns the sampling on/off.
>>
>>             Btw, both tests 2 & 3 failed without the locks.
>>
>>             As I worked on this, I saw a lot of places where the tests
>> are doing
>>             very similar things, I'm going to clean up the code a bit and
>> make a
>>             HeapAllocator class that all tests can call directly. This
>> will
>>             greatly simplify the code.
>>
>>             Thanks for any comments/criticisms!
>>             Jc
>>
>>
>>             On Mon, Oct 2, 2017 at 8:52 PM, JC Beyler <
>> jcbeyler at google.com
>>             <mailto:jcbeyler at google.com> <mailto:jcbeyler at google.com
>>             <mailto:jcbeyler at google.com>>> wrote:
>>
>>                  Dear all,
>>
>>                  Small update to the webrev:
>>             http://cr.openjdk.java.net/~rasbold/8171119/webrev.09_10/
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.09_10/>
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.09_10/
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.09_10/>>
>>
>>                  Full webrev is here:
>>             http://cr.openjdk.java.net/~rasbold/8171119/webrev.10/
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.10/>
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.10/
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.10/>>
>>
>>                  I updated a bit of the naming, removed a TODO comment,
>> and I
>>             added a test for testing the sampling rate. I also updated the
>>             maximum stack depth to 1024, there is no
>>                  reason to keep it so small. I did a micro benchmark that
>> tests
>>             the overhead and it seems relatively the same.
>>
>>                  I compared allocations from a stack depth of 10 and
>> allocations
>>             from a stack depth of 1024 (allocations are from the same
>> helper
>>             method in
>>             http://cr.openjdk.java.net/~rasbold/8171119/webrev.10/raw_fi
>> les/new/test/hotspot/jtreg/serviceability/jvmti/HeapMonitor/
>> MyPackage/HeapMonitorStatRateTest.java
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.10/raw_f
>> iles/new/test/hotspot/jtreg/serviceability/jvmti/HeapMonitor
>> /MyPackage/HeapMonitorStatRateTest.java>
>>                             <http://cr.openjdk.java.net/~r
>> asbold/8171119/webrev.10/raw_files/new/test/hotspot/jtreg/se
>> rviceability/jvmti/HeapMonitor/MyPackage/HeapMonitorStatRateTest.java
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.10/raw_f
>> iles/new/test/hotspot/jtreg/serviceability/jvmti/HeapMonitor
>> /MyPackage/HeapMonitorStatRateTest.java>>):
>>                             - For an array of 1 integer allocated in a
>> loop;
>>             stack depth 1024 vs stack depth 10: 1% slower
>>                             - For an array of 200k integers allocated in
>> a loop;
>>             stack depth 1024 vs stack depth 10: 3% slower
>>
>>                  So basically now moving the maximum stack depth to 1024
>> but we
>>             only copy over the stack depths actually used.
>>
>>                  For the next webrev, I will be adding a stack depth test
>> to
>>             show that it works and probably put back the mutex locking so
>> that
>>             we can see how difficult it is to keep
>>                  thread safe.
>>
>>                  Let me know what you think!
>>                  Jc
>>
>>
>>
>>                  On Mon, Sep 25, 2017 at 3:02 PM, JC Beyler <
>> jcbeyler at google.com
>>             <mailto:jcbeyler at google.com> <mailto:jcbeyler at google.com
>>             <mailto:jcbeyler at google.com>>> wrote:
>>
>>                      Forgot to say that for my numbers:
>>                        - Not in the test are the actual numbers I got for
>> the
>>             various array sizes, I ran the program 30 times and parsed the
>>             output; here are the averages and standard
>>                      deviation:
>>                             1000:     1.28% average; 1.13% standard
>> deviation
>>                             10000:    1.59% average; 1.25% standard
>> deviation
>>                             100000:   1.26% average; 1.26% standard
>> deviation
>>
>>                      The 1000/10000/100000 are the sizes of the arrays
>> being
>>             allocated. These are allocated 100k times and the sampling
>> rate is
>>             111 times the size of the array.
>>
>>                      Thanks!
>>                      Jc
>>
>>
>>                      On Mon, Sep 25, 2017 at 3:01 PM, JC Beyler
>>             <jcbeyler at google.com <mailto:jcbeyler at google.com>
>>             <mailto:jcbeyler at google.com <mailto:jcbeyler at google.com>>>
>> wrote:
>>
>>                          Hi all,
>>
>>                          After a bit of a break, I am back working on
>> this :).
>>             As before, here are two webrevs:
>>
>>                          - Full change set:
>>             http://cr.openjdk.java.net/~rasbold/8171119/webrev.09/
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.09/>
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.09/
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.09/>>
>>                          - Compared to version 8:
>>             http://cr.openjdk.java.net/~rasbold/8171119/webrev.08_09/
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.08_09/>
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.08_09/
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.08_09/>>
>>                               (This version is compared to version 8 I
>> last
>>             showed but ported to the new folder hierarchy)
>>
>>                          In this version I have:
>>                             - Handled Thomas' comments from his email of
>> 07/03:
>>                                  - Merged the logging to be standard
>>                                  - Fixed up the code a bit where asked
>>                                  - Added some notes about the code not
>> being
>>             thread-safe yet
>>                              - Removed additional dead code from the
>> version
>>             that modifies interpreter/c1/c2
>>                              - Fixed compiler issues so that it compiles
>> with
>>             --disable-precompiled-header
>>                                   - Tested with ./configure
>>             --with-boot-jdk=<jdk8> --with-debug-level=slowdebug
>>             --disable-precompiled-headers
>>
>>                          Additionally, I added a test to check the sanity
>> of the
>>             sampler: HeapMonitorStatCorrectnessTest
>>                                     (http://cr.openjdk.java.net/~r
>> asbold/8171119/webrev.08_09/test/hotspot/jtreg/serviceabilit
>> y/jvmti/HeapMonitor/MyPackage/HeapMonitorStatCorrectnessTest.java.patch
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.08_09/te
>> st/hotspot/jtreg/serviceability/jvmti/HeapMonitor/MyPackage/
>> HeapMonitorStatCorrectnessTest.java.patch>
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.08_09/te
>> st/hotspot/jtreg/serviceability/jvmti/HeapMonitor/MyPackage/
>> HeapMonitorStatCorrectnessTest.java.patch
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.08_09/te
>> st/hotspot/jtreg/serviceability/jvmti/HeapMonitor/MyPackage/
>> HeapMonitorStatCorrectnessTest.java.patch>>)
>>                              - This allocates a number of arrays and
>> checks that
>>             we obtain the number of samples we want with an accepted
>> error of
>>             5%. I tested it 100 times and it
>>                          passed everytime, I can test more if wanted
>>                              - Not in the test are the actual numbers I
>> got for
>>             the various array sizes, I ran the program 30 times and
>> parsed the
>>             output; here are the averages and
>>                          standard deviation:
>>                                 1000:     1.28% average; 1.13% standard
>> deviation
>>                                 10000:    1.59% average; 1.25% standard
>> deviation
>>                                 100000:   1.26% average; 1.26% standard
>> deviation
>>
>>                          What this means is that we were always at about
>> 1~2% of
>>             the number of samples the test expected.
>>
>>                          Let me know what you think,
>>                          Jc
>>
>>                          On Wed, Jul 5, 2017 at 9:31 PM, JC Beyler
>>             <jcbeyler at google.com <mailto:jcbeyler at google.com>
>>             <mailto:jcbeyler at google.com <mailto:jcbeyler at google.com>>>
>> wrote:
>>
>>                              Hi all,
>>
>>                              I apologize, I have not yet handled your
>> remarks
>>             but thought this new webrev would also be useful to see and
>> comment
>>             on perhaps.
>>
>>                              Here is the latest webrev, it is generated
>> slightly
>>             different than the others since now I'm using webrev.ksh
>> without the
>>             -N option:
>>             http://cr.openjdk.java.net/~rasbold/8171119/webrev.08/
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.08/>
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.08/
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.08/>>
>>
>>                              And the webrev.07 to webrev.08 diff is here:
>>             http://cr.openjdk.java.net/~rasbold/8171119/webrev.07_08/
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.07_08/>
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.07_08/
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.07_08/>>
>>
>>                              (Let me know if it works well)
>>
>>                              It's a small change between versions but it:
>>                                 - provides a fix that makes the average
>> sample
>>             rate correct (more on that below).
>>                                 - fixes the code to actually have it play
>> nicely
>>             with the fast tlab refill
>>                                 - cleaned up a bit the JVMTI text and now
>> use
>>             jvmtiFrameInfo
>>                              - moved the capability to be onload solo
>>
>>                              With this webrev, I've done a small study of
>> the
>>             random number generator we use here for the sampling rate. I
>> took a
>>             small program and it can be simplified to:
>>
>>                              for (outer loop)
>>                              for (inner loop)
>>                              int[] tmp = new int[arraySize];
>>
>>                              - I've fixed the outer and inner loops to
>> being 800
>>             for this experiment, meaning we allocate 640000 times an
>> array of a
>>             given array size.
>>
>>                              - Each program provides the average sample
>> size
>>             used for the whole execution
>>
>>                              - Then, I ran each variation 30 times and
>> then
>>             calculated the average of the average sample size used for
>> various
>>             array sizes. I selected the array size to
>>                              be one of the following: 1, 10, 100, 1000.
>>
>>                              - When compared to 512kb, the average sample
>> size
>>             of 30 runs:
>>                              1: 4.62% of error
>>                              10: 3.09% of error
>>                              100: 0.36% of error
>>                              1000: 0.1% of error
>>                              10000: 0.03% of error
>>
>>                              What it shows is that, depending on the
>> number of
>>             samples, the average does become better. This is because with
>> an
>>             allocation of 1 element per array, it
>>                              will take longer to hit one of the
>> thresholds. This
>>             is seen by looking at the sample count statistic I put in.
>> For the
>>             same number of iterations (800 *
>>                              800), the different array sizes provoke:
>>                              1: 62 samples
>>                              10: 125 samples
>>                              100: 788 samples
>>                              1000: 6166 samples
>>                              10000: 57721 samples
>>
>>                              And of course, the more samples you have,
>> the more
>>             sample rates you pick, which means that your average gets
>> closer
>>             using that math.
>>
>>                              Thanks,
>>                              Jc
>>
>>                              On Thu, Jun 29, 2017 at 10:01 PM, JC Beyler
>>             <jcbeyler at google.com <mailto:jcbeyler at google.com>
>>             <mailto:jcbeyler at google.com <mailto:jcbeyler at google.com>>>
>> wrote:
>>
>>                                  Thanks Robbin,
>>
>>                                  This seems to have worked. When I have
>> the next
>>             webrev ready, we will find out but I'm fairly confident it
>> will work!
>>
>>                                  Thanks agian!
>>                                  Jc
>>
>>                                  On Wed, Jun 28, 2017 at 11:46 PM, Robbin
>> Ehn
>>             <robbin.ehn at oracle.com <mailto:robbin.ehn at oracle.com>
>>             <mailto:robbin.ehn at oracle.com <mailto:robbin.ehn at oracle.com>>>
>> wrote:
>>
>>                                      Hi JC,
>>
>>                                      On 06/29/2017 12:15 AM, JC Beyler
>> wrote:
>>
>>                                          B) Incremental changes
>>
>>
>>                                      I guess the most common work flow
>> here is
>>             using mq :
>>                                      hg qnew fix_v1
>>                                      edit files
>>                                      hg qrefresh
>>                                      hg qnew fix_v2
>>                                      edit files
>>                                      hg qrefresh
>>
>>                                      if you do hg log you will see 2
>> commits
>>
>>                                      webrev.ksh -r -2 -o my_inc_v1_v2
>>                                      webrev.ksh -o my_full_v2
>>
>>
>>                                      In  your .hgrc you might need:
>>                                      [extensions]
>>                                      mq =
>>
>>                                      /Robbin
>>
>>
>>                                          Again another newbiew question
>> here...
>>
>>                                          For showing the incremental
>> changes, is
>>             there a link that explains how to do that? I apologize for my
>> newbie
>>             questions all the time :)
>>
>>                                          Right now, I do:
>>
>>                                             ksh ../webrev.ksh -m -N
>>
>>                                          That generates a webrev.zip and
>> send it
>>             to Chuck Rasbold. He then uploads it to a new webrev.
>>
>>                                          I tried commiting my change and
>> adding
>>             a small change. Then if I just do ksh ../webrev.ksh without
>> any
>>             options, it seems to produce a similar
>>                                          page but now with only the
>> changes I
>>             had (so the 06-07 comparison you were talking about) and a
>> changeset
>>             that has it all. I imagine that is
>>                                          what you meant.
>>
>>                                          Which means that my workflow
>> would become:
>>
>>                                          1) Make changes
>>                                          2) Make a webrev without any
>> options to
>>             show just the differences with the tip
>>                                          3) Amend my changes to my local
>> commit
>>             so that I have it done with
>>                                          4) Go to 1
>>
>>                                          Does that seem correct to you?
>>
>>                                          Note that when I do this, I only
>> see
>>             the full change of a file in the full change set (Side note
>> here:
>>             now the page says change set and not
>>                                          patch, which is maybe why
>> Serguei was
>>             having issues?).
>>
>>                                          Thanks!
>>                                          Jc
>>
>>
>>
>>                                          On Wed, Jun 28, 2017 at 1:12 AM,
>> Robbin
>>             Ehn <robbin.ehn at oracle.com <mailto:robbin.ehn at oracle.com>
>>             <mailto:robbin.ehn at oracle.com <mailto:robbin.ehn at oracle.com>>
>>             <mailto:robbin.ehn at oracle.com <mailto:robbin.ehn at oracle.com>
>>                                          <mailto:robbin.ehn at oracle.com
>>             <mailto:robbin.ehn at oracle.com>>>> wrote:
>>
>>                                               Hi,
>>
>>                                               On 06/28/2017 12:04 AM, JC
>> Beyler
>>             wrote:
>>
>>                                                   Dear Thomas et al,
>>
>>                                                   Here is the newest
>> webrev:
>>             http://cr.openjdk.java.net/~rasbold/8171119/webrev.07/
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.07/>
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.07/
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.07/>>
>>                                                     <
>> http://cr.openjdk.java.net/~rasbold/8171119/webrev.07/
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.07/>
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.07/
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.07/>>>
>>
>>
>>
>>                                               You have some more bits to
>> in
>>             there but generally this looks good and really nice with more
>> tests.
>>                                               I'll do and deep dive and
>> re-test
>>             this when I get back from my long vacation with whatever patch
>>             version you have then.
>>
>>                                               Also I think it's time you
>> provide
>>             incremental (v06->07 changes) as well as complete change-sets.
>>
>>                                               Thanks, Robbin
>>
>>
>>
>>
>>                                                   Thomas, I "think" I have
>>             answered all your remarks. The summary is:
>>
>>                                                   - The statistic system
>> is up
>>             and provides insight on what the heap sampler is doing
>>                                                        - I've noticed
>> that,
>>             though the sampling rate is at the right mean, we are missing
>> some
>>             samples, I have not yet tracked out why
>>                                          (details below)
>>
>>                                                   - I've run a tiny
>> benchmark
>>             that is the worse case: it is a very tight loop and allocated
>> a
>>             small array
>>                                                        - In this case, I
>> see no
>>             overhead when the system is off so that is a good start :)
>>                                                        - I see right now
>> a high
>>             overhead in this case when sampling is on. This is not a
>> really too
>>             surprising but I'm going to see if
>>                                          this is consistent with our
>>                                                   internal
>> implementation. The
>>             benchmark is really allocation stressful so I'm not too
>> surprised
>>             but I want to do the due diligence.
>>
>>                                                      - The statistic
>> system up
>>             is up and I have a new test
>>             http://cr.openjdk.java.net/~rasbold/8171119/webrev.07/test/
>> serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitorStatTest.java.patch
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.07/test/
>> serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitorStatTest.java.patch
>> >
>>                                                     <
>> http://cr.openjdk.java.net/~rasbold/8171119/webrev.07/test/
>> serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitorStatTest.java.patch
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.07/test/
>> serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitorStatTest.java.patch
>> >>
>>                                                              <
>> http://cr.openjdk.java.net/~rasbold/8171119/webrev.07/test
>> /serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitorStatT
>> est.java.patch
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.07/test/
>> serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitorStatTest.java.patch
>> >
>>                                                     <
>> http://cr.openjdk.java.net/~rasbold/8171119/webrev.07/test/
>> serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitorStatTest.java.patch
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.07/test/
>> serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitorStatTest.java.patch
>> >>>
>>                                                         - I did a bit of
>> a study
>>             about the random generator here, more details are below but
>>             basically it seems to work well
>>
>>                                                      - I added a
>> capability but
>>             since this is the first time doing this, I was not sure I did
>> it right
>>                                                        - I did add a test
>> though
>>             for it and the test seems to do what I expect (all methods are
>>             failing with the
>>                                          JVMTI_ERROR_MUST_POSSESS_CAPABILITY
>> error).
>>                                                            -
>>             http://cr.openjdk.java.net/~rasbold/8171119/webrev.07/test/
>> serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitorNoCapa
>> bilityTest.java.patch
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.07/test/
>> serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitorNoCapa
>> bilityTest.java.patch>
>>                                                     <
>> http://cr.openjdk.java.net/~rasbold/8171119/webrev.07/test/
>> serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitorNoCapa
>> bilityTest.java.patch
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.07/test/
>> serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitorNoCapa
>> bilityTest.java.patch>>
>>
>>               <http://cr.openjdk.java.net/~r
>> asbold/8171119/webrev.07/test/serviceability/jvmti/HeapMonit
>> or/MyPackage/HeapMonitorNoCapabilityTest.java.patch
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.07/test/
>> serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitorNoCapa
>> bilityTest.java.patch>
>>                                                     <
>> http://cr.openjdk.java.net/~rasbold/8171119/webrev.07/test/
>> serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitorNoCapa
>> bilityTest.java.patch
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.07/test/
>> serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitorNoCapa
>> bilityTest.java.patch>>>
>>
>>                                                      - I still need to
>> figure
>>             out what to do about the multi-agent vs single-agent issue
>>
>>                                                      - As far as
>> measurements,
>>             it seems I still need to look at:
>>                                                        - Why we do the 20
>> random
>>             calls first, are they necessary?
>>                                                        - Look at the mean
>> of the
>>             sampling rate that the random generator does and also what is
>>             actually sampled
>>                                                        - What is the
>> overhead in
>>             terms of memory/performance when on?
>>
>>                                                   I have inlined my
>> answers, I
>>             think I got them all in the new webrev, let me know your
>> thoughts.
>>
>>                                                   Thanks again!
>>                                                   Jc
>>
>>
>>                                                   On Fri, Jun 23, 2017 at
>> 3:52
>>             AM, Thomas Schatzl <thomas.schatzl at oracle.com
>>             <mailto:thomas.schatzl at oracle.com> <mailto:
>> thomas.schatzl at oracle.com
>>             <mailto:thomas.schatzl at oracle.com>>
>>                                          <mailto:thomas.schatzl at oracle.
>> com
>>             <mailto:thomas.schatzl at oracle.com> <mailto:
>> thomas.schatzl at oracle.com
>>             <mailto:thomas.schatzl at oracle.com>>>
>>             <mailto:thomas.schatzl at oracle.com <mailto:
>> thomas.schatzl at oracle.com>
>>             <mailto:thomas.schatzl at oracle.com <mailto:
>> thomas.schatzl at oracle.com>>
>>
>>                                                              <mailto:
>> thomas.schatzl at oracle.com <mailto:thomas.schatzl at oracle.com>
>>             <mailto:thomas.schatzl at oracle.com
>>             <mailto:thomas.schatzl at oracle.com>>>>> wrote:
>>
>>                                                        Hi,
>>
>>                                                        On Wed, 2017-06-21
>> at
>>             13:45 -0700, JC Beyler wrote:
>>                                                        > Hi all,
>>                                                        >
>>                                                        > First off:
>> Thanks again
>>             to Robbin and Thomas for their reviews :)
>>                                                        >
>>                                                        > Next, I've
>> uploaded a
>>             new webrev:
>>                                                        >
>>             http://cr.openjdk.java.net/~rasbold/8171119/webrev.06/
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.06/>
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.06/
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.06/>>
>>                                                     <
>> http://cr.openjdk.java.net/~rasbold/8171119/webrev.06/
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.06/>
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.06/
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.06/>>>
>>                                                              <
>> http://cr.openjdk.java.net/~rasbold/8171119/webrev.06/
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.06/>
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.06/
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.06/>>
>>                                                     <
>> http://cr.openjdk.java.net/~rasbold/8171119/webrev.06/
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.06/>
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.06/
>>             <http://cr.openjdk.java.net/~rasbold/8171119/webrev.06/>>>>
>>
>>                                                        >
>>                                                        > Here is an
>> update:
>>                                                        >
>>                                                        > - @Robbin, I
>> forgot to
>>             say that yes I need to look at implementing
>>                                                        > this for the
>> other
>>             architectures and testing it before it is all
>>                                                        > ready to go. Is
>> it
>>             common to have it working on all possible
>>                                                        > combinations or
>> is
>>             there a subset that I should be doing first and we
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20171025/5a0c311b/attachment-0001.html>


More information about the serviceability-dev mailing list