<div dir="ltr">Hi all,<br><div><br></div><div>A question came up between myself and Serguei about how to protect the newly allocated oop when the collector does the callback. We decided it might be best to ask the mailing list for help/guidance/opinion?</div><div><br></div><div> Consider the changes done in this file for example:</div><div><a href="http://cr.openjdk.java.net/~jcbeyler/8171119/heap_event.16/src/hotspot/share/gc/shared/collectedHeap.inline.hpp.udiff.html">http://cr.openjdk.java.net/~jcbeyler/8171119/heap_event.16/src/hotspot/share/gc/shared/collectedHeap.inline.hpp.udiff.html</a><br></div><div><br></div><div>For example, for obj_allocate, the change becomes:</div><div><div> oop CollectedHeap::obj_allocate(Klass* klass, int size, TRAPS) {</div><div>   debug_only(check_for_valid_allocation_state());</div><div>   assert(!Universe::heap()->is_gc_active(), "Allocation during gc not allowed");</div><div>   assert(size >= 0, "int won't convert to size_t");</div><div>+</div><div>+  HandleMark hm(THREAD);</div><div>+  Handle result;</div><div>+  {</div><div>+    JvmtiSampledObjectAllocEventCollector collector;</div><div>   HeapWord* obj = common_mem_allocate_init(klass, size, CHECK_NULL);</div><div>   post_allocation_setup_obj(klass, obj, size);</div><div>   NOT_PRODUCT(Universe::heap()->check_for_bad_heap_word_value(obj, size));</div><div>-  return (oop)obj;</div><div>+    result = Handle(THREAD, (oop) obj);</div><div>+  }</div><div>+  return result();</div><div> }</div></div><div><br></div><div>The question is: does anyone see an issue here in terms of performance or something we missed? When I measured it via the Dacapo run, I saw no performance degradation but I wanted to double check with you all if this would become a big no no for the final webrev?</div><div><br></div><div>Were other benchmarks show that there is no overhead incurred, would this be ok?</div><div><br></div><div>Thanks for your help,</div><div>Jc</div></div><br><div class="gmail_quote"><div dir="ltr">On Tue, Apr 17, 2018 at 9:51 PM Jeremy Manson <<a href="mailto:jeremymanson@google.com">jeremymanson@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Great, thanks!<div><br></div><div>Jeremy</div></div><br><div class="gmail_quote"><div dir="ltr">On Tue, Apr 17, 2018 at 2:01 PM <a href="mailto:serguei.spitsyn@oracle.com" target="_blank">serguei.spitsyn@oracle.com</a> <<a href="mailto:serguei.spitsyn@oracle.com" target="_blank">serguei.spitsyn@oracle.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
  
    
  
  <div text="#000000" bgcolor="#FFFFFF">
    <div class="m_-1011140630431552159m_9203410791999666704moz-cite-prefix">Hi Jeremy,<br>
      <br>
      We had a private discussion with Jc on this and decided the same.<br>
      We would like to sample all allocations if possible and on a low
      level.<br>
      <br>
      Thanks,<br>
      Serguei<br>
      <br>
      <br>
      On 4/17/18 12:38, Jeremy Manson wrote:<br>
    </div>
    <blockquote type="cite">
      
      <div dir="ltr">+1 to sampling anything the thread is allocating. 
        With the bytecode rewriting based version of this, I had
        complaints about missing allocations from JNI and APIs like
        Class.newInstance.  (I don't know how the placement of the
        collectors would affect this, but it did matter).<br>
        <div><br>
        </div>
        <div>Jeremy</div>
      </div>
      <br>
      <div class="gmail_quote">
        <div dir="ltr">On Thu, Apr 12, 2018 at 2:23 PM JC Beyler <<a href="mailto:jcbeyler@google.com" target="_blank">jcbeyler@google.com</a>> wrote:<br>
        </div>
        <blockquote class="gmail_quote">
          <div dir="ltr">Hi Karen,<br>
            <div><br>
            </div>
            <div>I apologize for sending too many webrevs. I try/tend to
              iterate fast and move in an iterative fashion. I also try
              to solve most, if not all, of the current items that are
              requested in one go. Perhaps I failed in doing that
              recently? I apologize for that.</div>
            <div><br>
            </div>
            <div>So I promise to not send a new webrev in this email or
              until I'm pretty sure I got all the current (And any
              incoming comment/reviews) handled :-)</div>
            <div><br>
            </div>
            <div>For the points you brought up:</div>
            <div>   a) What are we sampling? In my mind, I'd rather have
              the sampler be sampling anything the thread is allocating
              and not only sample bytecode allocations. It turns out
              that I was focusing on that first to get it up. As I was
              stuck in figuring out how to get the VM collector and the
              sampling collector to co-exist, there was a bit of issues
              there.</div>
            <div>      - That has been solved by now delaying the
              posting of a sampled object if a VM collector is present.
              So now that I've better understood interactions between
              collectors and when you could post an event, I'm way more
              able to talk about the feasibility and validity of the
              next item about bigger objects. </div>
            <div><br>
            </div>
            <div>   b) You bring up an excellent point of if we have a
              multi-array object or a more complex object (such as a
              cloned object for example), if the sampler is tripped on
              an internal allocation, should we send that smaller
              allocation or should we send the bigger object</div>
            <div>   - Because we get the stacktrace and we only use the
              oop to figure out GC information about the liveness of the
              object in our use-case in the JVMTI agent, this changes
              nothing really in practice. I do see value in sending the
              multi-array object as a whole to a user.</div>
            <div>      - If that is what you think is best, I can work
              on getting that supported and the multi-array test would
              then prove that if part of the multi-array is sampled, the
              sampler returns the whole multi-array.</div>
            <div><br>
            </div>
            <div>Hopefully that answers your concern on me sending too
              many webrevs, to which I sincerely apologize. Probably a
              learning curve of different approaches of reviews. And I
              hope that my other answers do show the direction you were
              hoping to see.</div>
            <div><br>
            </div>
            <div>Thanks again for all your help,</div>
            <div>Jc</div>
          </div>
          <br>
          <div class="gmail_quote">
            <div dir="ltr">On Thu, Apr 12, 2018 at 8:15 AM Karen Kinnear
              <<a href="mailto:karen.kinnear@oracle.com" target="_blank">karen.kinnear@oracle.com</a>>
              wrote:<br>
            </div>
            <blockquote class="gmail_quote">
              <div>JC,
                <div><br>
                  <div><br>
                    <blockquote type="cite">
                      <div>On Apr 11, 2018, at 8:17 PM, JC Beyler <<a href="mailto:jcbeyler@google.com" target="_blank">jcbeyler@google.com</a>>
                        wrote:</div>
                      <br class="m_-1011140630431552159m_9203410791999666704m_-2827008458729093471m_-1152325375000192003m_8368332985884548048Apple-interchange-newline">
                      <div>
                        <div dir="ltr">Hi Karen,<br>
                          <br>
                          I put up a new webrev that is feature complete
                          in my mind in terms of implementation. There
                          could be a few tid-bits of optimizations here
                          and there but I believe everything is now
                          there in terms of features and there is the
                          question of placement of collectors (I've now
                          put it lower than what you talk about in this
                          email thread).</div>
                      </div>
                    </blockquote>
                    I believe that the primary goal of your JEP is to
                    catch samples of allocation due to bytecodes. Given
                    that, it makes sense to</div>
                  <div>put the collectors in the code generators, so I
                    am ok with your leaving them where they are.</div>
                  <div><br>
                  </div>
                  <div>
                    <div>And it would save us all a lot of cycles if
                      rather than frequent webrev updates with subsets
                      of the requested changes - if you</div>
                    <div>could wait until you’ve added all the changes
                      people requested - then that would increase the
                      signal to noise ratio.</div>
                    <blockquote type="cite">
                      <div>
                        <div dir="ltr">
                          <div><br>
                          </div>
                          <div>The incremental webrev is here:
                            <div><a href="http://cr.openjdk.java.net/%7Ejcbeyler/8171119/heap_event.12_13/" target="_blank">http://cr.openjdk.java.net/~jcbeyler/8171119/heap_event.12_13/</a><br>
                            </div>
                            <div>and the full webrev is here:</div>
                            <div><a href="http://cr.openjdk.java.net/%7Ejcbeyler/8171119/heap_event.13/" target="_blank">http://cr.openjdk.java.net/~jcbeyler/8171119/heap_event.13/</a><br>
                            </div>
                            <div><br>
                            </div>
                            <div>The incremental webrev contains the
                              name change of TLAB fields as per the
                              conversation going on in the GC thread and
                              the Graal changes to make this webrev not
                              break anything. It also contains a test
                              for when VM and sampled events are enabled
                              at the same time. </div>
                            <div><br>
                            </div>
                            <div>I'll answer your questions inlined
                              below:</div>
                            <div><br>
                              <div>
                                <div class="gmail_quote">
                                  <blockquote class="gmail_quote">
                                    <div>
                                      <div dir="auto"><br>
                                        <div><br>
                                        </div>
                                        <div>I have a couple of design
                                          questions before getting to
                                          the detailed code review:</div>
                                        <div><br>
                                        </div>
                                        <div>1. jvmtiExport.cpp</div>
                                        <div>You have simplified the
                                          JvmtiObjectAllocEventCollector
                                          to assume there is only a
                                          single object.</div>
                                        <div>Do you have a test that
                                          allocates a multi-dimensional
                                          array?</div>
                                        <div>I would expect that to have
                                          multiple subarrays  - and need
                                          the logic that you removed.</div>
                                      </div>
                                    </div>
                                  </blockquote>
                                  <div><br>
                                  </div>
                                  <div>I "think" you misread this. I did
                                    not change the implementation of <span>JvmtiObjectAllocEventCollector
                                      to assume only one object.
                                      Actually the implementation is
                                      still doing what it was doing
                                      initially but now
                                      JvmtiObjectAllocEventCollector is
                                      a main class with two inherited
                                      behaviors:</span></div>
                                  <div><span>   -
                                      JvmtiVMObjectAllocEventCollector
                                      is following the same logic as
                                      before</span></div>
                                  <div>   -
                                    JvmtiSampledObjectAllocEventCollector
                                    has the logic of a single allocation
                                    during collection since it is placed
                                    that low. I'm thinking of perhaps
                                    separating the two classes now that
                                    the sampled case is so low and does
                                    not require the extra logic of
                                    handling a growable array.</div>
                                  <div><br>
                                  </div>
                                  <div>I don't have a test that tests
                                    multi-dimensional array. I have not
                                    looked at what exactly
                                    VMObjectAllocEvents track but I did
                                    do an example to see:</div>
                                  <div><br>
                                  </div>
                                  <div>- On a clean JDK, if I allocate:</div>
                                  <div>     int[][][] myAwesomeTable =
                                    new int[10][10][10];</div>
                                  <div><br>
                                  </div>
                                  <div>I get one single VMObject call
                                    back.</div>
                                </div>
                              </div>
                            </div>
                          </div>
                        </div>
                      </div>
                    </blockquote>
                    <blockquote type="cite">
                      <div>
                        <div dir="ltr">
                          <div>
                            <div>
                              <div>
                                <div class="gmail_quote">
                                  <div><br>
                                  </div>
                                  <div>- With my change, I get the same
                                    behavior.</div>
                                  <div><br>
                                  </div>
                                  <div>So instead, I did an object
                                    containing an array and cloned it.
                                    With the clean JDK I get two VM
                                    callbacks with and without my
                                    change. <br>
                                  </div>
                                  <div><br>
                                  </div>
                                  <div>I'll add a test in the next
                                    webrev to ensure correctness.</div>
                                  <div><br>
                                  </div>
                                  <blockquote class="gmail_quote">
                                    <div>
                                      <div dir="auto">
                                        <div><br>
                                        </div>
                                        <div>*** Please add a test in
                                          which you allocate a
                                          multi-dimensional array - and
                                          try it with your earlier
                                          version which</div>
                                        <div>will show the full set of
                                          allocated objects</div>
                                      </div>
                                    </div>
                                  </blockquote>
                                  <div><br>
                                  </div>
                                  <div>As I said, this works because it
                                    is so low in the system. I don't
                                    know the internals of the allocation
                                    of a three-dimensional array but:</div>
                                  <div>  - If I try to collect all
                                    allocations required for the <span>new
                                      int[10][10][10], I get more than a
                                      hundred allocation callbacks if
                                      the sample rate is 0, which is
                                      what I'd expect (getting a
                                      callback at each allocation, so
                                      what I expect)</span></div>
                                  <div> </div>
                                  <div>So though I only collect one
                                    object, I get a callback for each if
                                    that is what we want in regard to
                                    the sampling rate.</div>
                                </div>
                              </div>
                            </div>
                          </div>
                        </div>
                      </div>
                    </blockquote>
                    I’ll let you work this one out with Serguei - if he
                    is ok with your change to not have a growable array
                    and only report one object,</div>
                  <div>given that you are sampling, sounds like that is
                    not a functional loss for you.</div>
                  <div><br>
                  </div>
                  <div>And yes, please do add the test.</div>
                  <div>
                    <blockquote type="cite">
                      <div>
                        <div dir="ltr">
                          <div>
                            <div>
                              <div>
                                <div class="gmail_quote">
                                  <div><br>
                                  </div>
                                  <blockquote class="gmail_quote">
                                    <div>
                                      <div dir="auto">
                                        <div><br>
                                        </div>
                                        <div>2. Tests - didn’t read them
                                          all - just ran into a
                                          hardcoded “10% error ensures a
                                          sanity test without becoming
                                          flaky”</div>
                                        <div>do check with Serguei on
                                          this - that looks like a
                                          potential future test failure</div>
                                      </div>
                                    </div>
                                  </blockquote>
                                  <div><br>
                                  </div>
                                  <div>Because the sampling rate is a
                                    geometric variable around a mean of
                                    the sampling rate, any meaningful
                                    test is going to have to be
                                    statistical. Therefore, I do a bit
                                    of error acceptance to allow us to
                                    test for the real thing and not hack
                                    the code to have less "real" tests.
                                    This is what we do internally, let
                                    me know if you want me to do it
                                    otherwise.</div>
                                  <div><br>
                                  </div>
                                  <div>Flaky is probably a wrong term or
                                    perhaps I need to better explain
                                    this. I'll change the comment in the
                                    tests to explain that potentially
                                    flakyness comes from the nature of
                                    the geometrical mean. Because we
                                    don't want too long running tests,
                                    it makes sense to me to have this
                                    error percentage.</div>
                                  <div><br>
                                  </div>
                                  <div>Let me now answer the comments
                                    from the other email here as well so
                                    we have all answers and
                                    conversations in a single thread:</div>
                                  <blockquote class="gmail_quote">
                                    <div>
                                      <div dir="auto">
                                        <div>
                                          <div>
                                            <blockquote type="cite">
                                              <div>
                                                <div dir="ltr">
                                                  <div>
                                                    <div>        - Note
                                                      there is one
                                                      current caveat: if
                                                      the agent requests
                                                      the VMObjectAlloc
                                                      event, the sampler
                                                      defers to that
                                                      event due to a
                                                      limitation in its
                                                      implementation
                                                      (ie: I am not
                                                      convinced I can
                                                      safely send out
                                                      that event with
                                                      the VM collector
                                                      enabled, I'll
                                                      happily white
                                                      board that).</div>
                                                  </div>
                                                </div>
                                              </div>
                                            </blockquote>
                                            Please work that one out
                                            with Serguei and the
                                            serviceability folks.</div>
                                          <div><br>
                                          </div>
                                        </div>
                                      </div>
                                    </div>
                                  </blockquote>
                                  <div><br>
                                  </div>
                                  <div>Agreed. I'll follow up with
                                    Serguei if this is a potential
                                    problem, I have to double check and
                                    ensure I am right that there is an
                                    issue. I see it as just a matter of
                                    life and not a problem for now. IF
                                    you do want both events, having a
                                    sample drop due to this limitation
                                    does not invalidate the system in my
                                    mind. I could be wrong about it
                                    though and would happily go over
                                    what I saw.</div>
                                  <div>   </div>
                                  <div><br>
                                  </div>
                                  <blockquote class="gmail_quote">
                                    <div>
                                      <div dir="auto">
                                        <div>
                                          <blockquote type="cite">
                                            <div dir="ltr">
                                              <div class="gmail_quote">
                                                <blockquote class="gmail_quote">
                                                  <div dir="ltr">
                                                    <div class="gmail_quote">
                                                      <div><br>
                                                      </div>
                                                      <div>So the Heap
                                                        Sampling
                                                        Monitoring
                                                        System used to
                                                        have more
                                                        methods. It made
                                                        sense to have
                                                        them in a
                                                        separate
                                                        category. I now
                                                        have moved it to
                                                        the memory
                                                        category to be
                                                        consistent and
                                                        grouped there. I
                                                        also removed
                                                        that link btw. </div>
                                                    </div>
                                                  </div>
                                                </blockquote>
                                              </div>
                                            </div>
                                          </blockquote>
                                          Thanks.<br>
                                        </div>
                                      </div>
                                    </div>
                                  </blockquote>
                                  <div><br>
                                  </div>
                                  <div>Actually it did seem weird to put
                                    it there since there was only
                                    Allocate/Deallocate, so for now the
                                    method is still again in its own
                                    category once again. If someone has
                                    a better spot, let me know.</div>
                                  <div><br>
                                  </div>
                                  <blockquote class="gmail_quote">
                                    <div>
                                      <div dir="auto">
                                        <div>
                                          <blockquote type="cite">
                                            <div>
                                              <div dir="ltr">
                                                <div>
                                                  <div class="gmail_quote">
                                                    <blockquote class="gmail_quote">
                                                      <div dir="ltr">
                                                        <div>
                                                          <div class="gmail_quote">
                                                          <div> </div>
                                                          <blockquote class="gmail_quote">
                                                          <div>
                                                          <div>
                                                          <div><br>
                                                          </div>
                                                          <div>I was
                                                          trying to
                                                          figure out a
                                                          way to put the
                                                          collectors
                                                          farther down
                                                          the call stack
                                                          so as to both
                                                          catch more</div>
                                                          <div>cases and
                                                          to reduce the
                                                          maintenance
                                                          burden - i.e.
                                                          if you were to
                                                          add a new code
                                                          generator,
                                                          e.g. Graal -</div>
                                                          <div>if it
                                                          were to go
                                                          through an
                                                          existing
                                                          interface,
                                                          that might be
                                                          a place to
                                                          already have a
                                                          collector.</div>
                                                          <div><br>
                                                          </div>
                                                          <div>I do not
                                                          know the Graal
                                                          sources - I
                                                          did look at
                                                          jvmci/jvmciRuntime.cpp
                                                          - and it
                                                          appears that
                                                          there</div>
                                                          <div>are calls
                                                          to
                                                          instanceKlass::new_instance,
oopFactory::new_typeArray/new_ObjArray and ArrayKlass::multi-allocate,</div>
                                                          <div>so one
                                                          possibility
                                                          would be to
                                                          put hooks in
                                                          those calls
                                                          which would
                                                          catch many? (I
                                                          did not do a
                                                          thorough
                                                          search)</div>
                                                          <div>of the
                                                          slowpath calls
                                                          for the
                                                          bytecodes, and
                                                          then check the
                                                          fast paths in
                                                          detail.</div>
                                                          </div>
                                                          </div>
                                                          </blockquote>
                                                          <div><br>
                                                          </div>
                                                          <div>I'll come
                                                          to a major
                                                          issue with the
                                                          collector and
                                                          its placement
                                                          in the next
                                                          paragraph.</div>
                                                          </div>
                                                        </div>
                                                      </div>
                                                    </blockquote>
                                                  </div>
                                                </div>
                                              </div>
                                            </div>
                                          </blockquote>
                                          Still not clear on why you did
                                          not move the collectors into
                                          instanceKlass::new_instance
                                          and
                                          oopFactory::newtypeArray/newObjArray</div>
                                        <div>and
                                          ArrayKlass::multi-allocate.<br>
                                        </div>
                                      </div>
                                    </div>
                                  </blockquote>
                                </div>
                              </div>
                            </div>
                          </div>
                        </div>
                      </div>
                    </blockquote>
                    As I said above, I am ok with your leaving the
                    collectors in the code generators since that is your
                    focus.<br>
                    <blockquote type="cite">
                      <div>
                        <div dir="ltr">
                          <div>
                            <div>
                              <div>
                                <div class="gmail_quote">
                                  <div><br>
                                  </div>
                                  <div>I think what was happening is
                                    that the collectors would wrap the
                                    objects in handles but references to
                                    the originally allocated object
                                    would be on the stack still and
                                    would not get updated if required.
                                    Due to that issue, I believe was
                                    getting weird bugs. </div>
                                  <div><br>
                                  </div>
                                  <div>Because of this, it seems that
                                    any VM collector enabled has to
                                    guarantee that either:</div>
                                  <div>   - its path to destruction (and
                                    thus posting of events) has no means
                                    of triggering a GC that would move
                                    things around (as long as you are in
                                    VM code you should be fine I
                                    believe)</div>
                                </div>
                              </div>
                            </div>
                          </div>
                        </div>
                      </div>
                    </blockquote>
                    <blockquote type="cite">
                      <div>
                        <div dir="ltr">
                          <div>
                            <div>
                              <div>
                                <div class="gmail_quote">
                                  <div>   - if GC is occuring, the
                                    objects in its internal array are
                                    not somewhere on the stack without a
                                    handle around them to be able to be
                                    moved if need by from a GC
                                    operation.</div>
                                </div>
                              </div>
                            </div>
                          </div>
                        </div>
                      </div>
                    </blockquote>
                    <blockquote type="cite">
                      <div>
                        <div dir="ltr">
                          <div>
                            <div>
                              <div>
                                <div class="gmail_quote">
                                  <div><br>
                                  </div>
                                  <div>I'm not convinced this holds in
                                    the multithreaded with sampling and
                                    VM collection cases.</div>
                                </div>
                              </div>
                            </div>
                          </div>
                        </div>
                      </div>
                    </blockquote>
                    I will let you and Serguei work out whether you have
                    sufficient test coverage for the multithreaded
                    cases.</div>
                  <div><br>
                  </div>
                  <div>thanks,</div>
                  <div>Karen<br>
                    <blockquote type="cite">
                      <div>
                        <div dir="ltr">
                          <div>
                            <div>
                              <div>
                                <div class="gmail_quote">
                                  <div><br>
                                  </div>
                                  <blockquote class="gmail_quote">
                                    <div>
                                      <div dir="auto">
                                        <div>
                                          <blockquote type="cite">
                                            <div>
                                              <div dir="ltr">
                                                <div>
                                                  <div class="gmail_quote">
                                                    <blockquote class="gmail_quote">
                                                      <div dir="ltr">
                                                        <div>
                                                          <div class="gmail_quote">
                                                          <div> </div>
                                                          <blockquote class="gmail_quote">
                                                          <div>
                                                          <div>
                                                          <div><br>
                                                          </div>
                                                          <div>I had
                                                          wondered if it
                                                          made sense to
                                                          move the hooks
                                                          even farther
                                                          down, into
                                                          CollectedHeap:obj_allocate
                                                          and
                                                          array_allocate.</div>
                                                          <div>I do not
                                                          think so.
                                                          First reason
                                                          is that for
                                                          multidimensional
                                                          arrays,
                                                          ArrayKlass::multi_allocate
                                                          the outer
                                                          dimension
                                                          array would</div>
                                                          <div>have an
                                                          event before
                                                          storing the
                                                          inner
                                                          sub-arrays and
                                                          I don’t think
                                                          we want that
                                                          exposed, so
                                                          that won’t
                                                          work for
                                                          arrays.</div>
                                                          </div>
                                                          </div>
                                                          </blockquote>
                                                          <div><br>
                                                          </div>
                                                          <div>So the
                                                          major
                                                          difficulty is
                                                          that the steps
                                                          of collection
                                                          do this:</div>
                                                          <div><br>
                                                          </div>
                                                          <div>- An
                                                          object gets
                                                          allocated and
                                                          is decided to
                                                          be sampled</div>
                                                          <div>- The
                                                          original
                                                          pointer
                                                          placement
                                                          (where it
                                                          resides
                                                          originally in
                                                          memory) is
                                                          passed to the
                                                          collector</div>
                                                          <div>- Now one
                                                          important
                                                          thing of note:</div>
                                                          <div>    (a)
                                                          In the VM
                                                          code, until
                                                          the point
                                                          where the oop
                                                          is going to be
                                                          returned, GC
                                                          is not yet
                                                          aware of it</div>
                                                          </div>
                                                        </div>
                                                      </div>
                                                    </blockquote>
                                                  </div>
                                                </div>
                                              </div>
                                            </div>
                                          </blockquote>
                                          <blockquote type="cite">
                                            <div>
                                              <div dir="ltr">
                                                <div>
                                                  <div class="gmail_quote">
                                                    <blockquote class="gmail_quote">
                                                      <div dir="ltr">
                                                        <div>
                                                          <div class="gmail_quote">
                                                          <div>    (b)
                                                          so the
                                                          collector
                                                          can't yet send
                                                          it out to the
                                                          user via JVMTI
                                                          otherwise, the
                                                          agent could
                                                          put a weak
                                                          reference for
                                                          example</div>
                                                          <div><br>
                                                          </div>
                                                          <div>I'm a bit
                                                          fuzzy on this
                                                          and maybe it's
                                                          just that
                                                          there would be
                                                          more heavy
                                                          lifting to
                                                          make this
                                                          possible but
                                                          my initial
                                                          tests seem to
                                                          show problems
                                                          when
                                                          attempting
                                                          this in the
                                                          obj_allocate
                                                          area.</div>
                                                          </div>
                                                        </div>
                                                      </div>
                                                    </blockquote>
                                                  </div>
                                                </div>
                                              </div>
                                            </div>
                                          </blockquote>
                                          Not sure what you are seeing
                                          here - </div>
                                        <div><br>
                                        </div>
                                        <div>
                                          <div>Let me state it the way I
                                            understand it.</div>
                                          <div>1) You can collect the
                                            object into internal
                                            metadata at allocation point
                                            - which you already had</div>
                                          <div>Note: see comment in
                                            JvmtiExport.cpp: </div>
                                          <div>// In the case of the
                                            sampled object collector, we
                                            don’t want to perform the</div>
                                          <div>// oops_do because the
                                            object in the collector is
                                            still stored in registers </div>
                                          <div>// on the VM stack</div>
                                          <div>   - so GC will find
                                            these objects as roots, once
                                            we allow GC to run, which
                                            should be after the header
                                            is initialized</div>
                                          <div><br>
                                          </div>
                                          <div>Totally agree with you
                                            that you can not post the
                                            event in the source code in
                                            which you allocate the
                                            memory - keep reading</div>
                                          <div><br>
                                          </div>
                                          <div>2) event posting:</div>
                                          <div>   - you want to ensure
                                            that the object has been
                                            fully initialized</div>
                                          <div>   - the object needs to
                                            have the header set up - not
                                            just the memory allocated -
                                            so that applies to all
                                            objects</div>
                                          <div>   (and that is done in a
                                            caller of the allocation
                                            code - so it can’t be done
                                            at the location which does
                                            the memory allocation)</div>
                                          <div>   - the example I
                                            pointed out was the
                                            multianewarray case - all
                                            the subarrays need to be
                                            allocated</div>
                                          <div><br>
                                          </div>
                                          <div>- please add a test case
                                            for multi-array, so as you
                                            experiment with where to
                                            post the event, you ensure
                                            that you can</div>
                                          <div>access the subarrays
                                            (e.g. a 3D array of length 5
                                            has 5 2D arrays as
                                            subarrays)</div>
                                        </div>
                                      </div>
                                    </div>
                                  </blockquote>
                                  <div><br>
                                  </div>
                                  <div>Technically it's more than just
                                    initialized, it is the fact that you
                                    cannot perform a callback about an
                                    object if any object of that thread
                                    is being held by a collector and
                                    also in a register/stack space
                                    without protections.</div>
                                  <div><br>
                                  </div>
                                  <div> </div>
                                  <blockquote class="gmail_quote">
                                    <div>
                                      <div dir="auto">
                                        <div>
                                          <div>  - prior to setting up
                                            the object header
                                            information, GC would not
                                            know about the object</div>
                                          <div>    - was this  by chance
                                            the issue you ran into?</div>
                                        </div>
                                      </div>
                                    </div>
                                  </blockquote>
                                  <div><br>
                                  </div>
                                  <div>No I believe the issue I was
                                    running into was above where an
                                    object on the stack was pointing to
                                    an oop that got moved during a GC
                                    due to that thread doing an event
                                    callback.</div>
                                  <div> </div>
                                  <div>Thanks for your help,</div>
                                  <div>Jc</div>
                                  <div><br>
                                  </div>
                                  <blockquote class="gmail_quote">
                                    <div>
                                      <div dir="auto">
                                        <div>
                                          <div><br>
                                          </div>
                                          <div>3) event posting</div>
                                          <div>   - when you post the
                                            event to JVMTI</div>
                                          <div>   - in
                                            JvmtiObjectAllocEventMark:
                                            sets _jobj
                                            (object)to_jobject(obj),
                                            which creates
                                            JNIHandles::make_local(_thread,
                                            obj)</div>
                                          <blockquote type="cite">
                                            <div>
                                              <div dir="ltr">
                                                <div>
                                                  <div class="gmail_quote">
                                                    <blockquote class="gmail_quote">
                                                      <div dir="ltr">
                                                        <div>
                                                          <div class="gmail_quote">
                                                          <div> </div>
                                                          <blockquote class="gmail_quote">
                                                          <div>
                                                          <div>
                                                          <div><br>
                                                          </div>
                                                          <div>The
                                                          second reason
                                                          is that I
                                                          strongly
                                                          suspect the
                                                          scope you want
                                                          is bytecodes
                                                          only. I think
                                                          once you have
                                                          added hooks</div>
                                                          <div>to all
                                                          the fast paths
                                                          and slow paths
                                                          that this will
                                                          be pushing the
                                                          performance
                                                          overhead
                                                          constraints
                                                          you proposed
                                                          and</div>
                                                          <div>you won’t
                                                          want to see
                                                          e.g. internal
                                                          allocations. </div>
                                                          </div>
                                                          </div>
                                                          </blockquote>
                                                          <div><br>
                                                          </div>
                                                          <div>Yes
                                                          agreed,
                                                          allocations
                                                          from bytecodes
                                                          are mostly our
                                                          concern
                                                          generally :)</div>
                                                          <div> </div>
                                                          <blockquote class="gmail_quote">
                                                          <div>
                                                          <div>
                                                          <div> <br>
                                                          </div>
                                                          </div>
                                                          </div>
                                                          </blockquote>
                                                          <blockquote class="gmail_quote">
                                                          <div>
                                                          <div>
                                                          <div>But I
                                                          think you need
                                                          to experiment
                                                          with the set
                                                          of allocations
                                                          (or possible
                                                          alternative
                                                          sets of
                                                          allocations)
                                                          you want
                                                          recorded.</div>
                                                          <div><br>
                                                          </div>
                                                          <div>The hooks
                                                          I see today
                                                          include:</div>
                                                          <div>Interpreter:
                                                          (looking at
                                                          x86 as a
                                                          sample)</div>
                                                          <div>  -
                                                          slowpath in
                                                          InterpreterRuntime</div>
                                                          <div>  -
                                                          fastpath tlab
                                                          allocation -
                                                          your new
                                                          threshold
                                                          check handles
                                                          that</div>
                                                          </div>
                                                          </div>
                                                          </blockquote>
                                                          <div><br>
                                                          </div>
                                                          <div>Agreed<br>
                                                          </div>
                                                          <div> </div>
                                                          <blockquote class="gmail_quote">
                                                          <div>
                                                          <div>
                                                          <div>  -
                                                          allow_shared_alloc
                                                          (GC specific):
                                                          for _new isn’t
                                                          handled</div>
                                                          </div>
                                                          </div>
                                                          </blockquote>
                                                          <div><br>
                                                          </div>
                                                          <div>Where is
                                                          that exactly?
                                                          I can check
                                                          why we are not
                                                          catching it?</div>
                                                          <div> </div>
                                                          <blockquote class="gmail_quote">
                                                          <div>
                                                          <div>
                                                          <div><br>
                                                          </div>
                                                          <div>C1</div>
                                                          <div>  I don’t
                                                          see changes in
                                                          c1_Runtime.cpp</div>
                                                          <div>  note:
                                                          you also want
                                                          to look for
                                                          the fast path</div>
                                                          </div>
                                                          </div>
                                                          </blockquote>
                                                          <div><br>
                                                          </div>
                                                          <div>I added
                                                          the calls to
                                                          c1_Runtime in
                                                          the latest
                                                          webrev, but
                                                          was still
                                                          going through
                                                          testing before
                                                          pushing it
                                                          out. I had
                                                          waited on this
                                                          one a bit.
                                                          Fast path
                                                          would be
                                                          handled by the
                                                          threshold
                                                          check no?</div>
                                                          <div><br>
                                                          </div>
                                                          <blockquote class="gmail_quote">
                                                          <div>
                                                          <div>
                                                          <div><br>
                                                          </div>
                                                          <div>C2:
                                                          changes in
                                                          opto/runtime.cpp
                                                          for slow path</div>
                                                          <div>   did
                                                          you also catch
                                                          the fast path?</div>
                                                          </div>
                                                          </div>
                                                          </blockquote>
                                                          <div><br>
                                                          </div>
                                                          <div>Fast path
                                                          gets handled
                                                          by the same
                                                          threshold
                                                          check, no?
                                                          Perhaps I've
                                                          missed something
                                                          (very likely)?</div>
                                                          <div> </div>
                                                          <blockquote class="gmail_quote">
                                                          <div>
                                                          <div>
                                                          <div><br>
                                                          </div>
                                                          <div>3.
                                                          Performance - </div>
                                                          <div>After you
                                                          get all the
                                                          collectors
                                                          added - you
                                                          need to rerun
                                                          the
                                                          performance
                                                          numbers. </div>
                                                          </div>
                                                          </div>
                                                          </blockquote>
                                                          <div><br>
                                                          </div>
                                                          <div>Agreed :)</div>
                                                          <div> </div>
                                                          <blockquote class="gmail_quote">
                                                          <div>
                                                          <div>
                                                          <div><br>
                                                          </div>
                                                          <div>thanks,</div>
                                                          <div>Karen</div>
                                                          </div>
                                                          <div>
                                                          <div><br>
                                                          <blockquote type="cite">
                                                          <div>On Apr 5,
                                                          2018, at 2:15
                                                          PM, JC Beyler
                                                          <<a href="mailto:jcbeyler@google.com" target="_blank">jcbeyler@google.com</a>>
                                                          wrote:</div>
                                                          <br class="m_-1011140630431552159m_9203410791999666704m_-2827008458729093471m_-1152325375000192003m_8368332985884548048gmail-m_2654971596882259531gmail-m_2792660692152568680m_2153986306415466254m_1207530620928143740Apple-interchange-newline">
                                                          <div>
                                                          <div dir="ltr">Thanks
                                                          Boris and
                                                          Derek for
                                                          testing it.
                                                          <div><br>
                                                          </div>
                                                          <div>Yes I was
                                                          trying to get
                                                          a new version
                                                          out that had
                                                          the tests
                                                          ported as well
                                                          but got
                                                          sidetracked
                                                          while trying
                                                          to add tests
                                                          and two new
                                                          features.</div>
                                                          <div><br>
                                                          </div>
                                                          <div>Here is
                                                          the
                                                          incremental
                                                          webrev:</div>
                                                          <div><br>
                                                          </div>
                                                          <div>Here is
                                                          the full
                                                          webrev:</div>
                                                          <div><a href="http://cr.openjdk.java.net/%7Ejcbeyler/8171119/heap_event.11/" target="_blank">http://cr.openjdk.java.net/~jcbeyler/8171119/heap_event.11/</a><br>
                                                          </div>
                                                          <div><br>
                                                          </div>
                                                          <div>Basically,
                                                          the new tests
                                                          assert this:</div>
                                                          <div>  - Only
                                                          one agent can
                                                          currently ask
                                                          for the
                                                          sampling, I'm
                                                          currently
                                                          seeing if I
                                                          can push to a
                                                          next webrev
                                                          the
                                                          multi-agent
                                                          support to
                                                          start doing a
                                                          code freeze on
                                                          this one</div>
                                                          <div>  - The
                                                          event is not
                                                          thread-enabled,
                                                          meaning like
                                                          the
                                                          VMObjectAllocationEvent,
                                                          it's an all or
                                                          nothing event;
                                                          same as the
                                                          multi-agent,
                                                          I'm going to
                                                          see if a
                                                          future webrev
                                                          to add the
                                                          support is a
                                                          better idea to
                                                          freeze this
                                                          webrev a bit</div>
                                                          <div><br>
                                                          </div>
                                                          <div>There was
                                                          another item
                                                          that I added
                                                          here and I'm
                                                          unsure this
                                                          webrev is
                                                          stable in
                                                          debug mode: I
                                                          added an
                                                          assertion
                                                          system to
                                                          ascertain that
                                                          all paths
                                                          leading to a
                                                          TLAB slow path
                                                          (and hence a
                                                          sampling
                                                          point) have a
                                                          sampling
                                                          collector
                                                          ready to post
                                                          the event if a
                                                          user wants it.
                                                          This might
                                                          break a few
                                                          thing in debug
                                                          mode as I'm
                                                          working
                                                          through the
                                                          kinks of that
                                                          as well.
                                                          However, in
                                                          release mode,
                                                          this new
                                                          webrev passes
                                                          all the tests
                                                          in
                                                          hotspot/jtreg/serviceability/jvmti/HeapMonitor.</div>
                                                          <div><br>
                                                          </div>
                                                          <div>Let me
                                                          know what you
                                                          think,</div>
                                                          <div>Jc</div>
                                                          </div>
                                                          <br>
                                                          <div class="gmail_quote">
                                                          <div dir="ltr">On
                                                          Thu, Apr 5,
                                                          2018 at 4:56
                                                          AM Boris
                                                          Ulasevich <<a href="mailto:boris.ulasevich@bell-sw.com" target="_blank">boris.ulasevich@bell-sw.com</a>>
                                                          wrote:<br>
                                                          </div>
                                                          <blockquote class="gmail_quote">Hi
                                                          JC,<br>
                                                          <br>
                                                             I have just
                                                          checked on
                                                          arm32: your
                                                          patch compiles
                                                          and runs ok.<br>
                                                          <br>
                                                             As I can
                                                          see, jtreg
                                                          agentlib name
"-agentlib:HeapMonitor" does not<br>
                                                          correspond to
                                                          actual library
                                                          name:
                                                          libHeapMonitorTest.c
                                                          -><br>
libHeapMonitorTest.so<br>
                                                          <br>
                                                          Boris<br>
                                                          <br>
                                                          On 04.04.2018
                                                          01:54, White,
                                                          Derek wrote:<br>
                                                          > Thanks
                                                          JC,<br>
                                                          ><br>
                                                          > New patch
                                                          applies
                                                          cleanly.
                                                          Compiles and
                                                          runs (simple
                                                          test programs)
                                                          on<br>
                                                          > aarch64.<br>
                                                          ><br>
                                                          >   * Derek<br>
                                                          ><br>
                                                          > *From:*
                                                          JC Beyler
                                                          [mailto:<a href="mailto:jcbeyler@google.com" target="_blank">jcbeyler@google.com</a>]<br>
                                                          > *Sent:*
                                                          Monday, April
                                                          02, 2018 1:17
                                                          PM<br>
                                                          > *To:*
                                                          White, Derek
                                                          <<a href="mailto:Derek.White@cavium.com" target="_blank">Derek.White@cavium.com</a>><br>
                                                          > *Cc:*
                                                          Erik Österlund
                                                          <<a href="mailto:erik.osterlund@oracle.com" target="_blank">erik.osterlund@oracle.com</a>>;<br>
                                                          > <a href="mailto:serviceability-dev@openjdk.java.net" target="_blank">serviceability-dev@openjdk.java.net</a>;
hotspot-compiler-dev<br>
                                                          > <<a href="mailto:hotspot-compiler-dev@openjdk.java.net" target="_blank">hotspot-compiler-dev@openjdk.java.net</a>><br>
                                                          >
                                                          *Subject:* Re:
                                                          JDK-8171119:
                                                          Low-Overhead
                                                          Heap Profiling<br>
                                                          ><br>
                                                          > Hi Derek,<br>
                                                          ><br>
                                                          > I know
                                                          there were a
                                                          few things
                                                          that went in
                                                          that provoked
                                                          a merge<br>
                                                          > conflict.
                                                          I worked on it
                                                          and got it up
                                                          to date. Sadly
                                                          my lack of<br>
                                                          > knowledge
                                                          makes it a
                                                          full rebase
                                                          instead of
                                                          keeping all
                                                          the history.<br>
                                                          > However,
                                                          with a newly
                                                          cloned jdk/hs
                                                          you should now
                                                          be able to
                                                          use:<br>
                                                          ><br>
                                                          > <a href="http://cr.openjdk.java.net/%7Ejcbeyler/8171119/heap_event.10/" rel="noreferrer" target="_blank">http://cr.openjdk.java.net/~jcbeyler/8171119/heap_event.10/</a><br>
                                                          ><br>
                                                          > The
                                                          change you are
                                                          referring to
                                                          was done with
                                                          the others so
                                                          perhaps you<br>
                                                          > were
                                                          unlucky and I
                                                          forgot it in a
                                                          webrev and
                                                          fixed it in
                                                          another? I<br>
                                                          > don't
                                                          know but it's
                                                          been there and
                                                          I checked, it
                                                          is here:<br>
                                                          ><br>
                                                          > <a href="http://cr.openjdk.java.net/%7Ejcbeyler/8171119/heap_event.10/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp.udiff.html" rel="noreferrer" target="_blank">http://cr.openjdk.java.net/~jcbeyler/8171119/heap_event.10/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp.udiff.html</a><br>
                                                          ><br>
                                                          > I double
                                                          checked that
                                                          tlab_end_offset
                                                          no longer
                                                          appears in any<br>
                                                          >
                                                          architecture
                                                          (as far as I
                                                          can tell :)).<br>
                                                          ><br>
                                                          > Thanks
                                                          for testing
                                                          and let me
                                                          know if you
                                                          run into any
                                                          other issues!<br>
                                                          ><br>
                                                          > Jc<br>
                                                          ><br>
                                                          > On Fri,
                                                          Mar 30, 2018
                                                          at 4:24 PM
                                                          White, Derek
                                                          <<a href="mailto:Derek.White@cavium.com" target="_blank">Derek.White@cavium.com</a><br>
                                                          >
                                                          <mailto:<a href="mailto:Derek.White@cavium.com" target="_blank">Derek.White@cavium.com</a>>>
                                                          wrote:<br>
                                                          ><br>
                                                          >     Hi
                                                          Jc,<br>
                                                          ><br>
                                                          >     I’ve
                                                          been having
                                                          trouble
                                                          getting your
                                                          patch to apply
                                                          correctly. I<br>
                                                          >     may
                                                          have based it
                                                          on the wrong
                                                          version.<br>
                                                          ><br>
                                                          >     In
                                                          any case, I
                                                          think there’s
                                                          a missing
                                                          update to<br>
                                                          >   
                                                           macroAssembler_aarch64.cpp,
                                                          in
                                                          MacroAssembler::tlab_allocate(),<br>
                                                          >     where
“JavaThread::tlab_end_offset()” should become<br>
                                                          >   
                                                           “JavaThread::tlab_current_end_offset()”.<br>
                                                          ><br>
                                                          >     This
                                                          should
                                                          correspond to
                                                          the other
                                                          port’s changes
                                                          in<br>
                                                          >   
                                                           templateTable_<cpu>.cpp
                                                          files.<br>
                                                          ><br>
                                                          >   
                                                           Thanks!<br>
                                                          >     -
                                                          Derek<br>
                                                          ><br>
                                                          >   
                                                           *From:*
                                                          hotspot-compiler-dev<br>
                                                          >   
                                                           [mailto:<a href="mailto:hotspot-compiler-dev-bounces@openjdk.java.net" target="_blank">hotspot-compiler-dev-bounces@openjdk.java.net</a><br>
                                                          >   
                                                           <mailto:<a href="mailto:hotspot-compiler-dev-bounces@openjdk.java.net" target="_blank">hotspot-compiler-dev-bounces@openjdk.java.net</a>>]
                                                          *On Behalf<br>
                                                          >     Of
                                                          *JC Beyler<br>
                                                          >   
                                                           *Sent:*
                                                          Wednesday,
                                                          March 28, 2018
                                                          11:43 AM<br>
                                                          >     *To:*
                                                          Erik Österlund
                                                          <<a href="mailto:erik.osterlund@oracle.com" target="_blank">erik.osterlund@oracle.com</a><br>
                                                          >   
                                                           <mailto:<a href="mailto:erik.osterlund@oracle.com" target="_blank">erik.osterlund@oracle.com</a>>><br>
                                                          >     *Cc:*
                                                          <a href="mailto:serviceability-dev@openjdk.java.net" target="_blank">serviceability-dev@openjdk.java.net</a><br>
                                                          >   
                                                           <mailto:<a href="mailto:serviceability-dev@openjdk.java.net" target="_blank">serviceability-dev@openjdk.java.net</a>>;
hotspot-compiler-dev<br>
                                                          >     <<a href="mailto:hotspot-compiler-dev@openjdk.java.net" target="_blank">hotspot-compiler-dev@openjdk.java.net</a><br>
                                                          >   
                                                           <mailto:<a href="mailto:hotspot-compiler-dev@openjdk.java.net" target="_blank"></a></blockquote></div></div></blockquote></div></div></div></blockquote></div></div></div></blockquote></div></div></div></div></blockquote></div></div></div></blockquote></div></div></div></div></div></div></blockquote></div></div></div></blockquote></div></blockquote></div></blockquote></div></blockquote></div>
</blockquote></div>