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