Re: GcInfo: longest pause duration?

[Jeremy Manson]

Yes, this information is useful.

We've done a couple of things to get at it, which folks might find
interesting.

1) We extended the hsperfdata to include a large number of stats for CMS.
Parsing hsperfdata is ridiculously easy, and hasn't changed in years, so we
just parse it directly and report the values to monitoring tools.  I'm too
lazy to retype what we export, so are the relevant CMS variables from our
code; what they do should be relatively obvious:

_concurrent_mode_failure_count =
PerfDataManager::create_counter(NULL_NS,
"concurrent-mode-failure-count",
 PerfData::U_Events, CHECK);
_concurrent_mode_failure_due_to_fragmentation_count =
PerfDataManager::create_counter(NULL_NS,
"concurrent-mode-failure-due-to-fragmentation-count",
  PerfData::U_Events, CHECK);
_concurrent_mode_failure_time =
PerfDataManager::create_counter(NULL_NS,
"concurrent-mode-failure-time",
PerfData::U_Ticks, CHECK);_cms_initial_mark_count =
PerfDataManager::create_counter(NULL_NS, "cms-initial-mark-count",
   PerfData::U_Events, CHECK);
_cms_initial_mark_time =
PerfDataManager::create_counter(NULL_NS, "cms-initial-mark-time",
  PerfData::U_Ticks, CHECK);
_cms_remark_count =PerfDataManager::create_counter(NULL_NS,
"cms-remark-count",
PerfData::U_Events, CHECK);_cms_remark_time =
PerfDataManager::create_counter(NULL_NS, "cms-remark-time",
PerfData::U_Ticks, CHECK);
_cms_full_gc_count =PerfDataManager::create_counter(NULL_NS,
"cms-full-gc-count",
PerfData::U_Events, CHECK);_cms_full_gc_time =
PerfDataManager::create_counter(NULL_NS, "cms-full-gc-time",
 PerfData::U_Ticks, CHECK);
_cms_compacting_full_gc_count =
PerfDataManager::create_counter(NULL_NS,
"cms-compacting-full-gc-count",
PerfData::U_Events, CHECK);_cms_compacting_full_gc_time =
PerfDataManager::create_counter(NULL_NS,
"cms-compacting-full-gc-time",
PerfData::U_Ticks, CHECK);_cms_noncompacting_full_gc_count =
 PerfDataManager::create_counter(NULL_NS,
"cms-noncompacting-full-gc-count",
   PerfData::U_Events, CHECK);_cms_noncompacting_full_gc_time =
PerfDataManager::create_counter(NULL_NS,
"cms-noncompacting-full-gc-time",
  PerfData::U_Ticks, CHECK);_cms_conc_mark_count =
PerfDataManager::create_counter(NULL_NS, "cms-conc-mark-count",
PerfData::U_Events, CHECK);
_cms_conc_mark_time =PerfDataManager::create_counter(NULL_NS,
"cms-conc-mark-time",
PerfData::U_Ticks, CHECK);_cms_conc_preclean_count =
PerfDataManager::create_counter(NULL_NS, "cms-conc-preclean-count",
PerfData::U_Events, CHECK);
_cms_conc_preclean_time =
PerfDataManager::create_counter(NULL_NS, "cms-conc-preclean-time",
   PerfData::U_Ticks, CHECK);
_cms_conc_abortable_preclean_count =
PerfDataManager::create_counter(NULL_NS,
"cms-conc-abortable-preclean-count",
 PerfData::U_Events, CHECK);_cms_conc_abortable_preclean_time
=PerfDataManager::create_counter(NULL_NS,
"cms-conc-abortable-preclean-time",
PerfData::U_Ticks, CHECK);_cms_conc_sweep_count =
PerfDataManager::create_counter(NULL_NS, "cms-conc-sweep-count",
 PerfData::U_Events, CHECK);
_cms_conc_sweep_time =PerfDataManager::create_counter(NULL_NS,
"cms-conc-sweep-time",
PerfData::U_Ticks, CHECK);_cms_conc_reset_count =
PerfDataManager::create_counter(NULL_NS, "cms-conc-reset-count",
 PerfData::U_Events, CHECK);
_cms_conc_reset_time =PerfDataManager::create_counter(NULL_NS,
"cms-conc-reset-time",
PerfData::U_Ticks, CHECK);_cms_collection_count =
PerfDataManager::create_counter(NULL_NS, "cms-collection-count",
 PerfData::U_Events, CHECK);
_cms_collection_time =PerfDataManager::create_counter(NULL_NS,
"cms-collection-time",
PerfData::U_Ticks, CHECK);_cms_ref_proc_count =
PerfDataManager::create_counter(NULL_NS, "cms-ref-proc-count",
   PerfData::U_Events, CHECK);
_cms_ref_proc_time =PerfDataManager::create_counter(NULL_NS,
"cms-ref-proc-time",
PerfData::U_Ticks, CHECK);_yg_alloc_bytes =
PerfDataManager::create_variable(NULL_NS,
"jvm-gc-cms-young-gen-alloc-bytes",
 PerfData::U_None, CHECK);_yg_promo_bytes =
PerfDataManager::create_variable(NULL_NS,
"jvm-gc-cms-young-gen-promo-bytes",
 PerfData::U_None, CHECK);_og_direct_alloc_bytes =
PerfDataManager::create_variable(NULL_NS,
"jvm-gc-cms-old-gen-directalloc-bytes",
 PerfData::U_None, CHECK);_og_alloc_bytes =
PerfDataManager::create_variable(NULL_NS,
"jvm-gc-cms-old-gen-alloc-bytes",
   PerfData::U_None, CHECK);_total_alloc_bytes =
PerfDataManager::create_variable(NULL_NS,
"jvm-gc-cms-total-alloc-bytes",
 PerfData::U_None, CHECK);



2) We also implemented an API built on JFR's handlin

Thread Native ID Access

[Jeremy Manson]

Hey folks,

I mentioned earlier in the thread about the ThreadInfo.from() bug that I
found this because I was looking at fixing JDK-8154176, which proposes
introducing native thread ids to Thread and ThreadInfo.

https://bugs.openjdk.java.net/browse/JDK-8154176

I have a prototype for it.  I have a couple of questions, though:

0) Does anyone object to this being done or my doing it?  I see that it
already has an owner.

1) Should the ID be a long?  The Hotspot thread dump prints it out as 0x%x,
which is an unsigned hexadecimal integer.  The type hiding behind it is
platform-dependent, though: a pid_t on Linux, an unsigned long on Windows,
a thread_t on Solaris.  I could make it a String instead...

Jeremy

Re: Thread Native ID Access

[Jeremy Manson]

There isn't a pressing need.  I wrote this patch almost 5 years ago.  We
can do what we've always done and keep the patch locally.  It's just more
work for us to keep forward porting it, and loss of potential benefit to
the community.

I can certainly understand not wanting this in the high-value real estate
in java.lang.Thread.  I am reasonably happy just to add it to ThreadInfo,
or, if we think even that's too much, just to
com.sun.management.ThreadMXBean.

Since Alan cc'd loom-dev, I'll reiterate what I said before: IMO, for the
purposes of this feature, it doesn't matter how fibers map to Java Threads,
or how Java Threads map to OS threads.  As long as a Java Thread is running
on some OS thread (and nothing that we do in the JVM will change that),
this proposed API can return whatever OS thread the Java Thread currently
maps to.

David asked the following question:

For things like  /proc//task/ is there a way to say  in a
> way that means "current thread"? That may be a partial solution - or even
> whole if you could then read back the actual id?
>

Linux 3.17 and greater has /proc/thread-self.  But I'm not sure that's a
solution - if I want to use this to generate a list of threads with Java
names and OS-level state (which often differs from JVM thread state), then
I'm not sure how I would do that.

Mandy says this:

> - other than VM threads and Java to native OS threads mapping, any other
>   items on your list for monitoring?
>
>
There is a bunch of stuff - the ongoing thread with JC Beyler about our
heap profiler is the current thing on the front burner.  The only thing
that is relevant to thread monitoring is the aforementioned list of VM
threads, but that's not a point change to ThreadInfo.

For some of the other stuff, we need to do more due diligence on our end or
wait and see what you are doing before pushing it.  For example, we have
programmatic access to JFR event based GC history, but we're waiting to see
how the open-sourcing goes on that before pushing.  We have TSan support,
which we've mentioned in various forums before.  We let people register
native callbacks on allocation sites, but that's likely to get folded into
the heap profiling work.  We let monitoring tools look at the code cache
directly to avoid JVMTI overhead, but that's pretty dangerous, so I'm not
sure that it's a great idea as-is.  We have a bunch of extra entries in
perfdata, but we should probably review them to make sure we still care
about all of them.

We also have some bug fixes and performance improvements, and we should
probably just send those in.

Jeremy

On Thu, Feb 22, 2018 at 7:24 AM, Alan Bateman 
wrote:

> On 21/02/2018 22:40, Jeremy Manson wrote:
>
> Hey folks,
>
> I mentioned earlier in the thread about the ThreadInfo.from() bug that I
> found this because I was looking at fixing JDK-8154176, which proposes
> introducing native thread ids to Thread and ThreadInfo.
>
> https://bugs.openjdk.java.net/browse/JDK-8154176
>
> I have a prototype for it.  I have a couple of questions, though:
>
> 0) Does anyone object to this being done or my doing it?  I see that it
> already has an owner.
>
> 1) Should the ID be a long?  The Hotspot thread dump prints it out as
> 0x%x, which is an unsigned hexadecimal integer.  The type hiding behind it
> is platform-dependent, though: a pid_t on Linux, an unsigned long on
> Windows, a thread_t on Solaris.  I could make it a String instead...
>
> Mandy mentioned fibers in one of the replies and I think we do need to be
> cautious about exposing native thread IDs in the Java SE APIs until we have
> a better sense how some of these APIs will work. I expect there will be a
> Thread object per fiber but I'm less sure on ThreadMXBean. It may be that
> ThreadMXBean's dumpAllThreads returns the ThreadInfos for just the kernels
> threads, the equivalent of today where it only returns the ThreadInfos for
> the started threads, but once the project is further along then maybe a
> different choice might emerge.
>
> If there is pressing need then could we just expose it it in the
> JDK-specific com.sun.management.ThreadMXBean API instead?
>
> -Alan
>

Re: Thread Native ID Access

[Jeremy Manson]

I think we're all pretty much agreed that fibers imply native tids can
change.  I think that, as long as we document that, we're fine.

An API where there is probably a worse implementation problem is
ThreadMXBean#getThreadCpuTime
.
Right now, the implementation assumes a 1:1 mapping between OS thread and
Java thread.  Removing that assumption implies you have to do something
like track cpu time every time the scheduler switches you out, which is
going to make your switches expensive.

Jeremy

On Thu, Feb 22, 2018 at 9:16 PM, David Holmes 
wrote:

> On 23/02/2018 3:04 PM, John Rose wrote:
>
>> On Feb 22, 2018, at 8:49 PM, David Holmes > > wrote:
>>
>>>
>>> I don't think this request has any impact on Fibers at all. At any given
>>> time a piece of executing Java code is executing on a current Thread, and
>>> that current Thread must be running on a native thread (regardless of
>>> mapping) and the native thread has an id. The only use for exposing such an
>>> id is to allow you to correlate information obtained from inside the VM,
>>> with information observed outside, possibly by external tools.
>>>
>>
>> We may or may not bind fiber identity to the legacy java.lang.Thread type.
>>
>
> I'm not assuming that exposing native thread ids implies anything about
> the binding between those entities and anything higher-level. I have no
> issue with a Fiber/Continuation/Task/whatever running on different
> java.lang.Threads over its lifetime. For that matter I don't think it
> matters if java.lang.Threads could execute on different native threads
> (they don't but they could).
>
> David
> -
>
>
> This affects the question of whether the following code can return false:
>>
>> boolean testThreadShift(Runnable r) {
>>Thread t0 = Thread.current();
>>r.run();
>>Thread t1 = Thread.current();
>>return t0 == t1;
>> }
>>
>> This method can return false if (a) Thread.current() reports the identity
>> of the OS thread (not the fiber currently running), and (b) the runnable
>> includes a blocking operation which causes the current fiber to reschedule
>> on a different OS thread before it returns.
>>
>> Having this method return false is, perhaps, surprising enough to cause
>> us to inject fiber identity into java.lang.Thread, so that a
>> java.lang.Thread
>> identity is 1-1 with a fiber, rather than an OS "dinosaur" thread.  (You
>> can
>> only pick one!)  The alternative is to keep a 1-1 identity between OS
>> threads
>> and java.lang.Thread instances, making a new type which carries fiber
>> identity that multiplexes in a scheduler-dependent way over OS threads.
>>
>> It's an interesting trade-off, with many compatibility concerns.
>> I think we are leaning toward injecting fiber identity into Thread,
>> although this makes me shudder to think of the overheads we
>> will buy with this one move (ThreadLocal hash tables).
>>
>> So, now if you reify the OS thread identity, you could have the same
>> issue again.  Can this routine return false?
>>
>> boolean testNTIDShift(Runnable r) {
>>long t0 = NativeThreadID.current();
>>r.run();
>>long t1 = NativeThreadID.current();
>>return t0 == t1;
>> }
>>
>> I think, if you are digging down all the way to the native thread ID,
>> this routine *must* be allowed to return false under a Loom JVM.
>> There's no credible way to assign unique int64_t IDs to fiber objects
>> in the Java heap, as if they were OS objects.
>>
>> So my question is, do the proposed use cases for NTID allow
>> for testNTIDShift to return false?  If not, then it's pretty easy to say
>> that we don't want to supply NativeThreadID.current() for general
>> use, just to people implementing schedulers.
>>
>> — John
>>
>

Re: Thread Native ID Access

[Jeremy Manson]

Okay, I'll find time at some point to tinker with it.  I'm going to wait
until Mandy lands the fix to JDK-8198253, though.

Jeremy

On Fri, Feb 23, 2018 at 3:44 PM, Hohensee, Paul  wrote:

> I am. :)
>
>
>
> The process is to file an RFE and then a CSR associated with it. You file
> the CSR from the RFE page, not separately. See
>
>
>
> https://wiki.openjdk.java.net/display/csr/Main
>
> https://wiki.openjdk.java.net/display/csr/Fields+of+a+CSR+Request
>
> https://wiki.openjdk.java.net/display/csr/CSR+FAQs
>
>
>
> You need a review of the CSR by a domain expert before you can finalize it
> for committee review. If you go with com.sun.management versions of
> ThreadInfo and ThreadMXBean methods, there’s no change in the
> java.lang.management specification, which is important because the j.l.m
> interface must be supported by all implementations, which is a higher bar
> than for c.s.m changes.
>
>
>
> Paul
>
>
>
> *From: *Jeremy Manson 
> *Date: *Friday, February 23, 2018 at 3:12 PM
> *To: *"Hohensee, Paul" 
> *Cc: *David Holmes , "serviceability-dev@openjdk.
> java.net" , "
> loom-...@openjdk.java.net" , John Rose <
> john.r.r...@oracle.com>
>
> *Subject: *Re: Thread Native ID Access
>
>
>
> Thanks, Paul.  This is approximately what I was envisioning when Mandy and
> David said they didn't really want to expose it in java.lang.Thread.
>
>
>
> If folks are generally amenable to something like this, I can probably
> find the spare time to cobble it together.
>
>
>
> Jeremy
>
>
>
> On Fri, Feb 23, 2018 at 1:07 PM, Hohensee, Paul 
> wrote:
>
> Here’s a concrete proposal on which people may want to comment.
>
>
>
> We could add a method to ThreadInfo, vis.,
>
>
>
> long getPlatformThreadId()
>
>
>
> Returns the platform-specific thread ID of the thread associated with this
> ThreadInfo. The ID has no meaning to the Java runtime and is not guaranteed
> to have any meaning at all.
>
> I.e., it may be a constant value for all threads, or a native address, and
> may not be the same across multiple invocations.
>
>
>
> Returns:
>
>
>
> the platform-specific ID of the associated thread.
>
>
>
> For a fiber, a reasonable value would be the OS thread id for the OS
> thread that’s running the fiber at ThreadInfo construction time. If, say,
> the platform used a string or some other larger-than-long value, it could
> be a native address or some other value that mapped to one.
>
>
>
> If we don’t want to add a new method to ThreadInfo, we could define a
> com.sun.management.ThreadInfo to go with com.sun.management.ThreadMXBean.
>
>
>
> Parenthetically, to me a fiber is enough different from a thread to
> warrant a separate MXBean. I.e., a thread is a species of fiber, but not
> vica versa. So, we shouldn’t gate changes to ThreadMXBean on having a
> thorough understanding of how to support fibers in JMX.
>
>
>
> We could also add to com.sun.management.ThreadMXBean the methods
> getThreadId(long id), getThreadId(long [] ids), getPlatformThreadId(long
> id), and getPlatformThreadId(long [] ids). These presumably would be fast
> compared to creating ThreadInfo(s).
>
>
>
> We don’t want to add Thread.getPlatformId(), so the way to get the current
> thread’s platform id would be
>
>
>
> getThreadInfo( Thread.currentThread().getId() ).getPlatformThreadId()
>
>
>
> which is cumbersome and slow. Instead, by analogy to 
> ThreadMXBean.getCurrentThreadCpuTime(),
> we could add to com.sun.management.ThreadMXBean the methods
> getCurrentThreadId() and getCurrentPlatformThreadId().
>
>
>
> All this may seem somewhat overkill, but it’s no more so than how we
> currently create ThreadInfo objects and the various ThreadCpu/UserTime
> method definitions.
>
>
>
> Thanks,
>
>
>
> Paul
>
>
>
> *From: *serviceability-dev 
> on behalf of Jeremy Manson 
> *Date: *Thursday, February 22, 2018 at 9:42 PM
> *To: *David Holmes 
> *Cc: *"serviceability-dev@openjdk.java.net"  java.net>, "loom-...@openjdk.java.net" , John
> Rose 
> *Subject: *Re: Thread Native ID Access
>
>
>
> I think we're all pretty much agreed that fibers imply native tids can
> change.  I think that, as long as we document that, we're fine.
>
>
>
> An API where there is probably a worse implementation problem is
> ThreadMXBean#getThreadCpuTime
> <https://docs.oracle.com/javase/9/docs/api/java/lang/management/ThreadMXBean.html#getThreadCpuTime-long->
> .  Right now, the implementation assumes a 1:1 mapping betw

Re: RFR JDK-8198253: ThreadInfo.from(CompositeData) assigning fields incorrectly in JDK 9

[Jeremy Manson]

Yup, that's better.  I'd probably say "The same rule" instead of "Same
rule".

Jeremy

On Tue, Feb 27, 2018 at 10:55 AM, mandy chung 
wrote:

> Good point, Jeremy.  I notice some strange-ness when I wrote it but
> wasn't able to pin point the error.  Daniel also suggests to clarify
> MonitorInfo as well.
>
> Does this version look better?
>
>
>  * Returns a {@code ThreadInfo} object represented by the
>  * given {@code CompositeData}.
>  * 
>  * A {@code CompositeData} representing a {@code ThreadInfo} of
>  * version N must contain all of the attributes defined
>  * in version ≤ N unless specified otherwise.
>  * Same rule applies transitively to attributes whose type or
>  * component type is {@code CompositeType}.
>  * 
>  * A {@code CompositeData} representing {@code ThreadInfo} of version
>  * N contains {@code "stackTrace"} attribute representing
>  * an array of {@code StackTraceElement} of version N.
>  * The {@code "lockedMonitors"} attribute represents
>  * an array of {@link MonitorInfo} of version N
>  * which implies that its {@code "lockedStackFrame"} attribute also
>  * represents {@code StackTraceElement} of the same version, N.
>  * Otherwise, this method will throw {@code IllegalArgumentException}.
>
>
> Mandy
>
> On 2/27/18 9:56 AM, Jeremy Manson wrote:
>
> Comment on new doc wording:
>
>
> * A {@code CompositeData} representing a {@code ThreadInfo} of
> * version N must contain all the attributes defined
> * since N or earlier unless specified otherwise.
>
> Wouldn't "all of the attributes defined since N or earlier" just mean "all
> of the attributes"?  "Since" is basically the same as "after".  Would "must
> contain all of the attributes for every version up to and including N" work?
>
> Jeremy
>
>
>

Re: RFR 8205113: Update JVMTI doc references to object allocation tracking

[Jeremy Manson]

We haven't changed when a VM issues "VM object allocation" events.

There were references in the docs to a requirement to use bytecode
rewriting and JNI interception to track allocations.  With
SampledObjectAlloc, this is no longer the case - SampledObjectAlloc can
track them.  This change is supposed to remove the references to those
requirements, and provide suitable replacement text.

VM object alloc has specific language about being able to use it to track
allocations that cannot be tracked with bytecode instrumentation and JNI
interception.  My goal in rephrasing was to make it clear that, while you
can still use it for this, you can also just use SampledObjectAlloc for
everything.

Jeremy

On Sun, Jun 17, 2018 at 9:11 PM David Holmes 
wrote:

> Hi Jeremy,
>
> On 16/06/2018 2:33 AM, Jeremy Manson wrote:
> > Hi all,
> >
> > There are a number of references in the JVMTI doc to its not doing
> > object allocation tracking.  Now that JEP 331 has landed, these
> > references are obsolete.  This patch changes those references
> accordingly.
> >
> > While I was there, I took the liberty of fixing some spelling errors.
> >
> > As far as I know, these are non-normative changes and modify no API, so
> > they should not require a CSR.
>
> I'm unclear on the nature of the change to "VM Object Allocation". Does
> the existence of SampledObjectAlloc change when a VM should issue "VM
> object allocation" events?
>
> Thanks,
> David
>
> >
> > Bug:
> > https://bugs.openjdk.java.net/browse/JDK-8205113
> >
> > Webrev:
> > http://cr.openjdk.java.net/~jmanson/8205113/webrev.00/
> >
> > Thanks!
> >
> > Jeremy
>

Re: RFR 8205113: Update JVMTI doc references to object allocation tracking

[Jeremy Manson]

Yup!  The paragraph meanders a bit.  How about something like:

Sent when the virtual machine allocates an
Object visible to Java programming language code without using a
new bytecode variant or a JNI method.
Many approaches to tracking object allocation use a combination of
bytecode-based instrumentation and JNI
function
interception to intercept allocations.  However, this
approach can leave a number of allocations undetected.  Allocations that
are neither
triggered by bytecode nor JNI are executed directly by the VM.
When those allocations occur, the VM should send this event.
Virtual machines that are incapable of bytecode instrumentation
for some or all of their methods may also send this event.

Note that the SampledObjectAlloc
event is triggered on all Java object allocations, including those
triggered by bytecode,
JNI methods, and VM events.



On Mon, Jun 18, 2018 at 12:57 AM David Holmes 
wrote:

> On 18/06/2018 5:01 PM, Jeremy Manson wrote:
> > We haven't changed when a VM issues "VM object allocation" events.
> >
> > There were references in the docs to a requirement to use bytecode
> > rewriting and JNI interception to track allocations.  With
> > SampledObjectAlloc, this is no longer the case - SampledObjectAlloc can
> > track them.  This change is supposed to remove the references to those
> > requirements, and provide suitable replacement text.
> >
> > VM object alloc has specific language about being able to use it to
> > track allocations that cannot be tracked with bytecode instrumentation
> > and JNI interception.  My goal in rephrasing was to make it clear that,
> > while you can still use it for this, you can also just use
> > SampledObjectAlloc for everything.
>
> Okay. That doesn't come across clearly to me - sorry. So you will now
> get both kinds of events for allocations done in the VM?
>
> Thanks,
> David
>
>
> > Jeremy
> >
> > On Sun, Jun 17, 2018 at 9:11 PM David Holmes  > <mailto:david.hol...@oracle.com>> wrote:
> >
> > Hi Jeremy,
> >
> > On 16/06/2018 2:33 AM, Jeremy Manson wrote:
> >  > Hi all,
> >  >
> >  > There are a number of references in the JVMTI doc to its not doing
> >  > object allocation tracking.  Now that JEP 331 has landed, these
> >  > references are obsolete.  This patch changes those references
> > accordingly.
> >  >
> >  > While I was there, I took the liberty of fixing some
> spelling errors.
> >  >
> >  > As far as I know, these are non-normative changes and modify no
> > API, so
> >  > they should not require a CSR.
> >
> > I'm unclear on the nature of the change to "VM Object Allocation".
> Does
> > the existence of SampledObjectAlloc change when a VM should issue "VM
> > object allocation" events?
> >
> > Thanks,
> > David
> >
> >  >
> >  > Bug:
> >  > https://bugs.openjdk.java.net/browse/JDK-8205113
> >  >
> >  > Webrev:
> >  > http://cr.openjdk.java.net/~jmanson/8205113/webrev.00/
> >  >
> >  > Thanks!
> >  >
> >  > Jeremy
> >
>

Re: RFR 8205113: Update JVMTI doc references to object allocation tracking

[Jeremy Manson]

That would be okay with me, assuming that my other corrections are made.
I'd also like to fix the spelling of instrumentation in the first sentence.

Jeremy

On Mon, Jun 18, 2018 at 11:01 PM serguei.spit...@oracle.com <
serguei.spit...@oracle.com> wrote:

> Hi Jeremy and David,
>
> Sorry for being late to the party.
>
> I'm also concerned about the Jeremy's spec update is more intrusive than
> necessary.
> One specifics of the new SampledObjectAlloc event is that it is posted
> conditionally.
> So, it is not fully comparable with the VMObjectAlloc event and can not
> replace it in any way.
> I'm even not yet convinced that any spec update is necessary.
>
> However, something like below would look minimal and reasonable:
>
> == Sent when a method causes the virtual machine to allocate an
> == Object visible to Java programming language code and the allocation
> += is not detectable by other *unconditional* intrumentation mechanisms.
>
> == Generally object allocation should be detected by instrumenting
> == the bytecodes of allocating methods.
>
> == Object allocation generated in native code by JNI function
> == calls should be detected using
> == JNI function
> interception.
>
> == Some methods might not have associated bytecodes and are not
> == native methods, they instead are executed directly by the
> == VM. These methods should send this event.
>
> == Virtual machines which are incapable of bytecode instrumentation
> == for some or all of their methods can send this event.
>
> *++ Note that the  id="SampledObjectAlloc">SampledObjectAlloc*
> *++ event is conditionally triggered on all Java object allocations,
> including those*
> *++ caused by bytecode method execution, JNI method execution, and
> directly by VM methods.*
>
>
> Maybe, just adding the last statement would be good enough.
>
> Thanks,
> Serguei
>
>
> On 6/18/18 21:36, David Holmes wrote:
>
> On 19/06/2018 4:50 AM, Jeremy Manson wrote:
>
> Yup!  The paragraph meanders a bit.  How about something like:
>
>
> I'm not sure some of the change quite works. The original text considers
> there to be three kinds of methods that can cause allocation when executed:
> - Java (bytecode) methods
> - JNI methods
> - VM methods
>
> but you've turned this into three kinds of allocation: via bytecode, via
> JNI, and via the VM. You then refer to "triggering" an allocation when we
> tend to use triggering for events. You also refer to an allocation being
> "executed directly by the VM" (a phrase previously applied when the subject
> was a 'method') - but you don't really execute allocations.
>
> IIUC the problem with the existing text is just that it considers VM
> allocation events as being undetectable other than by this "VM object
> allocation event" - but that's no longer true. So how about something
> minimally changed like this:
>
> ---
>   Sent when a method causes the virtual machine to directly allocate an
>   Object visible to Java programming language code.
>   Generally object allocation can be detected by instrumenting
>   the bytecodes of allocating methods.
>   Object allocation generated in native code by JNI function
>   calls can be detected using
>   JNI function
> interception.
>Some methods might not have associated bytecodes and are not
>native methods, they instead are executed directly by the
>VM. These methods should send this event.
>Virtual machines which are incapable of bytecode instrumentation
>for some or all of their methods can send this event.
>
>Note that the id="SampledObjectAlloc">SampledObjectAlloc
>event is triggered on all Java object allocations, including those
>caused by bytecode method execution, JNI method execution, and
>directly by VM methods.
> ---
>
> Thanks,
> David
>
> Sent when the virtual machine allocates an
> Object visible to Java programming language code without using a
> new bytecode variant or a JNI method.
> Many approaches to tracking object allocation use a combination of
> bytecode-based instrumentation and JNI
> function
> interception to intercept allocations.  However, this
> approach can leave a number of allocations undetected.  Allocations that
> are neither
> triggered by bytecode nor JNI are executed directly by the VM.
> When those allocations occur, the VM should send this event.
> Virtual machines that are incapable of bytecode instrumentation
> for some or all of their methods may also send this event.
> 
> Note that the  id="SampledObjectAlloc">SampledObjectAlloc
> event is triggered on all

gdb and OpenJDK

[Jeremy Manson]

Hey folks,

I think I've mentioned to some of the people on this list that we (and by
we, I mean Sasha Smundak) have been working on interoperability between gdb
and Java.  Basically, what we have now allows us to get a backtrace in gdb
that includes Java frames with parameter values (no local variables yet).

Needless to say, this has been terribly, terribly helpful in our JVM and
JNI debugging.

Support for this feature is basically two parts:

First, gdb needed to be extended to support the ability to plug in a frame
unwinder.  The process of submitting this to gdb (which has been ongoing
for quite a while) is finally starting to make reasonable progress.

Next, we need a plugin for OpenJDK.  We have a plugin written that calls
out to the serviceability agent.  (It's written in Python, per gdb's
requirements).

Pending the gdb support being finalized, we'd love to contribute the plugin
to OpenJDK.What should we do make that happen?  Write a JEP and submit
it to this list?  Technically, neither of us are committers (which you need
to be to submit a JEP), but I imagine we can work around that.

Here's the gdb patch, for anyone interested:

https://sourceware.org/ml/gdb-patches/2014-12/msg00408.html

Thanks!

Jeremy

Re: gdb and OpenJDK

[Jeremy Manson]

Hey folks,

I just want to make sure we are on the same page here:

1) Andrew is talking about using the gdbjit interface and generated DWARF
information.  I believe this wouldn't work for interpreted code, because
there is no JIT information emitted for it.  It would also require extra
work to make it work for core dumps, because you would need some sort of
post-mortem way of getting the DWARF information.

2) RedHat has a lot more sway over gdb than we do, especially this part of
gdb.  If you folks *don't* want our patch in gdb, please speak up soon!  If
you folks are going to go your own way with this, it will make our attempts
to get this into gdb irrelevant.  Also, if you *do* want our patch in gdb,
please speak up soon!  It would be great to hear from you, since we've been
desperately trying to get the gdb community to accept this patch for the
better part of a year, and we're having to beg, plead and cajole code
reviews out of people.

3) If the community does like our approach, we still have no idea what we
need to do to get it accepted in OpenJDK.  As Sasha says, we believe the
licenses are compatible, but we will still have to come to some agreement
about what direction to go.

Thanks!

Jeremy

On Thu, Feb 19, 2015 at 11:47 AM, Alexander Smundak 
wrote:

> On Mon, Feb 16, 2015 at 2:52 AM, Andrew Haley  wrote:
> > It would be, long term.  I've been discussing this with Red Hat's GDB
> > group and I'm hoping to come up with a proposal and hopefully some
> > working code.
> I have the enabling patch to GDB being reviewed at a glacial pace (see
> https://sourceware.org/ml/gdb-patches/2014-12/msg00408.html). Perhaps
> Red Hat's GDB group can respond there?
>

Re: RFR: 6588467: Add isDaemon() and getPriority() to ThreadInfo

[Jeremy Manson]

Thanks, Mandy.

I guess it is time to submit.  I don't have a committer bit.  Any
volunteers?

Thanks to all for the review!

Jeremy

On Mon, Feb 23, 2015 at 5:29 PM, Mandy Chung  wrote:

> On 2/23/15 11:49 AM, Jeremy Manson wrote:
>
>> Okey-doke:
>>
>> http://cr.openjdk.java.net/~jmanson/6588467/webrev.03/ <
>> http://cr.openjdk.java.net/%7Ejmanson/6588467/webrev.03/>
>>
>>
> Looks good.  Minor comment:  @see Thread#isDaemon can be removed since you
> have it in @linkplain.  No need for a new webrev.
>
> Thanks.
> Mandy
>

Low-Overhead Heap Profiling

[Jeremy Manson]

Hey folks,

(cc'ing Aleksey and John, because I mentioned this to them at the JVMLS
last year, but I never followed up.)

We have a patch at Google I've always wanted to contribute to OpenJDK, but
I always figured it would be unwanted.  I've recently been thinking that
might not be as true, though.  I thought I would ask if there is any
interest / if I should write a JEP / if I should just forget it.

The basic problem is that there is no low-overhead supported way to figure
out where allocation hotspots are.  That is, sets of stack traces where
lots of allocation / large allocations took place.

What I had originally done (this was in ~2007) was use bytecode rewriting
to instrument allocation sites.  The instrumentation would call a Java
method, and the method would capture a stack trace.  To reduce overhead,
there was a per-thread counter that only took a stack trace once every N
bytes allocated, where N is a randomly chosen point in a probability
distribution that centered around ~512K.

This was *way* too slow, and it didn't pick up allocations through JNI, so
I instrumented allocations at the VM level, and the overhead went away.
The sampling is always turned on in our internal VMs, and a user can just
query an interface for a list of sampled stack traces.  The allocated stack
traces are held with weak refs, so you only get live samples.

The additional overhead for allocations amounts to a subtraction, and an
occasional stack trace, which is usually a very, very small amount of our
CPU (although I had to do some jiggering in JDK8 to fix some performance
regressions).

There really isn't another good way to do this with low overhead.  I was
wondering how the gruop would feel about our contributing it?

Thoughts?

Jeremy

Re: Low-Overhead Heap Profiling

[Jeremy Manson]

While I'm glad to hear that our colleagues at RedHat would love it, it will
still need a sponsor from Oracle.  Any volunteers?

It will also need a little polish to be available to the world.  Open
design questions for upstreaming are things like:

- Should the interval between samples be configurable?

- Should it *just* take a stack trace, or should the behavior be
configurable?  For example, we have a variant that allows it to invoke a
callback on allocation.  Do you want this?  Because it is being called
during allocation, the callback can't invoke JNI (because of the potential
for a safepoint), so it might be somewhat confusing to the user.

- If the answer to the above is yes, should it be able to invoke *multiple*
callbacks with multiple intervals?  That could get very expensive and hairy.

- Other than stack trace, what kind of information should the sampled data
contain?  Right now, the structure is:

struct StackTraceData {
  ASGCT_CallTrace *trace;
  jint byte_size;
  jlong thread_id;
  const jbyte *name;
  jint name_length;
  jlong uid;
};

(where name is the class name, and uid is just a unique identifier.)  For
general consumption, we'll probably have to change the ASGCT_CallTrace to a
jvmtiStackInfo, I suppose.

Jeremy

Re: Low-Overhead Heap Profiling

[Jeremy Manson]

Mario and Kirk -

Random followup to the comments:

Mario: Internally, we have both a callback mechanism (which is off by
default) and a sampling mechanism, which is always on.  We can certainly
contribute both.  The callback mechanism has a strict rule that you don't
call JNI in it.  The one thing you can do is register to throw an OOME
after you return from the allocation - some folks wanted to use this to
rate limit how much allocation a given request could do.

Kirk: We have had a bug open for a long time for an extension that says how
many GC cycles a given sample survived.  That might be useful, and would
actually be pretty easy to implement.

I don't want to do to much work on this (or, more likely, tell one of the
folks on my team to do it) until we have a way forward to get this
interface into some JDK distribution at some point.

Jeremy

On Mon, Jun 22, 2015 at 11:46 PM, Mario Torre <
neugens.limasoftw...@gmail.com> wrote:

> 2015-06-23 6:58 GMT+02:00 Jeremy Manson :
> > While I'm glad to hear that our colleagues at RedHat would love it, it
> will
> > still need a sponsor from Oracle.  Any volunteers?
> >
> > It will also need a little polish to be available to the world.  Open
> design
> > questions for upstreaming are things like:
> >
> > - Should the interval between samples be configurable?
>
> Yeah, this would be nice.
>
> > - Should it *just* take a stack trace, or should the behavior be
> > configurable?  For example, we have a variant that allows it to invoke a
> > callback on allocation.  Do you want this?  Because it is being called
> > during allocation, the callback can't invoke JNI (because of the
> potential
> > for a safe point) , so it might be somewhat confusing to the user.
>
> Yes, a perhaps optional callbacks would be nice too. I don't think the
> "no JNI here" is a problem given the scope of the tool, although I can
> see how it could be a bit confusing or kind of limiting in some cases,
> but probably the benefit outweighs this issue. I can see, for example,
> how a callback here could provide a plug for things like system tap
> probes etc...
>
> > - If the answer to the above is yes, should it be able to invoke
> *multiple*
> > callbacks with multiple intervals?  That could get very expensive and
> hairy.
>
> I don't think you need multiple callbacks, since the callback would be
> user code, users can call additional callbacks themselves, but it also
> depends on how this user code is installed. For instance, if the
> outside world interface is some kind of JVMTi interface, then we
> probably need to support multiple callbacks (as in multiple agents
> installing their custom code). I suspect that to keep the overhead low
> an higher level interface is needed to collect the allocation events,
> while the callback interface mechanisms would be most useful to
> implement/extend this interface, but not really meant to be used by
> default.
>
> > - Other than stack trace, what kind of information should the sampled
> data
> > contain?  Right now, the structure is:
> >
> > struct StackTraceData {
> >   ASGCT_CallTrace *trace;
> >   jint byte_size;
> >   jlong thread_id;
> >   const jbyte *name;
> >   jint name_length;
> >   jlong uid;
> > };
> >
> > (where name is the class name, and uid is just a unique identifier.)  For
> > general consumption, we'll probably have to change the ASGCT_CallTrace
> to a
> > jvmtiStackInfo, I suppose.
>
> It looks to me that most of the interesting stuff is here. The
> questions that remain to me are how the outside interface would look
> and how it would work, we can probably take a more abstracted and high
> level look at it and then refine the specific step by step.
>
> Cheers,
> Mario
>
> --
> pgp key: http://subkeys.pgp.net/ PGP Key ID: 80F240CF
> Fingerprint: BA39 9666 94EC 8B73 27FA  FC7C 4086 63E3 80F2 40CF
>
> Java Champion - Blog: http://neugens.wordpress.com - Twitter: @neugens
> Proud GNU Classpath developer: http://www.classpath.org/
> OpenJDK: http://openjdk.java.net/projects/caciocavallo/
>
> Please, support open standards:
> http://endsoftpatents.org/
>

Re: Low-Overhead Heap Profiling

[Jeremy Manson]

I haven't tried!  Another problem with our submitting things is that we
can't really test on anything other than Linux.

The reason we use ASGCT is that our modified version of ASGCT gathers
native frames *and* Java frames, getting a mixed mode stack trace that
crosses JNI boundaries.  I haven't checked on the details, but we could
probably use a more standard stack trace gathering mechanism for general
consumption.

As I said, I think we'd have to change that to jvmtiStackInfo.  Would folks
like to see some proposed JVMTI interfaces for this?

Jeremy

On Tue, Jun 23, 2015 at 6:14 AM, Daniel D. Daugherty <
daniel.daughe...@oracle.com> wrote:

> > ASGCT_CallTrace *trace;
>
> So it looks like this uses the AsyncGetTrace() infrastructure.
> Does your tool work on Windows and MacOS X?
>
> Dan
>
>
>
> On 6/22/15 10:58 PM, Jeremy Manson wrote:
>
>> While I'm glad to hear that our colleagues at RedHat would love it, it
>> will still need a sponsor from Oracle.  Any volunteers?
>>
>> It will also need a little polish to be available to the world.  Open
>> design questions for upstreaming are things like:
>>
>> - Should the interval between samples be configurable?
>>
>> - Should it *just* take a stack trace, or should the behavior be
>> configurable?  For example, we have a variant that allows it to invoke a
>> callback on allocation. Do you want this?  Because it is being called
>> during allocation, the callback can't invoke JNI (because of the potential
>> for a safepoint), so it might be somewhat confusing to the user.
>>
>> - If the answer to the above is yes, should it be able to invoke
>> *multiple* callbacks with multiple intervals?  That could get very
>> expensive and hairy.
>>
>> - Other than stack trace, what kind of information should the sampled
>> data contain?  Right now, the structure is:
>>
>> struct StackTraceData {
>>   ASGCT_CallTrace *trace;
>>   jint byte_size;
>>   jlong thread_id;
>>   const jbyte *name;
>>   jint name_length;
>>   jlong uid;
>> };
>>
>> (where name is the class name, and uid is just a unique identifier.)  For
>> general consumption, we'll probably have to change the ASGCT_CallTrace to a
>> jvmtiStackInfo, I suppose.
>>
>> Jeremy
>>
>
>

Re: Low-Overhead Heap Profiling

[Jeremy Manson]

If it is a blocker for this work, we can find a way to test on OS X and
Windows.

Jeremy

On Tue, Jun 23, 2015 at 9:14 AM, Jeremy Manson 
wrote:

> I haven't tried!  Another problem with our submitting things is that we
> can't really test on anything other than Linux.
>
> The reason we use ASGCT is that our modified version of ASGCT gathers
> native frames *and* Java frames, getting a mixed mode stack trace that
> crosses JNI boundaries.  I haven't checked on the details, but we could
> probably use a more standard stack trace gathering mechanism for general
> consumption.
>
> As I said, I think we'd have to change that to jvmtiStackInfo.  Would
> folks like to see some proposed JVMTI interfaces for this?
>
> Jeremy
>
> On Tue, Jun 23, 2015 at 6:14 AM, Daniel D. Daugherty <
> daniel.daughe...@oracle.com> wrote:
>
>> > ASGCT_CallTrace *trace;
>>
>> So it looks like this uses the AsyncGetTrace() infrastructure.
>> Does your tool work on Windows and MacOS X?
>>
>> Dan
>>
>>
>>
>> On 6/22/15 10:58 PM, Jeremy Manson wrote:
>>
>>> While I'm glad to hear that our colleagues at RedHat would love it, it
>>> will still need a sponsor from Oracle.  Any volunteers?
>>>
>>> It will also need a little polish to be available to the world.  Open
>>> design questions for upstreaming are things like:
>>>
>>> - Should the interval between samples be configurable?
>>>
>>> - Should it *just* take a stack trace, or should the behavior be
>>> configurable?  For example, we have a variant that allows it to invoke a
>>> callback on allocation. Do you want this?  Because it is being called
>>> during allocation, the callback can't invoke JNI (because of the potential
>>> for a safepoint), so it might be somewhat confusing to the user.
>>>
>>> - If the answer to the above is yes, should it be able to invoke
>>> *multiple* callbacks with multiple intervals?  That could get very
>>> expensive and hairy.
>>>
>>> - Other than stack trace, what kind of information should the sampled
>>> data contain?  Right now, the structure is:
>>>
>>> struct StackTraceData {
>>>   ASGCT_CallTrace *trace;
>>>   jint byte_size;
>>>   jlong thread_id;
>>>   const jbyte *name;
>>>   jint name_length;
>>>   jlong uid;
>>> };
>>>
>>> (where name is the class name, and uid is just a unique identifier.)
>>> For general consumption, we'll probably have to change the ASGCT_CallTrace
>>> to a jvmtiStackInfo, I suppose.
>>>
>>> Jeremy
>>>
>>
>>
>

Re: Low-Overhead Heap Profiling

[Jeremy Manson]

I don't want the size of the TLAB, which is ergonomically adjusted, to be
tied to the sampling rate.  There is no reason to do that.  I want
reasonable statistical sampling of the allocations.

All this requires is a separate counter that is set to the next sampling
interval, and decremented when an allocation happens, which goes into a
slow path when the decrement hits 0.  Doing a subtraction and a pointer
bump in allocation instead of just a pointer bump is basically free.  Note
that it has been doing an additional addition (to keep track of per thread
allocation) as part of allocation since Java 7, and no one has complained.

I'm not worried about the ease of implementation here, because we've
already implemented it.  It hasn't even been hard for us to do the forward
port, except when the relevant Hotspot code is significantly refactored.

We can also turn the sampling off, if we want.  We can set the sampling
rate to 2^32, have the sampling code do nothing, and no one will ever
notice.  In fact, we could just have the sampling code do nothing, and no
one would ever notice.

Honestly, no one ever notices the overhead of the sampling, anyway.  JDK8
made it more expensive to grab a stack trace (the cost became proportional
to the number of loaded classes), but we have a patch that mitigates that,
which we would also be happy to upstream.

As for the other concern: my concern about *just* having the callback
mechanism is that there is quite a lot you can't do from user code during
an allocation, because of lack of access to JNI.  However, you can do
pretty much anything from the VM itself.  Crucially (for us), we don't just
log the stack traces, we also keep track of which are live and which
aren't.  We can't do this in a callback, if the callback can't create weak
refs to the object.

What we do at Google is to have two methods: one that you pass a callback
to (the callback gets invoked with a StackTraceData object, as I've defined
above), and another that just tells you which sampled objects are still
live.  We could also add a third, which allowed a callback to set the
sampling interval (basically, the VM would call it to get the integer
number of bytes to be allocated before the next sample).

Would people be amenable to that?  It makes the code more complex, but, as
I say, it's nice for detecting memory leaks ("Hey!  Where did that 1 GB
object come from?").

Jeremy


On Tue, Jun 23, 2015 at 1:06 PM, Tony Printezis 
wrote:

> Jeremy (and all),
>
> I’m not on the serviceability list so I won’t include the messages so far.
> :-) Also CCing the hotspot GC list, in case they have some feedback on this.
>
> Could I suggest a (much) simpler but at least as powerful and flexible way
> to do this? (This is something we’ve been meaning to do for a while now for
> TwitterJDK, the JDK we develop and deploy here at Twitter.) You can force
> allocations to go into the slow path periodically by artificially setting
> the TLAB top to a lower value. So, imagine a TLAB is 4M. You can set top to
> (bottom+1M). When an allocation thinks the TLAB is full (in this case, the
> first 1MB is full) it will call the allocation slow path. There, you can
> intercept it, sample the allocation (and, like in your case, you’ll also
> have the correct stack trace), notice that the TLAB is not actually full,
> extend its to top to, say, (bottom+2M), and you’re done.
>
> Advantages of this approach:
>
> * This is a much smaller, simpler, and self-contained change (no compiler
> changes necessary to maintain...).
>
> * When it’s off, the overhead is only one extra test at the slow path TLAB
> allocation (i.e., negligible; we do some sampling on TLABs in TwitterJDK
> using a similar mechanism and, when it’s off, I’ve observed no performance
> overhead).
>
> * (most importantly) You can turn this on and off, and adjust the sampling
> rate, dynamically. If you do the sampling based on JITed code, you’ll have
> to recompile all methods with allocation sites to turn the sampling on or
> off. (You can of course have it always on and just discard the output; it’d
> be nice not to have to do that though. IMHO, at least.)
>
> * You can also very cheaply turn this on and off (or adjust the sampling
> frequncy) per thread, if that’s be helpful in some way (just add the
> appropriate info on the thread’s TLAB).
>
> A few extra comments on the previous discussion:
>
> * "JFR samples per new TLAB allocation. It provides really very good
> picture and I haven't seen overhead more than 2” : When TLABs get very
> large, I don’t think sampling one object per TLAB is enough to get a good
> sample (IMHO, at least). It’s probably OK for something like jbb which
> mostly allocates instances of a handful of classes and has very few
> allocation sites. But, a lot of the code we run at Twitter is a lot more
> elaborate than that and, in our experience, sampling one object per TLAB is
> not enough. You can, of course, decrease the TLAB size to increase the
> samp

Re: Low-Overhead Heap Profiling

[Jeremy Manson]

On Wed, Jun 24, 2015 at 10:57 AM, Tony Printezis 
wrote:

> Hi Jeremy,
>
> Please see inline.
>
> On June 23, 2015 at 7:22:13 PM, Jeremy Manson (jeremyman...@google.com)
> wrote:
>
> I don't want the size of the TLAB, which is ergonomically adjusted, to be
> tied to the sampling rate.  There is no reason to do that.  I want
> reasonable statistical sampling of the allocations.
>
>
> As I said explicitly in my e-mail, I totally agree with this. Which is why
> I never suggested to resize TLABs in order to vary the sampling rate.
> (Apologies if my e-mail was not clear.)
>

My fault - I misread it.  Doesn't your proposal miss out of TLAB allocs
entirely (and, in fact, not work if TLAB support is turned off)?  I might
be missing something obvious (and see my response below).


> All this requires is a separate counter that is set to the next sampling
> interval, and decremented when an allocation happens, which goes into a
> slow path when the decrement hits 0.  Doing a subtraction and a pointer
> bump in allocation instead of just a pointer bump is basically free.
>
>
> Maybe on intel is cheap, but maybe it’s not on other platforms that other
> folks care about.
>
Really?  A memory read and a subtraction?  Which architectures care about
that?

Again, notice that no one has complained about the addition that was added
for total bytes allocated per thread.  I note that was actually added in
the 6u20 timeframe.

Note that it has been doing an additional addition (to keep track of per
> thread allocation) as part of allocation since Java 7,
>
>
> Interesting. I hadn’t realized that. Does that keep track of total size
> allocated per thread or number of allocated objects per thread? If it’s the
> former, why isn’t it possible to calculate that from the TLABs information?
>

Total size allocated per thread.  It isn't possible to calculate that from
the TLAB because of out-of-TLAB allocation (and hypothetically disabled
TLABs).

For some reason, they never included it in the ThreadMXBean interface, but
it is in com.sun.management.ThreadMXBean, so you can cast your ThreadMXBean
to a com.sun.management.ThreadMXBean and call getThreadAllocatedBytes() on
it.


> and no one has complained.
>
> I'm not worried about the ease of implementation here, because we've
> already implemented it.
>
>
> Yeah, but someone will have to maintain it moving forward.
>

I've been maintaining it internally to Google for 5 years.  It's actually
pretty self-contained.  The only work involved is when they refactor
something (so I've had to move it), or when a bug in the existing
implementation is discovered.  It is very closely parallel to the TLAB
code, which doesn't change much / at all.


> It hasn't even been hard for us to do the forward port, except when the
> relevant Hotspot code is significantly refactored.
>
> We can also turn the sampling off, if we want.  We can set the sampling
> rate to 2^32, have the sampling code do nothing, and no one will ever
> notice.
>
>
> You still have extra instructions in the allocation path, so it’s not
> turned off (i.e., you have the tax without any benefit).
>
>
Hey, you have a counter in your allocation path you've never noticed, which
none of your code uses.  Pipelining is a wonderful thing.  :)

In fact, we could just have the sampling code do nothing, and no one would
> ever notice.
>
> Honestly, no one ever notices the overhead of the sampling, anyway.  JDK8
> made it more expensive to grab a stack trace (the cost became proportional
> to the number of loaded classes), but we have a patch that mitigates that,
> which we would also be happy to upstream.
>
> As for the other concern: my concern about *just* having the callback
> mechanism is that there is quite a lot you can't do from user code during
> an allocation, because of lack of access to JNI.
>
>
> Maybe I missed something. Are the callbacks in Java? I.e., do you call
> them using JNI from the slow path you call directly from the allocation
> code?
>
> (For context: this referred to the hypothetical feature where we can
provide a callback that invokes some code from allocation.)

(It's not actually hypothetical, because we've already implemented it, but
let's call it hypothetical for the moment.)

We invoke native code.  You can't invoke any Java code during allocation,
including calling JNI methods, because that would make allocation
potentially reentrant, which doesn't work for all sorts of reasons.  The
native code doesn't even get passed a JNIEnv * - there is nothing it can do
with it without making the VM crash a lot.

Or, rather, you might be able to do that, but it would take a lot of
Hotspot rearchitecting.  When I tried to do it, I r

Re: Low-Overhead Heap Profiling

[Jeremy Manson]

On Thu, Jun 25, 2015 at 1:28 PM, Tony Printezis 
wrote:

> Hi Jeremy,
>
> Inline.
>
> On June 24, 2015 at 7:26:55 PM, Jeremy Manson (jeremyman...@google.com)
> wrote:
>
>
>
> On Wed, Jun 24, 2015 at 10:57 AM, Tony Printezis 
> wrote:
>
>> Hi Jeremy,
>>
>> Please see inline.
>>
>> On June 23, 2015 at 7:22:13 PM, Jeremy Manson (jeremyman...@google.com)
>> wrote:
>>
>> I don't want the size of the TLAB, which is ergonomically adjusted, to be
>> tied to the sampling rate.  There is no reason to do that.  I want
>> reasonable statistical sampling of the allocations.
>>
>>
>> As I said explicitly in my e-mail, I totally agree with this. Which is
>> why I never suggested to resize TLABs in order to vary the sampling rate.
>> (Apologies if my e-mail was not clear.)
>>
>
> My fault - I misread it.  Doesn't your proposal miss out of TLAB allocs
> entirely
>
>
> This is correct: We’ll also have to intercept the outside-TLAB allocs.
> But, IMHO, this is a feature as it’s helpful to know how many (and which)
> allocations happen outside TLABs. These are generally very infrequent (and
> slow anyway), so sampling all of those, instead of only sampling some of
> them, does not have much of an overhead. But, you could also do sampling
> for the outside-TLAB allocs too, if you want: just accumulate their size on
> a separate per-thread counter and sample the one that bumps that counter
> goes over a limit.
>
>
The outside-TLAB allocations generally get caught anyway, because they tend
to be large enough to jump over the sample size immediately.


> An additional observation (orthogonal to the main point, but I thought I’d
> mention it anyway): For the outside-TLAB allocs it’d be helpful to also
> know which generation the object ended up in (e.g., young gen or
> direct-to-old-gen). This is very helpful in some situations when you’re
> trying to work out which allocation(s) grew the old gen occupancy between
> two young GCs.
>

True.  We don't have this implemented, but it would be reasonably
straightforward to glean it from the oop.


> FWIW, the existing JFR events follow the approach I described above:
>
> * one event for each new TLAB + first alloc in that TLAB (my proposal
> basically generalizes this and removes the 1-1 relationship between object
> alloc sampling and new TLAB operation)
>
> * one event for all allocs outside a TLAB
>
> I think the above separation is helpful. But if you think it could confuse
> users, you can of course easily just combine the information (but I
> strongly believe it’s better to report the information separately).
>

I do think it would make a confusing API.  It might make more sense to have
a reporting mechanism that had a set number of fields with very concrete
information (size, class, stacktrace), but allowed for platform-specific
metadata.  We end up with a very long list of things we want in the sample:
generation (how do you describe a generation?), object age (by number of
GCs survived?  What kind of GC?), was it a TLAB allocation, etc.


(and, in fact, not work if TLAB support is turned off)?
>
>
> Who turns off TLABs? Is -UseTLAB even tested by Oracle? (This is a genuine
> question.)
>

I don't think they do.  I have turned them off for various reasons
(usually, I'm trying to instrument allocations and I don't want to muck
about with thinking about TLABs), and the code paths seem a little crufty.
ISTR at some point finding something that clearly only worked by mistake,
but I can't remember now what it was.

[snip]



>   However, you can do pretty much anything from the VM itself.  Crucially
>> (for us), we don't just log the stack traces, we also keep track of which
>> are live and which aren't.  We can't do this in a callback, if the callback
>> can't create weak refs to the object.
>>
>> What we do at Google is to have two methods: one that you pass a callback
>> to (the callback gets invoked with a StackTraceData object, as I've defined
>> above), and another that just tells you which sampled objects are still
>> live.  We could also add a third, which allowed a callback to set the
>> sampling interval (basically, the VM would call it to get the integer
>> number of bytes to be allocated before the next sample).
>>
>> Would people be amenable to that?  It makes the code more complex, but,
>> as I say, it's nice for detecting memory leaks ("Hey!  Where did that 1 GB
>> object come from?").
>>
>>
>> Well, that 1GB object would have most likely been allocated outside a
>> TLAB and you could have identified it by instrumenting the “outside-of-TL

Re: Low-Overhead Heap Profiling

[Jeremy Manson]

Karen,

I understand your concerns.  For reference, this is the additional code in
the x86 assembler.  There are very small stubs in C1 and the template
interpreter to call out to this macro (forgive the gratuitous use of the
string "google" - we sprinkle it around a bit because it makes it a little
easier to distinguish our code from upstream code).

#define GOOGLE_HEAP_MONITORING(ma, thread, var_size_in_bytes,
con_size_in_bytes, object, t1, t2, sample_invocation) \
do {\
  { \
SkipIfEqual skip_if(ma, &GoogleHeapMonitor, 0); \
Label skip_sample;  \
Register thr = thread;  \
if (!thr->is_valid()) { \
  NOT_LP64(assert(t1 != noreg,  \
  "Need temporary register for constants"));\
  thr = NOT_LP64(t1) LP64_ONLY(r15_thread); \
  NOT_LP64(ma -> get_thread(thr);)  \
}   \
/* Trigger heap monitoring event */ \
Address bus(thr,\
JavaThread::google_bytes_until_sample_offset());\
\
if (var_size_in_bytes->is_valid()) {\
  ma -> NOT_LP64(subl) LP64_ONLY(subq)(bus, var_size_in_bytes); \
} else {\
  int csib = (con_size_in_bytes);   \
  assert(t2 != noreg,   \
 "Need temporary register for constants");  \
  ma -> NOT_LP64(movl) LP64_ONLY(mov64)(t2, csib);  \
  ma -> NOT_LP64(subl) LP64_ONLY(subq)(bus, t2);\
}   \
\
ma -> jcc(Assembler::positive, skip_sample);\
\
{   \
  sample_invocation \
}   \
ma -> bind(skip_sample);\
  } \
} while(0)

It's not all that hard to port to additional architectures, but we'll have
to think about it.

Jeremy

On Thu, Jun 25, 2015 at 1:41 PM, Karen Kinnear 
wrote:

> Jeremy,
>
> Did I follow this correctly - that your approach modifies the compilers
> and interpreters and Tony's modifies the
> common allocation code?
>
> Given that the number of compilers and interpreters and interpreter
> platforms keeps expanding - I'd like to
> add a vote to have heap allocation profiling in common allocation code.
>
> thanks,
> Karen
>
> On Jun 25, 2015, at 4:28 PM, Tony Printezis wrote:
>
> Hi Jeremy,
>
> Inline.
>
> On June 24, 2015 at 7:26:55 PM, Jeremy Manson (jeremyman...@google.com)
> wrote:
>
>
>
> On Wed, Jun 24, 2015 at 10:57 AM, Tony Printezis 
> wrote:
>
>> Hi Jeremy,
>>
>> Please see inline.
>>
>> On June 23, 2015 at 7:22:13 PM, Jeremy Manson (jeremyman...@google.com)
>> wrote:
>>
>> I don't want the size of the TLAB, which is ergonomically adjusted, to be
>> tied to the sampling rate.  There is no reason to do that.  I want
>> reasonable statistical sampling of the allocations.
>>
>>
>> As I said explicitly in my e-mail, I totally agree with this. Which is
>> why I never suggested to resize TLABs in order to vary the sampling rate.
>> (Apologies if my e-mail was not clear.)
>>
>
> My fault - I misread it.  Doesn't your proposal miss out of TLAB allocs
> entirely
>
>
> This is correct: We’ll also have to intercept the outside-TLAB allocs.
> But, IMHO, this is a feature as it’s helpful to know how many (and which)
> allocations happen outside TLABs. These are generally very infrequent (and
> slow anyway), so sampling all of those, instead of only sampling some of
> them, does not have much of an overhead. But, you could also do sampling
> for the outside-TLAB allocs too, if you want: just accumulate their size on
> a separate per-thread counter and sample the one that bumps that counter
> goes over a limit.
>
> An additional observation

Re: Low-Overhead Heap Profiling

[Jeremy Manson]

On Thu, Jun 25, 2015 at 1:55 PM, Tony Printezis 
wrote:

> Bernd,
>
> I like the idea of buffering up the sampled objects in, some data
> structure. But I assume it’d have to be a per-thread data structure to
> avoid conention issues. So, we’ll also need a periodic task that collects
> all such data structures and makes them available (somehow) to whoever
> wants to consume them?
>

This is easily done.  But, per my last message, I don't think it should be
the default.  It can just be available as another callback, if you want it.

Jeremy

Re: Low-Overhead Heap Profiling

[Jeremy Manson]

Another thought.  Since:

- It would be kind of surprising for Thread->allocated_bytes() to be
different from the number used as the interval for tracking (e.g., if your
interval is, say, 512K, you check allocated bytes, it says 0, you allocate
512K, you check allocated bytes, it says 512K, but no sample was taken),
AND
- We're already taking the maintenance hit to maintain the allocated bytes
counter everywhere,

Maybe a good compromise would be to piggyback on the allocated bytes
counter?  If the allocated bytes counter is at N, and we pick a next
sampling interval of K, we set a per-thread variable to N+K, and everywhere
we increment the allocated bytes counter, we just test to see if it is
greater than N+K?

This would add an additional load and another easily predicted branch, but
no additional subtraction.  Also, it would have very obvious and tractable
modifications to make in existing places that already have logic for the
counter, so there wouldn't be much of an additional maintenance burden.
Finally, it would more-or-less address my concerns, because the non-TLAB
fast paths I'm worried about are already instrumented for it.

Jeremy

On Thu, Jun 25, 2015 at 10:27 PM, Jeremy Manson 
wrote:

>
>
> On Thu, Jun 25, 2015 at 1:28 PM, Tony Printezis 
> wrote:
>
>> Hi Jeremy,
>>
>> Inline.
>>
>> On June 24, 2015 at 7:26:55 PM, Jeremy Manson (jeremyman...@google.com)
>> wrote:
>>
>>
>>
>> On Wed, Jun 24, 2015 at 10:57 AM, Tony Printezis 
>>  wrote:
>>
>>> Hi Jeremy,
>>>
>>> Please see inline.
>>>
>>> On June 23, 2015 at 7:22:13 PM, Jeremy Manson (jeremyman...@google.com)
>>> wrote:
>>>
>>> I don't want the size of the TLAB, which is ergonomically adjusted, to
>>> be tied to the sampling rate.  There is no reason to do that.  I want
>>> reasonable statistical sampling of the allocations.
>>>
>>>
>>> As I said explicitly in my e-mail, I totally agree with this. Which is
>>> why I never suggested to resize TLABs in order to vary the sampling rate.
>>> (Apologies if my e-mail was not clear.)
>>>
>>
>> My fault - I misread it.  Doesn't your proposal miss out of TLAB allocs
>> entirely
>>
>>
>> This is correct: We’ll also have to intercept the outside-TLAB allocs.
>> But, IMHO, this is a feature as it’s helpful to know how many (and which)
>> allocations happen outside TLABs. These are generally very infrequent (and
>> slow anyway), so sampling all of those, instead of only sampling some of
>> them, does not have much of an overhead. But, you could also do sampling
>> for the outside-TLAB allocs too, if you want: just accumulate their size on
>> a separate per-thread counter and sample the one that bumps that counter
>> goes over a limit.
>>
>>
> The outside-TLAB allocations generally get caught anyway, because they
> tend to be large enough to jump over the sample size immediately.
>
>
>> An additional observation (orthogonal to the main point, but I thought
>> I’d mention it anyway): For the outside-TLAB allocs it’d be helpful to also
>> know which generation the object ended up in (e.g., young gen or
>> direct-to-old-gen). This is very helpful in some situations when you’re
>> trying to work out which allocation(s) grew the old gen occupancy between
>> two young GCs.
>>
>
> True.  We don't have this implemented, but it would be reasonably
> straightforward to glean it from the oop.
>
>
>> FWIW, the existing JFR events follow the approach I described above:
>>
>> * one event for each new TLAB + first alloc in that TLAB (my proposal
>> basically generalizes this and removes the 1-1 relationship between object
>> alloc sampling and new TLAB operation)
>>
>> * one event for all allocs outside a TLAB
>>
>> I think the above separation is helpful. But if you think it could
>> confuse users, you can of course easily just combine the information (but I
>> strongly believe it’s better to report the information separately).
>>
>
> I do think it would make a confusing API.  It might make more sense to
> have a reporting mechanism that had a set number of fields with very
> concrete information (size, class, stacktrace), but allowed for
> platform-specific metadata.  We end up with a very long list of things we
> want in the sample: generation (how do you describe a generation?), object
> age (by number of GCs survived?  What kind of GC?), was it a TLAB
> allocation, etc.
>
>
> (and, in fact, not work if TLAB support is turned off)?
>>
>>
>> Who turns off TLABs? Is -UseTLAB even te

Re: Low-Overhead Heap Profiling

[Jeremy Manson]

>
>
>
> On Thu, Jun 25, 2015 at 2:23 PM, Tony Printezis 
>  wrote:
>
>> BTW, Could we get a reaction from the Oracle folks on this?
>>
>
I contacted Staffan (who is probably the right person) offline, and he is,
apparently, on vacation until the beginning of August.

Unless there is someone else at Oracle who might be the right person, we
might have to hold off on a final decision until then.  In the meantime, we
can probably continue the conversation - I might draft a JEP.

Jeremy

os::current_thread_id on Linux

[Jeremy Manson]

Hey folks,

os::current_thread_id on Linux now maps to pthread_self.  The problem with
pthread_self is that it only makes sense in the context of the running
process.  When it is written out to the log (as it is in several places),
there really isn't a way (AFAICT) for the user to map it back to anything
useful.

As it happens, that value is mostly used to write to the log.  The places
where it doesn't do so don't seem to need to use pthread_self for any
particular reason.

Meanwhile, the SIGQUIT stack dump uses java_thread->osthread()->thread_id()
as the nid.  On Linux, that maps back to os::Linux::gettid(), whish is also
what gets exposed in /proc.  That makes it much easier to see what threads
might be doing the log write.

Would it be okay to change os::current_thread_id to point
to os::Linux::gettid()?  That way, it can be mapped back to the output of a
SIGQUIT dump.

The downside of gettid() is that it is only available on Linux>2.4.11, but
that dates from 2001.  If we really still want to support it, we could
check for that.

Thoughts?

Jeremy

Re: os::current_thread_id on Linux

[Jeremy Manson]

Example of where this would help:

Current output of SIGQUIT:

"C1 CompilerThread3" #8 daemon prio=9 os_prio=0 tid=0x7f0890247800
nid=0x2e3d waiting on condition [0x]
   java.lang.Thread.State: RUNNABLE

"C2 CompilerThread2" #7 daemon prio=9 os_prio=0 tid=0x7f0890245000
nid=0x2e3c waiting on condition [0x]
   java.lang.Thread.State: RUNNABLE

"C2 CompilerThread1" #6 daemon prio=9 os_prio=0 tid=0x7f0890243800
nid=0x2e3b waiting on condition [0x]
   java.lang.Thread.State: RUNNABLE

"C2 CompilerThread0" #5 daemon prio=9 os_prio=0 tid=0x7f0890240800
nid=0x2e3a waiting on condition [0x]
   java.lang.Thread.State: RUNNABLE

Current directory listing from /tmp with  -XX:+UnlockDiagnosticVMOptions
-XX:+LogCompilation:

$ ls -d /tmp/hs*
hs_c139673756251904_pid11818.log  hs_c139673759409920_pid11818.log
hs_c139673757304576_pid11818.log  hsperfdata_jeremymanson/
hs_c139673758357248_pid11818.log  hsperfdata_root/


Output with this change would look like:

$ ls -d /tmp/hs*
hs_c11837_pid11818.log  hs_c11834_pid11818.log
hs_c11836_pid11818.log  hsperfdata_jeremymanson/
hs_c11835_pid11818.log  hsperfdata_root/

Where 1183x maps neatly back to 0x2e3x in the SIGQUIT dump.

Jeremy

On Wed, Jul 22, 2015 at 11:22 AM, Jeremy Manson 
wrote:

> Hey folks,
>
> os::current_thread_id on Linux now maps to pthread_self.  The problem with
> pthread_self is that it only makes sense in the context of the running
> process.  When it is written out to the log (as it is in several places),
> there really isn't a way (AFAICT) for the user to map it back to anything
> useful.
>
> As it happens, that value is mostly used to write to the log.  The places
> where it doesn't do so don't seem to need to use pthread_self for any
> particular reason.
>
> Meanwhile, the SIGQUIT stack dump
> uses java_thread->osthread()->thread_id() as the nid.  On Linux, that maps
> back to os::Linux::gettid(), whish is also what gets exposed in /proc.
> That makes it much easier to see what threads might be doing the log write.
>
> Would it be okay to change os::current_thread_id to point
> to os::Linux::gettid()?  That way, it can be mapped back to the output of a
> SIGQUIT dump.
>
> The downside of gettid() is that it is only available on Linux>2.4.11, but
> that dates from 2001.  If we really still want to support it, we could
> check for that.
>
> Thoughts?
>
> Jeremy
>

Re: os::current_thread_id on Linux

[Jeremy Manson]

Based on the feedback, this seems to be a good idea, approximately.  Coleen
would have sponsored, but she's going on vacation.  Anyone else feel like
sponsoring?

Jeremy

On Wed, Jul 22, 2015 at 11:22 AM, Jeremy Manson 
wrote:

> Hey folks,
>
> os::current_thread_id on Linux now maps to pthread_self.  The problem with
> pthread_self is that it only makes sense in the context of the running
> process.  When it is written out to the log (as it is in several places),
> there really isn't a way (AFAICT) for the user to map it back to anything
> useful.
>
> As it happens, that value is mostly used to write to the log.  The places
> where it doesn't do so don't seem to need to use pthread_self for any
> particular reason.
>
> Meanwhile, the SIGQUIT stack dump
> uses java_thread->osthread()->thread_id() as the nid.  On Linux, that maps
> back to os::Linux::gettid(), whish is also what gets exposed in /proc.
> That makes it much easier to see what threads might be doing the log write.
>
> Would it be okay to change os::current_thread_id to point
> to os::Linux::gettid()?  That way, it can be mapped back to the output of a
> SIGQUIT dump.
>
> The downside of gettid() is that it is only available on Linux>2.4.11, but
> that dates from 2001.  If we really still want to support it, we could
> check for that.
>
> Thoughts?
>
> Jeremy
>

Re: os::current_thread_id on Linux

[Jeremy Manson]

Hey David,

Thanks for the offer of sponsorship.  My goal here is really to make the
log output on Linux usable.  I want to be able to map the log output back
to an actual thread.  I don't think it really matters to users if the API
consistently means "kernel thread ID" or "threading API thread ID", as long
as they can figure out what the output means.

Since what I am doing (in effect) to accomplish my goal is to ensure that
the API returns the same value as osthread()->thread_id() does for the
current thread,  I could just... do precisely that.
os::current_thread_id could
just return osthread()->thread_id() for the current thread.  I don't have
easy access to anything for testing other than Linux, though, so whether it
worked (or even compiled) on the other platforms would be a bit of a guess
(per the last time we did this dance).

Seem reasonable?

Jeremy

On Wed, Jul 22, 2015 at 7:08 PM, David Holmes 
wrote:

> On 23/07/2015 8:01 AM, Jeremy Manson wrote:
>
>> Based on the feedback, this seems to be a good idea, approximately.
>> Coleen would have sponsored, but she's going on vacation.  Anyone else
>> feel like sponsoring?
>>
>
> Hold up a minute! :) There are different notions of "native thread id"
> that exist. First we have the "user level thread id" - this is what is
> reported by pthread_self in POSIX and thr_self in UI. Then we also have the
> OS specific "thread" id, also referred to as a LWP or "kernel scheduling
> entity" or "kernel thread" - the id for this is what gettid() maps back to
> on Linux. This distinction may not exist on all platforms.
>
> Unfortunately os::current_thread_id does not define which of these it
> represents:
>
>  // thread id on Linux/64bit is 64bit, on Windows and Solaris, it's 32bit
>   static intx current_thread_id();
>
> and so on some platforms it returns the "user thread id" (eg
> pthread_self()), and on some it returns the same as gettid (ie OSX - but I
> don't know if the mach thread id is truly a "LWP" id ?).
>
> Also note that on some platforms the osThread stores the id of the
> "user-level thread" and on some the "kernel thread". Hence another source
> of confusion. :(
>
> So if you want to enforce that os::current_thread_id() represents the
> "kernel thread" then that should be applied consistently across all
> platforms**, and for platforms for which there is a change to make you have
> to ensure the usage of os::current_thread_id() is not semantically altered
> by the change.
>
> ** Of course a platform may only have a single notion of "thread"
>
> I'm happy to sponsor such a proposal. And don't worry about maintaining
> compatibility with archaic Linux versions for JDK9 (less cleanup to do
> later).
>
> Thanks,
> David
>
>  Jeremy
>>
>> On Wed, Jul 22, 2015 at 11:22 AM, Jeremy Manson > <mailto:jeremyman...@google.com>> wrote:
>>
>> Hey folks,
>>
>> os::current_thread_id on Linux now maps to pthread_self.  The
>> problem with pthread_self is that it only makes sense in the context
>> of the running process.  When it is written out to the log (as it is
>> in several places), there really isn't a way (AFAICT) for the user
>> to map it back to anything useful.
>>
>> As it happens, that value is mostly used to write to the log.  The
>> places where it doesn't do so don't seem to need to use pthread_self
>> for any particular reason.
>>
>> Meanwhile, the SIGQUIT stack dump
>> uses java_thread->osthread()->thread_id() as the nid.  On Linux,
>> that maps back to os::Linux::gettid(), whish is also what gets
>> exposed in /proc.  That makes it much easier to see what threads
>> might be doing the log write.
>>
>> Would it be okay to change os::current_thread_id to point
>> to os::Linux::gettid()?  That way, it can be mapped back to the
>> output of a SIGQUIT dump.
>>
>> The downside of gettid() is that it is only available on
>> Linux>2.4.11, but that dates from 2001.  If we really still want to
>> support it, we could check for that.
>>
>> Thoughts?
>>
>> Jeremy
>>
>>
>>

RFR (XS): 6661889: thread id on Linux is inconsistent in error and log outputs

[Jeremy Manson]

David, can you review/sponsor this, per our conversation of last week?

It fixes an issue where, on Linux, some of the Hotspot logging output
identifies threads by pthread_id, and some of it identifies threads by
kernel-level TID.  This happened because Hotspot calls into two different
APIs that provide the thread ID, and developers seem to have picked which
one they call at random.

The CR makes these APIs consistently use the kernel-level TID, and makes
one of the APIs a thin shim over the other.

Conversation:
http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2015-July/015507.html
Bug: https://bugs.openjdk.java.net/browse/JDK-6661889
Webrev: http://cr.openjdk.java.net/~jmanson/6661889/webrev.01/

Jeremy

Re: RFR (XS): 6661889: thread id on Linux is inconsistent in error and log outputs

[Jeremy Manson]

Thanks, folks!

Jeremy

On Wed, Jul 29, 2015 at 8:03 PM, David Holmes 
wrote:

> On 30/07/2015 9:07 AM, Vladimir Kozlov wrote:
>
>> Finally after 7 years, since I filed this bug, it will be fixed :)
>>
>
> I hope it now does what you wanted :)
>
> On its way.
>
> Thanks,
> David
>
>
>  Looks good.
>>
>> Thanks,
>> Vladimir
>>
>> On 7/29/15 3:31 PM, David Holmes wrote:
>>
>>> Hi Jeremy,
>>>
>>> On 30/07/2015 6:34 AM, Jeremy Manson wrote:
>>>
>>>> David, can you review/sponsor this, per our conversation of last week?
>>>>
>>>
>>> Yes I can sponsor and it is also Reviewed.
>>>
>>> But I'd like a second review from someone else.
>>>
>>> Thanks,
>>> David
>>>
>>>  It fixes an issue where, on Linux, some of the Hotspot logging output
>>>> identifies threads by pthread_id, and some of it identifies threads by
>>>> kernel-level TID.  This happened because Hotspot calls into two
>>>> different APIs that provide the thread ID, and developers seem to have
>>>> picked which one they call at random.
>>>>
>>>> The CR makes these APIs consistently use the kernel-level TID, and makes
>>>> one of the APIs a thin shim over the other.
>>>>
>>>> Conversation:
>>>>
>>>> http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2015-July/015507.html
>>>>
>>>>
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-6661889
>>>> Webrev: http://cr.openjdk.java.net/~jmanson/6661889/webrev.01/
>>>>
>>>> Jeremy
>>>>
>>>

Re: Low-Overhead Heap Profiling

[Jeremy Manson]

Working on it.  I hope to have something to share in the next couple of
weeks.

Jeremy

On Wed, Aug 12, 2015 at 4:28 AM, Kees Jan Koster  wrote:

> Guys,
>
> This piqued my interest. Where can I find the draft JEP and the sources
> for this, please?
>
> Kees Jan
>
>
> > On 4 Aug 2015, at 23:22, Jeremy Manson  wrote:
> >
> > Thanks, Staffan.  I've been tinkering with a draft JEP, but it has been
> going slowly because of other craziness in my life.  Some responses:
> >
> > 1) It was a fair bit of work to do it, mostly because it was the first
> time I had tinkered with c2.  This was 5 years ago.  Most of the work since
> then has been dealing with merge conflicts.
> >
> > 2) I did envision a JVMTI interface.  More in the JEP.
> >
> > 3) We have some internal tests, but obviously, we'd have to figure out
> how to externalize them.  For native code, it's much easier only to have to
> worry about building and running on Linux.  Presumably, there's already
> code in there for JVMTI.
> >
> > I'll try to get a JEP going for real, although it might not happen in
> the next week or so, because I'm moving next week (+the JVMLS).
> >
> > Jeremy
> >
> > On Tue, Aug 4, 2015 at 4:04 AM, Staffan Larsen <
> staffan.lar...@oracle.com> wrote:
> > Hi,
> >
> > Sorry for being away for so long.
> >
> > If memory serves me right then the current AllocObjectInNewTLAB JFR
> event was written as a way to quickly get some allocation profiling
> information with a minimum of VM changes. It probably also carried over to
> Hotspot from JRockit. I agree that we can and should collect better
> information than what that event does.
> >
> > I like the approach of instrumenting the interpreter and compiler to do
> the sampling. I had thought it would be a lot of work to do it and was
> reluctant to go down that path. If the work is already done and Jeremy has
> maintained it for a few years without great problems, I think we should
> look at bringing it in. I am not worried about the overhead of a decrement
> and a compare in the allocation path, but of course we should benchmark
> that.
> >
> > It wasn’t clear to me from the discussion how (or if) this was being
> exposed to end users. Was the proposal to just have the native entry points
> in the VM and have these be used by various agents? Would this be part of
> JVMTI?
> >
> > I think a draft JEP would be the logical next step and make it easier
> for us all to discuss what exactly the proposal should contain. Also, let’s
> not forget the need for tests for the feature.
> >
> > Thanks,
> > /Staffan
> >
> >
> >
>
>
> --
> Kees Jan
>
> http://java-monitor.com/
> kjkos...@kjkoster.org
> +31651838192
>
> The secret of success lies in the stability of the goal. -- Benjamin
> Disraeli
>
>

Re: RFR 8062116: JVMTI GetClassMethods is Slow

[Jeremy Manson]

Thanks for taking a look, Coleen!

On Mon, Nov 3, 2014 at 12:19 PM, Coleen Phillimore <
coleen.phillim...@oracle.com> wrote:

>
> Hi Jeremy,
>
> I reviewed your new code and it looks fine.  I had one comment in
>
> http://cr.openjdk.java.net/~jmanson/8062116/webrev.00/src/
> share/vm/prims/jvmtiEnv.cpp.udiff.html
>
> The name "need_to_resolve" doesn't make sense when reading this code.
> Isn't it more like "need_to_ensure_space" ?  I think method resolution with
> the other name, which it doesn't do.
>

Hmmm... it is there to tell you that there are jmethodids for that class
that haven't been instantiated.  Is it all right if I change it to
"jmethodids_found" (and reverse the sense of it)?


> I was trying to find a way to make this new code not appear twice (maybe
> with a local jvmtiEnv function get_jmethodID(m) - instanceK_h is
> m->method_holder()).
>

You know, I initially did that, but this file is parsed with some weird XSL
setup that doesn't allow methods other than the ones that map directly to
the JVMTI calls.

Also, I was trying to figure out if the new class in utilities called
> chunkedList.hpp could be used to store jmethodIDs, since the data
> structures are similar.  There is still more things in JNIMethodBlock has
> to do so I think a specialized structure is still needed (which is why I
> originally wrote it to be very simple).  I'm not sure if the comment above
> it still applies.  Maybe only the first and third sentences.  Can you
> rewrite the comment slightly?
>

chunkedList wouldn't work as is, because it doesn't let you parameterize
the bucket size, but it could probably be made to work (in the same way I
made this one work).  It's also an oddly bare-bones class - I'm not sure
why it doesn't have contains and insert methods and so on.

I'm not in love with the idea of doing it, because a) it would complicate
my backport and b) I don't really have a lot of time to do hotspot
refactoring, but if you think it should happen, I can make it happen
(perhaps not in a timely way :) ).

As for the comment, I'll eliminate all but the first and third sentences.


> Your other comments in the changes are good.
>
> I can't completely answer your question about reusing free_methods - but
> if a jmethodID is created provisionally in InstanceKlass::get_jmethod_id
> and not needed because it loses the race in the method id cache, it's never
> handed back to native code, so it's safe to reuse.  This is different than
> jmethodIDs for methods that are unloaded.  They are cleared and never
> reused.  At least that's my reading of this caching code but it's pretty
> complicated stuff.
>

Ah, I see.  Thanks.


> I've also run our nsk and jck vm/jvmti on this change and they all
> passed.  I'd be happy to sponsor it with these suggested changes and it
> needs another reviewer.
>

I've cc'd Chuck Rasbold, who has already reviewed it internally and given
it the thumbs-up.  I'm sure he would be happy to do so publicly, too.

Thanks for diagnosing and fixing this problem!


Happy to do it!  And so are the programs that use my JVMTI!

Jeremy

Re: RFR 8062116: JVMTI GetClassMethods is Slow

[Jeremy Manson]

Wow, go take care of my toddler for a few hours, come back, and all the
questions are answered for me!  Thanks, Coleen.

To be fair, the original code was actually correct (instead of, you know,
implementation-dependent-correct), so I feel a little weird about the whole
thing.

Jeremy

On Wed, Nov 5, 2014 at 9:35 PM, David Holmes 
wrote:

> On 6/11/2014 3:02 PM, Coleen Phillimore wrote:
>
>>
>> David and Serguei (and Jeremy), see below.   Summary: I think Jeremy's
>> code and comments are good.
>>
>> On 11/5/14, 10:11 PM, David Holmes wrote:
>>
>>> On 6/11/2014 1:00 PM, Coleen Phillimore wrote:
>>>
>>>>
>>>> On 11/5/14, 8:11 PM, serguei.spit...@oracle.com wrote:
>>>>
>>>>>
>>>>> On 11/5/14 4:35 PM, Jeremy Manson wrote:
>>>>>
>>>>>>
>>>>>>
>>>>>> On Wed, Nov 5, 2014 at 3:40 PM, Coleen Phillimore
>>>>>> mailto:coleen.phillim...@oracle.com>>
>>>>>> wrote:
>>>>>>
>>>>>>
>>>>>> On 11/5/14, 6:13 PM, Jeremy Manson wrote:
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Tue, Nov 4, 2014 at 8:56 PM, serguei.spit...@oracle.com
>>>>>>> <mailto:serguei.spit...@oracle.com> >>>>>> <mailto:serguei.spit...@oracle.com>> wrote:
>>>>>>>
>>>>>>> The fix looks good in general.
>>>>>>>
>>>>>>> src/share/vm/oops/method.cpp
>>>>>>>
>>>>>>> 1785   bool contains(Method** m) {
>>>>>>> 1786 if (m == NULL) return false;
>>>>>>> 1787 for (JNIMethodBlockNode* b = &_head; b != NULL; b
>>>>>>> = b->_next) {
>>>>>>> 1788   if (b->_methods <= m && m < b->_methods +
>>>>>>> b->_number_of_methods) {
>>>>>>> *1789 ptrdiff_t idx = m - b->_methods;**
>>>>>>> **1790 if (b->_methods + idx == m) {**
>>>>>>> 1791   return true;
>>>>>>> 1792 }*
>>>>>>> 1793   }
>>>>>>> 1794 }
>>>>>>> 1795 return false;  // not found
>>>>>>> 1796   }
>>>>>>>
>>>>>>>
>>>>>>> Just noticed that the lines 1789-1792 can be replaced with
>>>>>>> one liner:
>>>>>>> *return true;*
>>>>>>>
>>>>>>>
>>>>>>> Ah, you have found our crappy workaround for wild pointers to
>>>>>>> non-aligned places in the middle of _methods.
>>>>>>>
>>>>>>
>>>>>> Can you explain this?  Why are there wild pointers?
>>>>>>
>>>>>>
>>>>>> My belief was that end user code could pass any old garbage to this
>>>>>> function.  It's called by Method::is_method_id, which is called
>>>>>> by jniCheck::validate_jmethod_id.  The idea was that this would help
>>>>>> check jni deliver useful information in the case of the end user
>>>>>> inputting garbage that happened to be in the right memory range.
>>>>>>
>>>>>> Having said that, at a second glance, it looks as if it that call is
>>>>>> protected by a call to is_method() (in checked_resolve_jmethod_id),
>>>>>> so the program will probably crash before it gets to this check.
>>>>>>
>>>>>> The other point about it was that the result of >= and < is
>>>>>> technically unspecified; if it were ever implemented as anything
>>>>>> other than a binary comparison between integers (which it never is,
>>>>>> now that no one has a segmented architecture), the comparison could
>>>>>> pass spuriously, so checking would be a good thing.  Of course, the
>>>>>> comparison could fail spuriously, too.
>>>>>>
>>>>>> Anyway, I'm happy to leave it in as belt-and-suspenders (and add a
>>>>>> comment, obviously, since it has caused confusion), or take it out.
>>>>>> Your call.

Deadlocked Thread State is RUNNABLE?

[Jeremy Manson]

Hi folks,

I notice that when you send a sigquit to a VM, and threads are
deadlocked in class initialization, then the thread state is reported
as RUNNABLE.  Is this deliberate?

With the attached program (which mostly deadlocks on clinit - I don't
have the time to remember how to write the
100-percent-will-always-deadlock version today), here is the output
from JDK7 b74.

2009-11-17 15:41:14
Full thread dump OpenJDK Server VM (17.0-b03 mixed mode):

"DestroyJavaVM" prio=10 tid=0xaf519c00 nid=0x2221 waiting on condition
[0x]
   java.lang.Thread.State: RUNNABLE

"Thread-1" prio=10 tid=0xaf518000 nid=0x2230 in Object.wait() [0xaf3db000]
   java.lang.Thread.State: RUNNABLE
at NPE$2.run(NPE.java:60)
at java.lang.Thread.run(Thread.java:717)

"Thread-0" prio=10 tid=0xaf516800 nid=0x222f in Object.wait() [0xaf42c000]
   java.lang.Thread.State: RUNNABLE
at Base.(NPE.java:33)
at NPE$1.run(NPE.java:54)
at java.lang.Thread.run(Thread.java:717)

"Low Memory Detector" daemon prio=10 tid=0xaf500c00 nid=0x222d
runnable [0x]
   java.lang.Thread.State: RUNNABLE

"CompilerThread1" daemon prio=10 tid=0x08109c00 nid=0x222c waiting on
condition [0x]
   java.lang.Thread.State: RUNNABLE

"CompilerThread0" daemon prio=10 tid=0x08107800 nid=0x222b waiting on
condition [0x]
   java.lang.Thread.State: RUNNABLE

"Signal Dispatcher" daemon prio=10 tid=0x08105c00 nid=0x222a waiting
on condition [0x]
   java.lang.Thread.State: RUNNABLE

"Finalizer" daemon prio=10 tid=0x080f6c00 nid=0x2229 in Object.wait()
[0xaf821000]
   java.lang.Thread.State: WAITING (on object monitor)
at java.lang.Object.wait(Native Method)
- waiting on <0xecf30bd0> (a java.lang.ref.ReferenceQueue$Lock)
at java.lang.ref.ReferenceQueue.remove(ReferenceQueue.java:135)
- locked <0xecf30bd0> (a java.lang.ref.ReferenceQueue$Lock)
at java.lang.ref.ReferenceQueue.remove(ReferenceQueue.java:151)
at java.lang.ref.Finalizer$FinalizerThread.run(Finalizer.java:177)

"Reference Handler" daemon prio=10 tid=0x080f2000 nid=0x2228 in
Object.wait() [0xaf872000]
   java.lang.Thread.State: WAITING (on object monitor)
at java.lang.Object.wait(Native Method)
- waiting on <0xecf30990> (a java.lang.ref.Reference$Lock)
at java.lang.Object.wait(Object.java:502)
at java.lang.ref.Reference$ReferenceHandler.run(Reference.java:133)
- locked <0xecf30990> (a java.lang.ref.Reference$Lock)

"VM Thread" prio=10 tid=0x080ed400 nid=0x2227 runnable

"GC task thread#0 (ParallelGC)" prio=10 tid=0x08057400 nid=0x runnable

"GC task thread#1 (ParallelGC)" prio=10 tid=0x08058c00 nid=0x2223 runnable

"GC task thread#2 (ParallelGC)" prio=10 tid=0x0805a000 nid=0x2224 runnable

"GC task thread#3 (ParallelGC)" prio=10 tid=0x0805b800 nid=0x2225 runnable

"VM Periodic Task Thread" prio=10 tid=0xaf502c00 nid=0x222e waiting on
condition

JNI global references: 643

Heap
 PSYoungGen  total 12480K, used 860K [0xecf3, 0xedd1, 0xf40f)
  eden space 10752K, 8% used [0xecf3,0xed007148,0xed9b)
  from space 1728K, 0% used [0xedb6,0xedb6,0xedd1)
  to   space 1728K, 0% used [0xed9b,0xed9b,0xedb6)
 PSOldGentotal 113792K, used 0K [0xb40f, 0xbb01, 0xecf3)
  object space 113792K, 0% used [0xb40f,0xb40f,0xbb01)
 PSPermGen   total 16384K, used 1764K [0xb00f, 0xb10f, 0xb40f)
  object space 16384K, 10% used [0xb00f,0xb02a91d0,0xb10f)
import java.util.*;
import java.util.concurrent.*;

import java.security.SecureRandom;

import java.io.FileInputStream;
import java.io.IOException;
import java.io.OutputStream;
import java.lang.management.ManagementFactory;
import java.util.concurrent.*;
import java.util.concurrent.locks.*;
import java.net.*;
 import java.lang.reflect.Field;


import javax.management.*;
import java.lang.management.*;

import javax.script.*;

import java.io.*;

import java.util.Random;
import java.security.SecureRandom;

abstract class Base {
  static DerivedA a;
  static DerivedB b;
  static {
a = new DerivedA();
Thread.yield();
while (NPE.proceed != 2);
b = new DerivedB();
  }
}

class DerivedA extends Base {

}

class DerivedB extends Base {

}


public class NPE {
  public static volatile int proceed = 0;
  static DerivedA a;
  static DerivedB b;
  public static void main(String[] args) {
new Thread(new Runnable() {
public void run() {
  proceed ++;
  a = new DerivedA();
}
  }).start();
new Thread(new Runnable() {
public void run() {
  proceed ++;
  b = new DerivedB();
}
  }).start();
  }
}

DTraceAllocProbes in x86-64 templateTable?

[Jeremy Manson]

Hi folks,

I noticed that the call to DTraceAllocProbes in the 64-bit x86
templateTable is directly after a jump instruction that jumps over it.
 I don't have dtrace to check, but that can't be right, can it?

It's line 3245 of:

http://hg.openjdk.java.net/jdk7/hotspot-comp-gate/hotspot/file/c028504fdaa6/src/cpu/x86/vm/templateTable_x86_64.cpp

The 32-bit version appears to have this code correctly placed above the jmp.

Jeremy

Re: DTraceAllocProbes in x86-64 templateTable?

[Jeremy Manson]

This has seriously gone unfixed for two years?

Jeremy

On Thu, Feb 4, 2010 at 6:24 PM, David Holmes - Sun Microsystems
 wrote:
> Known bug: 6587322
>
> Cheers,
> David
>
> Keith McGuigan said the following on 02/05/10 12:13:
>>
>> Jeremy Manson wrote:
>>>
>>> Hi folks,
>>>
>>> I noticed that the call to DTraceAllocProbes in the 64-bit x86
>>> templateTable is directly after a jump instruction that jumps over it.
>>>  I don't have dtrace to check, but that can't be right, can it?
>>>
>>> It's line 3245 of:
>>>
>>>
>>> http://hg.openjdk.java.net/jdk7/hotspot-comp-gate/hotspot/file/c028504fdaa6/src/cpu/x86/vm/templateTable_x86_64.cpp
>>>
>>> The 32-bit version appears to have this code correctly placed above the
>>> jmp.
>>
>> Yes, that looks wrong.  Good catch.  I'll verify w/dtrace that this is
>> actually wrong and file a bug if so (which I'm sure it will be).
>>
>> --
>> - Keith
>

Re: DTraceAllocProbes in x86-64 templateTable?

[Jeremy Manson]

Seems to be fixed now.  Thanks!

Jeremy

On Thu, Feb 4, 2010 at 8:13 PM, David Holmes - Sun Microsystems
 wrote:
> I've "touched" the CR to raise it to the attention of the RE.
>
> David
>
> Jeremy Manson said the following on 02/05/10 13:03:
>>
>> This has seriously gone unfixed for two years?
>>
>> Jeremy
>>
>> On Thu, Feb 4, 2010 at 6:24 PM, David Holmes - Sun Microsystems
>>  wrote:
>>>
>>> Known bug: 6587322
>>>
>>> Cheers,
>>> David
>>>
>>> Keith McGuigan said the following on 02/05/10 12:13:
>>>>
>>>> Jeremy Manson wrote:
>>>>>
>>>>> Hi folks,
>>>>>
>>>>> I noticed that the call to DTraceAllocProbes in the 64-bit x86
>>>>> templateTable is directly after a jump instruction that jumps over it.
>>>>>  I don't have dtrace to check, but that can't be right, can it?
>>>>>
>>>>> It's line 3245 of:
>>>>>
>>>>>
>>>>>
>>>>> http://hg.openjdk.java.net/jdk7/hotspot-comp-gate/hotspot/file/c028504fdaa6/src/cpu/x86/vm/templateTable_x86_64.cpp
>>>>>
>>>>> The 32-bit version appears to have this code correctly placed above the
>>>>> jmp.
>>>>
>>>> Yes, that looks wrong.  Good catch.  I'll verify w/dtrace that this is
>>>> actually wrong and file a bug if so (which I'm sure it will be).
>>>>
>>>> --
>>>> - Keith
>

Re: code review request for JVM/TI CompiledMethodLoad event extension (6580131)

[Jeremy Manson]

Resurrecting an old thread...

Is there any reason that no one ever frees the arrays that are created
by this changeset in create_inline_record() in jvmtiExport.cpp?  Or am
I just missing where they are freed?

I don't think they are freed elsewhere...  I added a method that
performed some obvious freeing at the end of
post_compiled_method_load(), ran a CompileTheWorld, and it seemed to
work okay...

Jeremy

On Wed, Jan 20, 2010 at 6:30 PM, Daniel D. Daugherty
 wrote:
> Greetings,
>
> The jvmticmlr.h stuff is in:
>
> - OpenJDK6 - JDK and HotSpot sides
> - JDK7 - JDK side
> - HSX17 - HotSpot side
>
> When HSX17-B08 pushes to JDK7, then the HotSpot side will
> be in JDK7 also.
>
> We'll be looking at the JDK6-Update train next...
>
> Dan
>
>
>
>
> Daniel D. Daugherty wrote:
>>
>> Greetings,
>>
>> The folks at AMD Labs have been kind enough to provide an
>> extension to the JVM/TI CompileMethodLoad event in order
>> to provide additional information about in-lining. This
>> extension uses the existing (but previously unused)
>> compile_info paramter:
>>
>>
>> http://java.sun.com/javase/6/docs/platform/jvmti/jvmti.html#CompiledMethodLoad
>>
>> Vasanth and company provided the HotSpot code changes and
>> the original demo program. I just did the Makefile changes
>> to export the new jvmticmlr.h file in the HotSpot repo and
>> the integration of the demo program into JAVA_HOME/demo/jvmti
>> in the JDK repo.
>>
>> Here is the webrev for the OpenJDK6 version of the changes:
>>
>>   http://cr.openjdk.java.net/~dcubed/6580131-webrev/0/
>>
>> The OpenJDK7 version of these changes are not expected to be
>> very different from this version.
>>
>> For the Sun folks, the CCC request for adding the jvmticmlr.h
>> is almost final. I'm waiting for the VM-SQE team to agree that
>> the latest version addresses their review concerns, but I
>> think the St Petersburg team is on holiday at the moment.
>>
>> Any reviews are appreciated.
>>
>> Dan
>>
>
>
> -- Forwarded message --
> From: daniel.daughe...@sun.com
> To: jdk6-...@openjdk.java.net
> Date: Wed, 20 Jan 2010 20:14:48 +
> Subject: hg: jdk6/jdk6/hotspot: 2 new changesets
> Changeset: 7fbf850d87b7
> Author:    dcubed
> Date:      2010-01-13 09:39 -0700
> URL:       http://hg.openjdk.java.net/jdk6/jdk6/hotspot/rev/7fbf850d87b7
>
> 6580131: 3/4 CompiledMethodLoad events don't produce the expected extra
> notifications to describe inlining
> Summary: Add support for additional implementation specific info to the
> JVM/TI CompiledMethodLoad event via the compile_info parameter.
> Reviewed-by: never, ohair, tbell, tdeneau
> Contributed-by: Vasanth Venkatachalam 
>
> ! make/Makefile
> ! make/defs.make
> + src/share/vm/code/jvmticmlr.h
> ! src/share/vm/includeDB_core
> ! src/share/vm/prims/jvmtiExport.cpp
>
> Changeset: 7dbe24cb959c
> Author:    dcubed
> Date:      2010-01-20 10:35 -0700
> URL:       http://hg.openjdk.java.net/jdk6/jdk6/hotspot/rev/7dbe24cb959c
>
> Merge
>
> ! make/Makefile
> ! make/defs.make
> ! src/share/vm/includeDB_core
> ! src/share/vm/prims/jvmtiExport.cpp
>
>
>
>
> -- Forwarded message --
> From: daniel.daughe...@sun.com
> To: jdk7-chan...@openjdk.java.net, hotspot-runtime-...@openjdk.java.net,
> serviceability-dev@openjdk.java.net
> Date: Wed, 20 Jan 2010 21:10:36 +
> Subject: hg: jdk7/hotspot-rt/hotspot: 2 new changesets
> Changeset: 7fbf850d87b7
> Author:    dcubed
> Date:      2010-01-13 09:39 -0700
> URL:
> http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/7fbf850d87b7
>
> 6580131: 3/4 CompiledMethodLoad events don't produce the expected extra
> notifications to describe inlining
> Summary: Add support for additional implementation specific info to the
> JVM/TI CompiledMethodLoad event via the compile_info parameter.
> Reviewed-by: never, ohair, tbell, tdeneau
> Contributed-by: Vasanth Venkatachalam 
>
> ! make/Makefile
> ! make/defs.make
> + src/share/vm/code/jvmticmlr.h
> ! src/share/vm/includeDB_core
> ! src/share/vm/prims/jvmtiExport.cpp
>
> Changeset: 3908ad124838
> Author:    dcubed
> Date:      2010-01-20 11:32 -0700
> URL:
> http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/3908ad124838
>
> Merge
>
> ! make/Makefile
> ! make/defs.make
> ! src/share/vm/includeDB_core
> ! src/share/vm/prims/jvmtiExport.cpp
>
>
>
>
> -- Forwarded message --
> From: daniel.daughe...@sun.com
> To: jdk6-...@openjdk.java.net
> Date: Wed, 20 Jan 2010 22:10:34 +
> Subject: hg: jdk6/jdk6/jdk: 2 new changesets
> Changeset: 6073840fbd22
> Author:    dcubed
> Date:      2010-01-13 09:42 -0700
> URL:       http://hg.openjdk.java.net/jdk6/jdk6/jdk/rev/6073840fbd22
>
> 6580131: 3/4 CompiledMethodLoad events don't produce the expected extra
> notifications to describe inlining
> Summary: Add support for additional implementation specific info to the
> JVM/TI CompiledMethodLoad event via the compile_info parameter.
> Reviewed-by: never, ohair, tbell, tdeneau
> Contributed-by: Vasanth Venkatachalam 
>
> ! make/

Re: code review request for JVM/TI CompiledMethodLoad event extension (6580131)

[Jeremy Manson]

Okay, thanks.

Jeremy

On Mon, May 3, 2010 at 4:22 PM, Tom Rodriguez  wrote:
> They are resource allocated so they don't need to be free'd.  The destructor 
> of ResourceMark will release any storage that was allocated.
>
> tom

Re: Code Review for WeakReference leak in the Logging API (6942989)

[Jeremy Manson]

We also fixed this bug internally at Google (sending the patch out was
a TODO, but we never got around to it).  If you have any interest in
comparing / contrasting approaches, let us know.

Jeremy

On Thu, Jun 10, 2010 at 10:13 AM, Daniel D. Daugherty
 wrote:
> Greetings,
>
> I need a couple of code reviews for my fix for the WeakReference leak
> in the Logging API. The webrev is relative to OpenJDK7, but the bug
> is escalated so the fix will be backported to the JDK6-Update train.
> That's why I need at least two code reviewers.
>
> Here is the URL for the webrev:
>
>   http://cr.openjdk.java.net/~dcubed/6942989-webrev/0/
>
> For some reason, the OpenJDK bug link isn't working which is strange
> considering the bug report came in from forums.java.net.
>
> Thanks, in advance, for any reviews.
>
> Dan
>
>

Re: Code Review for WeakReference leak in the Logging API (6942989)

[Jeremy Manson]

Daniel,

The fix hasn't made it to OpenJDK6.  We were planning on pushing it to
OpenJDK6/7, but we haven't had time for it yet.  If your fix is better
(I haven't had a chance to look at it), then we'll happily drop ours
in favor of yours.

For testing: I hand tested it with the "create lots of anonymous
loggers" test.  My major observation was that creating that many weak
references and rebuilding the internal data structures from scratch
repeatedly can bring a system to its knees.  This is why the
weakReferencesProcessed variable exists - we don't rebuild the
internal data structures with every additional logger that we lose.

We also ran jtreg, which didn't seem to have any problems.

The doc fixes: Our secret goal was to sneak those in without having to
file a separate bug, but I guess you caught us. ;)

Jeremy

On Mon, Jun 14, 2010 at 9:46 AM, Daniel D. Daugherty
 wrote:
> On 6/11/2010 4:41 PM, Martin Buchholz wrote:
>>
>> On Fri, Jun 11, 2010 at 14:46, Daniel D. Daugherty
>>  wrote:
>>
>>>
>>> Jeremy,
>>>
>>> I'm definitely interested in learning about your approach to this issue.
>>>
>>
>> Here's the patch against openjdk6 by Jeremy.
>> http://cr.openjdk.java.net/~martin/WeakLogger-jeremymanson.patch
>> (It would take a bit of merging to port to openjdk7)
>>
>> Feel free to take anything from our change.
>> Apologies for the perforce-isms.
>>
>> Martin
>>
>
> Jeremy and Martin,
>
> Thanks for the proposed fix. A couple of questions:
>
> - This changeset is private to Google right now, correct? As in
>  it hasn't made it into OpenJDK6 yet.
> - Do you plan on pushing this changeset to OpenJDK6?
> - What kind of testing has been done on it?
>
> Thanks for the offer for the code. I'll start wading through the
> diffs today. Because this is an escalated issue, I will likely
> be taking just the code and comments directly related to the
> problem at hand. The JavaDoc fixes, even though they are useful,
> will have to wait for a different changeset.
>
> Thanks for jumping in on this thread.
>
> Dan
>
>

Re: Code Review for WeakReference leak in the Logging API (6942989)

[Jeremy Manson]

Daniel,

We're happy to contribute.  Like you, we had a customer complaint,
which is why this happened.

My suspicion is that we don't have access to the VM/NSK test suite.
Feel free to run the patch against it.

Jeremy

On Mon, Jun 14, 2010 at 10:45 AM, Daniel D. Daugherty
 wrote:
> On 6/14/2010 11:30 AM, Jeremy Manson wrote:
>>
>> Daniel,
>>
>> The fix hasn't made it to OpenJDK6.  We were planning on pushing it to
>> OpenJDK6/7, but we haven't had time for it yet.  If your fix is better
>> (I haven't had a chance to look at it), then we'll happily drop ours
>> in favor of yours.
>>
>
> I will be looking at your fix today. Mine is a brute force big hammer
> style fix that I'm not fond of, but does the job. Your fix will likely
> address Eamonn's review comments and also solve the potential performance
> impact that mine would have in systems with lots of Loggers.
>
>> For testing: I hand tested it with the "create lots of anonymous
>> loggers" test.  My major observation was that creating that many weak
>> references and rebuilding the internal data structures from scratch
>> repeatedly can bring a system to its knees.  This is why the
>> weakReferencesProcessed variable exists - we don't rebuild the
>> internal data structures with every additional logger that we lose.
>>
>
> I'll be sure to keep my eyes open for that part of the fix.
>
>
>> We also ran jtreg, which didn't seem to have any problems.
>>
>
> There are only 10 or so tests in JTREG. The VM/NSK logging test suite
> has 550+ tests and it caught my breakage due to the currently legal
> Logger loop in one of the tests.
>
>
>> The doc fixes: Our secret goal was to sneak those in without having to
>> file a separate bug, but I guess you caught us. ;)
>>
>
> Perhaps I can sweep those up for you. We'll have to see how it goes.
>
> Thanks again for contributing the fix!
>
> Dan
>
>
>> Jeremy
>>
>> On Mon, Jun 14, 2010 at 9:46 AM, Daniel D. Daugherty
>>  wrote:
>>
>>>
>>> On 6/11/2010 4:41 PM, Martin Buchholz wrote:
>>>
>>>>
>>>> On Fri, Jun 11, 2010 at 14:46, Daniel D. Daugherty
>>>>  wrote:
>>>>
>>>>
>>>>>
>>>>> Jeremy,
>>>>>
>>>>> I'm definitely interested in learning about your approach to this
>>>>> issue.
>>>>>
>>>>>
>>>>
>>>> Here's the patch against openjdk6 by Jeremy.
>>>> http://cr.openjdk.java.net/~martin/WeakLogger-jeremymanson.patch
>>>> (It would take a bit of merging to port to openjdk7)
>>>>
>>>> Feel free to take anything from our change.
>>>> Apologies for the perforce-isms.
>>>>
>>>> Martin
>>>>
>>>>
>>>
>>> Jeremy and Martin,
>>>
>>> Thanks for the proposed fix. A couple of questions:
>>>
>>> - This changeset is private to Google right now, correct? As in
>>>  it hasn't made it into OpenJDK6 yet.
>>> - Do you plan on pushing this changeset to OpenJDK6?
>>> - What kind of testing has been done on it?
>>>
>>> Thanks for the offer for the code. I'll start wading through the
>>> diffs today. Because this is an escalated issue, I will likely
>>> be taking just the code and comments directly related to the
>>> problem at hand. The JavaDoc fixes, even though they are useful,
>>> will have to wait for a different changeset.
>>>
>>> Thanks for jumping in on this thread.
>>>
>>> Dan
>>>
>>>
>>>
>

Re: Code Review for WeakReference leak in the Logging API (6942989)

[Jeremy Manson]

Okay.  It sounds as if the changes were helpful, anyway.  I'll be
interested to see what you do.

Jeremy

On Mon, Jun 14, 2010 at 4:20 PM, Daniel D. Daugherty
 wrote:
> On 6/14/2010 12:36 PM, Jeremy Manson wrote:
>>
>> Daniel,
>>
>> We're happy to contribute.  Like you, we had a customer complaint,
>> which is why this happened.
>>
>
> And I see that you did this work against an earlier bug. I've
> made myself the RE for 6931561 and I've update the evaluation
> to indicate that I'm likely to close it as a dup of the escalated
> bug (6942989).
>
>
>> My suspicion is that we don't have access to the VM/NSK test suite.
>>
>
> You're right. I think that is currently an internal test suite.
>
>
>> Feel free to run the patch against it.
>>
>
> I've just finished my code review of the patch and I have some
> issues with it. In the interests of making forward progress on my
> escalation, I'm going to investigate creating a hybrid solution
> of some of your changes, some of Eamonn's ideas about creating
> WeakReference subclasses, and Tony P's ideas in the following
> http://java.sun.com/developer/technicalArticles/javase/finalization/
>
> In particular, Tony's article points out that a strong reference
> to the WeakReference subclass needs to be held to make sure that
> the WeakReference subclass object is still around when we want to
> process it off the ReferenceQueue. Of course, that assumes I
> understood what Tony said :-)
>
> Dan
>
>
>
>> Jeremy
>>
>> On Mon, Jun 14, 2010 at 10:45 AM, Daniel D. Daugherty
>>  wrote:
>>
>>>
>>> On 6/14/2010 11:30 AM, Jeremy Manson wrote:
>>>
>>>>
>>>> Daniel,
>>>>
>>>> The fix hasn't made it to OpenJDK6.  We were planning on pushing it to
>>>> OpenJDK6/7, but we haven't had time for it yet.  If your fix is better
>>>> (I haven't had a chance to look at it), then we'll happily drop ours
>>>> in favor of yours.
>>>>
>>>>
>>>
>>> I will be looking at your fix today. Mine is a brute force big hammer
>>> style fix that I'm not fond of, but does the job. Your fix will likely
>>> address Eamonn's review comments and also solve the potential performance
>>> impact that mine would have in systems with lots of Loggers.
>>>
>>>
>>>>
>>>> For testing: I hand tested it with the "create lots of anonymous
>>>> loggers" test.  My major observation was that creating that many weak
>>>> references and rebuilding the internal data structures from scratch
>>>> repeatedly can bring a system to its knees.  This is why the
>>>> weakReferencesProcessed variable exists - we don't rebuild the
>>>> internal data structures with every additional logger that we lose.
>>>>
>>>>
>>>
>>> I'll be sure to keep my eyes open for that part of the fix.
>>>
>>>
>>>
>>>>
>>>> We also ran jtreg, which didn't seem to have any problems.
>>>>
>>>>
>>>
>>> There are only 10 or so tests in JTREG. The VM/NSK logging test suite
>>> has 550+ tests and it caught my breakage due to the currently legal
>>> Logger loop in one of the tests.
>>>
>>>
>>>
>>>>
>>>> The doc fixes: Our secret goal was to sneak those in without having to
>>>> file a separate bug, but I guess you caught us. ;)
>>>>
>>>>
>>>
>>> Perhaps I can sweep those up for you. We'll have to see how it goes.
>>>
>>> Thanks again for contributing the fix!
>>>
>>> Dan
>>>
>>>
>>>
>>>>
>>>> Jeremy
>>>>
>>>> On Mon, Jun 14, 2010 at 9:46 AM, Daniel D. Daugherty
>>>>  wrote:
>>>>
>>>>
>>>>>
>>>>> On 6/11/2010 4:41 PM, Martin Buchholz wrote:
>>>>>
>>>>>
>>>>>>
>>>>>> On Fri, Jun 11, 2010 at 14:46, Daniel D. Daugherty
>>>>>>  wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> Jeremy,
>>>>>>>
>>>>>>> I'm definitely interested in learning about your approach to this
>>>>>>> issue.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> Here's the patch against openjdk6 by Jeremy.
>>>>>> http://cr.openjdk.java.net/~martin/WeakLogger-jeremymanson.patch
>>>>>> (It would take a bit of merging to port to openjdk7)
>>>>>>
>>>>>> Feel free to take anything from our change.
>>>>>> Apologies for the perforce-isms.
>>>>>>
>>>>>> Martin
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>> Jeremy and Martin,
>>>>>
>>>>> Thanks for the proposed fix. A couple of questions:
>>>>>
>>>>> - This changeset is private to Google right now, correct? As in
>>>>>  it hasn't made it into OpenJDK6 yet.
>>>>> - Do you plan on pushing this changeset to OpenJDK6?
>>>>> - What kind of testing has been done on it?
>>>>>
>>>>> Thanks for the offer for the code. I'll start wading through the
>>>>> diffs today. Because this is an escalated issue, I will likely
>>>>> be taking just the code and comments directly related to the
>>>>> problem at hand. The JavaDoc fixes, even though they are useful,
>>>>> will have to wait for a different changeset.
>>>>>
>>>>> Thanks for jumping in on this thread.
>>>>>
>>>>> Dan
>>>>>
>>>>>
>>>>>
>>>>>
>>
>>
>

Re: Second Code Review for WeakReference leak in the Logging API (6942989)

[Jeremy Manson]

Hi Daniel,

I'm sorry I missed this (I heavily filter these lists, and check rarely).

My main feeling is that you are missing a good bet by not
reconstructing the Hashtable in LogManager and the ArrayList in Logger
every so often when you remove the loggers.  In a test case where
there are a LOT of short-lived loggers, the backing array for the
Hashtable can get very big.  It is permanent, and doesn't go anywhere.
 You can end up with a lot of extra memory lying around that way.

Specifically, when I didn't reconstruct those data structures, the
test case listed in the bug (where it just creates lots and lots of
anonymous loggers) killed the Java instance with an OOM, even if I
*did* clean up the weakreferences to the loggers.

I'm assuming you have a customer waiting for this - if that is similar
to their usage pattern, this fix may not fix their problem.

You obviously don't want to rebuild those structures every time,
though.  What I did in my change was to reconstruct the backing data
structures every time ~as many loggers were collected as were present
in the data structure.

Jeremy

On Fri, Jun 18, 2010 at 12:25 PM, Daniel D. Daugherty
 wrote:
> Greetings,
>
> I have a new version of my fix for the WeakReference leak in the
> Logging API done. This version uses ReferenceQueues; thanks to Eamonn
> McManus, Jeremy Manson and Tony Printezis for their insights on using
> ReferenceQueues. Here's a pointer to Tony's paper for background info:
>
>    http://java.sun.com/developer/technicalArticles/javase/finalization/
>
> This version also has limits on the number of dead Loggers that are
> cleaned up per call; thanks to Alan Bateman for politely pushing me in
> that direction.
>
> The webrev is again relative to OpenJDK7, but the bug is escalated so
> the fix will be backported to the JDK6-Update train. So again, I'll
> need a minimum of two code reviewers.
>
> Here is the URL for the webrev:
>
>    http://cr.openjdk.java.net/~dcubed/6942989-webrev/1/
>
> Thanks, in advance, for any reviews.
>
> Dan
>
>

Re: Second Code Review for WeakReference leak in the Logging API (6942989)

[Jeremy Manson]

GMT+00:00 daniel.daughe...@oracle.com
>>
>> I ran the AnonLoggerWeakRefLeak test with the same environment
>> variables for a 60 minute duration using the fixed bits from
>> JPRT. Here are the last 15 lines of each log (much less
>> interesting info):
>>
>> $ tail -15 AnonLogger.jdk7-B100+_60min.log
>> INFO: instance_cnt = 5801
>> INFO: instance_cnt = 5901
>> INFO: instance_cnt = 5701
>> INFO: instance_cnt = 5601
>> INFO: jmap exited with exit code = 1
>> INFO: The likely reason is that AnonLoggerWeakRefLeak has finished
>> running.
>> INFO: increasing_cnt = 227
>> INFO: decreasing_cnt = 383
>> INFO: The instance count of java.lang.ref.WeakReference objects
>> INFO: is both increasing and decreasing.
>> PASS: This indicates that there is not a memory leak.
>>
>> real    60m5.099s
>> user    2m18.882s
>> sys     2m31.127s
>>
>> $ tail -15 AnonLoggerWeakRefLeak.jdk7-B100+_60min.log
>> INFO: call count = 3484000
>> INFO: call count = 3485000
>> INFO: call count = 3486000
>> INFO: call count = 3487000
>> INFO: call count = 3488000
>> INFO: call count = 3489000
>> INFO: call count = 349
>> INFO: call count = 3491000
>> INFO: call count = 3492000
>> INFO: call count = 3493000
>> INFO: call count = 3494000
>> INFO: call count = 3495000
>> INFO: call count = 3496000
>> INFO: call count = 3497000
>> INFO: final loop count = 3497200
>>
>>
>> I ran the LoggerWeakRefLeak test with the same environment
>> variables for a 60 minute duration using the fixed bits from
>> JPRT. Here are the last 15 lines of each log (much less
>> interesting info):
>>
>> $ tail -15 Logger.jdk7-B100+_60min.log
>> INFO: instance_cnt = 1813
>> INFO: instance_cnt = 1638
>> INFO: instance_cnt = 1514
>> INFO: instance_cnt = 1511
>> INFO: jmap exited with exit code = 1
>> INFO: The likely reason is that LoggerWeakRefLeak has finished running.
>> INFO: increasing_cnt = 293
>> INFO: decreasing_cnt = 318
>> INFO: The instance count of java.lang.ref.WeakReference objects
>> INFO: is both increasing and decreasing.
>> PASS: This indicates that there is not a memory leak.
>>
>> real    60m6.783s
>> user    2m20.592s
>> sys     2m31.997s
>>
>> $ tail -15 LoggerWeakRefLeak.jdk7-B100+_60min.log
>> INFO: call count = 3502000
>> INFO: call count = 3503000
>> INFO: call count = 3504000
>> INFO: call count = 3505000
>> INFO: call count = 3506000
>> INFO: call count = 3507000
>> INFO: call count = 3508000
>> INFO: call count = 3509000
>> INFO: call count = 351
>> INFO: call count = 3511000
>> INFO: call count = 3512000
>> INFO: call count = 3513000
>> INFO: call count = 3514000
>> INFO: call count = 3515000
>> INFO: final loop count = 3515800
>>
>> *** (#2 of 3): 2010-06-29 01:46:49 GMT+00:00 daniel.daughe...@oracle.com
>>
>> I ran the AnonLoggerWeakRefLeak test with the same environment
>> variables for a 12 hour duration using the fixed bits from JPRT.
>> For this run I saved the "Total" line from the jmap output from
>> every 10th sample:
>>
>> $ diff ../AnonLoggerWeakRefLeak.sh AnonLoggerWeakRefLeak.sh
>> 161a162
>>
>>>>
>>>> > > "$TEST_NAME.totals"
>>>>
>>
>> 225a227,233
>>
>>>
>>> > > set +e
>>> > mod=`expr "$loop_cnt" % 10`
>>> > set -e
>>> > if [ "$mod" = 0 ]; then
>>> > tail -1 "$TEST_NAME.jmap" >> "$TEST_NAME.totals"
>>> > fi
>>>
>>
>> Here is an analysis of the .totals data:
>>
>> $ sh analyze_totals < AnonLoggerWeakRefLeak.totals
>>          #objs      #bytes
>> first:    30537     2243648
>> lo:       30537     2243648
>> hi:       57072     3197904
>> last:     35676     2882488
>> avg:      36853     2929982
>> # samples: 647
>>
>> The first sample is also the lowest set of values which isn't
>> a surprise given that the first sample is taken shortly after
>> the target program has started running. The hi value occurred
>> in sample #269 of 647 and the last sample was below the average.
>> This data indicates that the values are both rising and falling
>> over time which does not indicate any more memory leaks.
>>
>>
>>
>> I also ran the LoggerWeakRefLeak test with the same environment
>> variables for a 12 hour duration using the fixed b

JVMTI OOM handling when arrays / objects are too large

[Jeremy Manson]

Hi folks,

I was talking to some of the HS team at JavaOne, and they encouraged
me to send some of the patches that we are working on at Google back
to Sun.  (Everyone at Google is covered under a blanket SCA, so we can
contribute back anything done by any Googler).

To get started and test the waters, and because I have never tried to
contribute to OpenJDK before, I thought I would send on a
nearly-trivial fix that I made last year in response to a user
complaint.  The issue was that when the user specifies
-XX:OnOutOfMemoryError, and the OOM is thrown because a single memory
allocation was too close to Integer.MAX_VALUE, the OnOutOfMemoryError
command would not be executed.  I've attached the patch.  Let me know
what I should do to proceed.

Thanks!

Jeremy
# HG changeset patch
# User jeremyman...@iago.mtv.corp.google.com
# Date 1244235210 25200
# Node ID cf2fd9253952b92e24b982f83ba1bc2faf584e8b
# Parent  aa0c48844632739a1efa8ff78f24ff4017fda89f
JVMTI does not execute post_resource_exhausted when allocating arrays close to 
Integer.MAX_VALUE.

diff -r aa0c48844632 -r cf2fd9253952 
src/share/vm/gc_interface/collectedHeap.inline.hpp
--- a/src/share/vm/gc_interface/collectedHeap.inline.hppThu May 14 
10:57:58 2009 -0700
+++ b/src/share/vm/gc_interface/collectedHeap.inline.hppFri Jun 05 
13:53:30 2009 -0700
@@ -135,28 +135,14 @@ HeapWord* CollectedHeap::common_mem_allo
   }
 
 
-  if (!gc_overhead_limit_was_exceeded) {
-// -XX:+HeapDumpOnOutOfMemoryError and -XX:OnOutOfMemoryError support
-report_java_out_of_memory("Java heap space");
-
-if (JvmtiExport::should_post_resource_exhausted()) {
-  JvmtiExport::post_resource_exhausted(
-JVMTI_RESOURCE_EXHAUSTED_OOM_ERROR | 
JVMTI_RESOURCE_EXHAUSTED_JAVA_HEAP,
-"Java heap space");
-}
-
-THROW_OOP_0(Universe::out_of_memory_error_java_heap());
+ if (!gc_overhead_limit_was_exceeded) {
+THROW_OOM("Java heap space",
+  JVMTI_RESOURCE_EXHAUSTED_OOM_ERROR | 
JVMTI_RESOURCE_EXHAUSTED_JAVA_HEAP,
+  Universe::out_of_memory_error_java_heap());
   } else {
-// -XX:+HeapDumpOnOutOfMemoryError and -XX:OnOutOfMemoryError support
-report_java_out_of_memory("GC overhead limit exceeded");
-
-if (JvmtiExport::should_post_resource_exhausted()) {
-  JvmtiExport::post_resource_exhausted(
-JVMTI_RESOURCE_EXHAUSTED_OOM_ERROR | 
JVMTI_RESOURCE_EXHAUSTED_JAVA_HEAP,
-"GC overhead limit exceeded");
-}
-
-THROW_OOP_0(Universe::out_of_memory_error_gc_overhead_limit());
+THROW_OOM("GC overhead limit exceeded",
+  JVMTI_RESOURCE_EXHAUSTED_OOM_ERROR | 
JVMTI_RESOURCE_EXHAUSTED_JAVA_HEAP,
+  Universe::out_of_memory_error_gc_overhead_limit());
   }
 }
 
@@ -191,16 +177,10 @@ HeapWord* CollectedHeap::common_permanen
"Unexpected exception, will result in uninitialized storage");
 return result;
   }
-  // -XX:+HeapDumpOnOutOfMemoryError and -XX:OnOutOfMemoryError support
-  report_java_out_of_memory("PermGen space");
 
-  if (JvmtiExport::should_post_resource_exhausted()) {
-JvmtiExport::post_resource_exhausted(
-JVMTI_RESOURCE_EXHAUSTED_OOM_ERROR,
-"PermGen space");
-  }
-
-  THROW_OOP_0(Universe::out_of_memory_error_perm_gen());
+  THROW_OOM("PermGen space",
+JVMTI_RESOURCE_EXHAUSTED_OOM_ERROR,
+Universe::out_of_memory_error_perm_gen());
 }
 
 HeapWord* CollectedHeap::common_permanent_mem_allocate_init(size_t size, 
TRAPS) {
diff -r aa0c48844632 -r cf2fd9253952 src/share/vm/oops/arrayKlass.cpp
--- a/src/share/vm/oops/arrayKlass.cpp  Thu May 14 10:57:58 2009 -0700
+++ b/src/share/vm/oops/arrayKlass.cpp  Fri Jun 05 13:53:30 2009 -0700
@@ -140,7 +140,10 @@ objArrayOop arrayKlass::allocate_arrayAr
 THROW_0(vmSymbols::java_lang_NegativeArraySizeException());
   }
   if (length > arrayOopDesc::max_array_length(T_ARRAY)) {
-THROW_OOP_0(Universe::out_of_memory_error_array_size());
+THROW_OOM("Java heap space",
+  JVMTI_RESOURCE_EXHAUSTED_OOM_ERROR |
+  JVMTI_RESOURCE_EXHAUSTED_JAVA_HEAP,
+  Universe::out_of_memory_error_array_size());
   }
   int size = objArrayOopDesc::object_size(length);
   klassOop k = array_klass(n+dimension(), CHECK_0);
diff -r aa0c48844632 -r cf2fd9253952 src/share/vm/oops/instanceKlass.cpp
--- a/src/share/vm/oops/instanceKlass.cpp   Thu May 14 10:57:58 2009 -0700
+++ b/src/share/vm/oops/instanceKlass.cpp   Fri Jun 05 13:53:30 2009 -0700
@@ -497,7 +497,10 @@ objArrayOop instanceKlass::allocate_objA
 objArrayOop instanceKlass::allocate_objArray(int n, int length, TRAPS) {
   if (length < 0) THROW_0(vmSymbols::java_lang_NegativeArraySizeException());
   if (length > arrayOopDesc::max_array_length(T_OBJECT)) {
-THROW_OOP_0(Universe::out_of_memory_error_array_size());
+THROW_OOM("Java heap space",
+  JVMTI_RESOURCE_EXHAUSTED_OOM_ERROR |
+  JVMTI_RESOURCE_EXHAUSTED_JAVA_HEAP,
+ 

Re: JVMTI OOM handling when arrays / objects are too large

[Jeremy Manson]

Thanks Tim!

Martin Buchholz tells me that three things have to happen to get a patch in:

1) Someone inside Sun has to file a bug.  Usually, this seems to be in
the other bug database, but I guess it doesn't matter?

2) Two OpenJDK members have to review it (in practice).

Is my understanding correct?

Jeremy

On Fri, Jun 5, 2009 at 4:31 PM, Tim Bell  wrote:
> Hi Jeremy
>>
>> I was talking to some of the HS team at JavaOne, and they encouraged
>> me to send some of the patches that we are working on at Google back
>> to Sun.  (Everyone at Google is covered under a blanket SCA, so we can
>> contribute back anything done by any Googler).
>>
>> To get started and test the waters, and because I have never tried to
>> contribute to OpenJDK before, I thought I would send on a
>> nearly-trivial fix that I made last year in response to a user
>> complaint.  The issue was that when the user specifies
>> -XX:OnOutOfMemoryError, and the OOM is thrown because a single memory
>> allocation was too close to Integer.MAX_VALUE, the OnOutOfMemoryError
>> command would not be executed.  I've attached the patch.  Let me know
>> what I should do to proceed.
>
> I created a bugzilla report for this issue:
>
> https://bugs.openjdk.java.net/show_bug.cgi?id=100067
>
> That way we won't lose it in a pile of email.
>
> Tim
>

Re: JVMTI OOM handling when arrays / objects are too large

[Jeremy Manson]

This one happens to be pretty easy to trigger.  I'm not sure how to
put together a jtreg test, but the following code works:

public class OOM {
  public static void main(String[] args) throws Exception {
int size = Integer.MAX_VALUE;
Integer[] props = new Integer[size];
  }
}

And then invoke it:

java -XX:OnOutOfMemoryError="echo PASS"  OOM

If it prints out "PASS", it works.  My understanding is that jtreg
tests fail by throwing an exception, so this whole thing would
probably have to be wrapped in another Java program that throws an
exception if it doesn't see "PASS" in the output.

Jeremy

On Sun, Jun 7, 2009 at 12:31 PM, Tim Bell wrote:
> Hi Jeremy
>>
>> Martin Buchholz tells me that three things have to happen to get a patch
>> in:
>
> Hi Martin :-)
>
>> 1) Someone inside Sun has to file a bug.  Usually, this seems to be in
>> the other bug database, but I guess it doesn't matter?
>
> We hope to get a bridge working between the external bugzilla system
> (good!) and the internal, proprietary bug tracking system (bad!).
> Hopefully after we all recover from JavaOne week.
>
>> 2) Two OpenJDK members have to review it (in practice).
>
> One reviewer is required until the late stages of a release, then
> two reviewers.  Of course, more reviewers are always better.
>
>> Is my understanding correct?
>
> What are the chances of getting a jtreg test case for this issue?
>
> Tests that attempt to exhaust the heap are usually problematical,
> but it would be worth a try.  Nice to have if we can get a test.
>
> Tim
>
>
>> Jeremy
>>
>> On Fri, Jun 5, 2009 at 4:31 PM, Tim Bell  wrote:
>>>
>>> Hi Jeremy

 I was talking to some of the HS team at JavaOne, and they encouraged
 me to send some of the patches that we are working on at Google back
 to Sun.  (Everyone at Google is covered under a blanket SCA, so we can
 contribute back anything done by any Googler).

 To get started and test the waters, and because I have never tried to
 contribute to OpenJDK before, I thought I would send on a
 nearly-trivial fix that I made last year in response to a user
 complaint.  The issue was that when the user specifies
 -XX:OnOutOfMemoryError, and the OOM is thrown because a single memory
 allocation was too close to Integer.MAX_VALUE, the OnOutOfMemoryError
 command would not be executed.  I've attached the patch.  Let me know
 what I should do to proceed.
>>>
>>> I created a bugzilla report for this issue:
>>>
>>> https://bugs.openjdk.java.net/show_bug.cgi?id=100067
>>>
>>> That way we won't lose it in a pile of email.
>>>
>>> Tim
>>>
>
>

Re: JVMTI OOM handling when arrays / objects are too large

[Jeremy Manson]

Hi Tomas (and David, presumably),

Why is this a programming error?  I thought it was just a VM
limitation.  Surely the JVMS allows arrays of any nonnegative integral
size, no?

Jeremy

On Mon, Jun 15, 2009 at 12:20 AM, Tomas Hurka wrote:
> Hi Martin and David,
>
> On 14 Jun 2009, at 21:37, Martin Buchholz wrote:
>
>> I've polished the changes in preparation for committing.
>> I'll commit once I have reviewer approval.
>
> I have the same concern as David. I think that throwing OOME, in places
> where there is obviously a simple programming error, is not a good idea.
> Will it be possible to change it to throw IllegalArgumentException instead?
> I have one more comment about the patch itself - changes to
> src/share/vm/gc_interface/collectedHeap.inline.hpp and
> src/share/vm/utilities/exceptions.hpp are unrelated to the bugfix. I think
> it will be better to separate them to the different changeset.
>
> Bye,
> --
> Tomas Hurka   
> NetBeans Profiler http://profiler.netbeans.org
> VisualVM http://visualvm.dev.java.net
> Software Engineer, Developer Platforms Group
> Sun Microsystems, Praha Czech Republic
>
>

Re: JVMTI OOM handling when arrays / objects are too large

[Jeremy Manson]

FWIW, I also think that having a call to new throw anything that isn't
an OOME is opening up a much larger compatibility can of worms than
anything else I intended with this patch.

Jeremy

On Mon, Jun 15, 2009 at 12:39 AM, Jeremy Manson wrote:
> Hi Tomas (and David, presumably),
>
> Why is this a programming error?  I thought it was just a VM
> limitation.  Surely the JVMS allows arrays of any nonnegative integral
> size, no?
>
> Jeremy
>
> On Mon, Jun 15, 2009 at 12:20 AM, Tomas Hurka wrote:
>> Hi Martin and David,
>>
>> On 14 Jun 2009, at 21:37, Martin Buchholz wrote:
>>
>>> I've polished the changes in preparation for committing.
>>> I'll commit once I have reviewer approval.
>>
>> I have the same concern as David. I think that throwing OOME, in places
>> where there is obviously a simple programming error, is not a good idea.
>> Will it be possible to change it to throw IllegalArgumentException instead?
>> I have one more comment about the patch itself - changes to
>> src/share/vm/gc_interface/collectedHeap.inline.hpp and
>> src/share/vm/utilities/exceptions.hpp are unrelated to the bugfix. I think
>> it will be better to separate them to the different changeset.
>>
>> Bye,
>> --
>> Tomas Hurka   <mailto:tomas.hu...@sun.com>
>> NetBeans Profiler http://profiler.netbeans.org
>> VisualVM http://visualvm.dev.java.net
>> Software Engineer, Developer Platforms Group
>> Sun Microsystems, Praha Czech Republic
>>
>>
>

Re: JVMTI OOM handling when arrays / objects are too large

[Jeremy Manson]

I'm not sure if the heap dump is as interesting as the fact that you
are going to take some action when you do OnOutOfMemoryError.  It is
rather surprising to have specified that a particular action is taken
when a OOME occurs, and have it not happen when an OOME occurs.

I guess that because it is a limitation in the VM, you could throw
VirtualMachineError instead.

Jeremy

On Mon, Jun 15, 2009 at 2:37 AM, Alan Bateman wrote:
> David Holmes - Sun Microsystems wrote:
>>
>> Martin, Jeremy,
>>
>> This change has been bugging me. I guess what I don't like is not the
>> "fix" but the fact that we report OutOfMemoryError for this situation in the
>> first place! We are not out-of-memory! We've been asked to allocate
>> something that is impossible to allocate given the physical constraints of
>> the memory system. The heap could actually be nearly empty! If I were
>> executing a OOM handler using the "onError" mechanism I'm not sure I'd want
>> it to run for this case.
>>
>> This fix makes this usage consistent with all the other OOM situations,
>> but we post JVMTI resource exhausted events when the resource need not be
>> exhausted at all! I'm not sure that is the right thing to do. Even assuming
>> it is the right thing to do, this may impact some tests as it now generates
>> additional JVMTI events.
>>
>> David Holmes
>
> I'm glad you brought this up. I added the heap dump and also this
> (undocumented) OnOutOfMemoryError option some time ago. My memory on this is
> hazy but I think we decided to deliberately not to generate a heap dump or
> run the data collection actions for this case because it's not really
> resource related. The JVM TI event came later. I agree it is bit
> inconsistent but the exception message for this case ("Requested array size
> exceeds VM limit") does help to identity the problem.  The exception message
> and stack trace is lot more useful than a heap dump for this case.
>
> -Alan.
>

Re: JVMTI OOM handling when arrays / objects are too large

[Jeremy Manson]

Not that I actually have a vote, but if Alan feels it is not worth
making, I would go back to arguing for something like a
VirtualMachineError to be thrown, because the behavior is out of spec.
 The current behavior - a sort-of-but-not-really OOME - is a little
odd.

Jeremy

On Tue, Jun 16, 2009 at 1:57 PM, Martin Buchholz wrote:
> The resolution of this is still uncertain.
> We have one approval I think,
> but all of the maintainers have expressed reluctance at making this change.
> I continue to agree with Jeremy and be in favor of making this change,
> but it's no big deal either way.
> Perhaps Alan's opinion is authoritative, since he was the author.
> Alan, perhaps you should give us an actual VETO, and we will shut up.
>
> Martin
>
> On Mon, Jun 15, 2009 at 02:37, Alan Bateman  wrote:
>>
>> David Holmes - Sun Microsystems wrote:
>>>
>>> Martin, Jeremy,
>>>
>>> This change has been bugging me. I guess what I don't like is not the
>>> "fix" but the fact that we report OutOfMemoryError for this situation in the
>>> first place! We are not out-of-memory! We've been asked to allocate
>>> something that is impossible to allocate given the physical constraints of
>>> the memory system. The heap could actually be nearly empty! If I were
>>> executing a OOM handler using the "onError" mechanism I'm not sure I'd want
>>> it to run for this case.
>>>
>>> This fix makes this usage consistent with all the other OOM situations,
>>> but we post JVMTI resource exhausted events when the resource need not be
>>> exhausted at all! I'm not sure that is the right thing to do. Even assuming
>>> it is the right thing to do, this may impact some tests as it now generates
>>> additional JVMTI events.
>>>
>>> David Holmes
>>
>> I'm glad you brought this up. I added the heap dump and also this
>> (undocumented) OnOutOfMemoryError option some time ago. My memory on this is
>> hazy but I think we decided to deliberately not to generate a heap dump or
>> run the data collection actions for this case because it's not really
>> resource related. The JVM TI event came later. I agree it is bit
>> inconsistent but the exception message for this case ("Requested array size
>> exceeds VM limit") does help to identity the problem.  The exception message
>> and stack trace is lot more useful than a heap dump for this case.
>>
>> -Alan.
>
>

Re: JVMTI OOM handling when arrays / objects are too large

[Jeremy Manson]

On Wed, Jun 17, 2009 at 3:13 AM, Alan Bateman wrote:

> Hopefully this helps. I can review the patch but as I'm not working in this
> area on a daily basis, so it would be best to have a reviewer from the
> hotspot team (and I assume you'll need someone to push this through JPRT).

So, does anyone want to step up and review it?  I know several of you
have already looked at it.

Jeremy

Re: JVMTI OOM handling when arrays / objects are too large

[Jeremy Manson]

So, it should have the JVMTI_RESOURCE_EXHAUSTED_OOM_ERROR but not the
JVMTI_RESOURCE_EXHAUSTED_JAVA_HEAP?  I can change that.

Jeremy


On Tue, Jun 23, 2009 at 12:45 PM, Alan Bateman wrote:
> Jeremy Manson wrote:
>>
>> On Wed, Jun 17, 2009 at 3:13 AM, Alan Bateman wrote:
>>
>>
>>>
>>> Hopefully this helps. I can review the patch but as I'm not working in
>>> this
>>> area on a daily basis, so it would be best to have a reviewer from the
>>> hotspot team (and I assume you'll need someone to push this through
>>> JPRT).
>>>
>>
>> So, does anyone want to step up and review it?  I know several of you
>> have already looked at it.
>>
>> Jeremy
>>
>
> Is there is an updated webrev? In my last year I had hoped we wouldn't send
> the JVM TI ResourceExhausted event because this isn't really a resource
> exhaustion case.
>
> -Alan.
>