Re: JVMTI retransformation and addition of private methods

[Karen Kinnear]

Sergeui,

You were right - I read the sources incorrectly. Would still help to understand 
both
the motivation and the reason to not add to the spec.

Robert - do you remember why we didn’t add this to the specification? (6404550)

> On Feb 21, 2018, at 4:44 PM, serguei.spit...@oracle.com wrote:
> 
> Hi Karen,
> 
> Thank you for sorting this out!
> 
> 
> On 2/21/18 09:55, Karen Kinnear wrote:
>> Dan,
>> 
>> Thank you for all the background digging. This is really helpful.
>> 
>> Serguei - do you know what tests exist for this behavior?
> 
> Dan already replied (thanks, Dan!)
> There are two tests in the open/test/jdk/java/lang/instrument:
>   RedefineMethodAddInvoke*
>   RedefineMethodDelInvoke*
> 
> 
>> The way I read the source code - we currently allow ADD and DELETE for
>> PRIVATE OR STATIC OR FINAL methods. Did I read that correctly?
> The above does not look correct to me.
> We have the same check for both ADD and DELETE method:
>   if ((old_flags & JVM_ACC_PRIVATE) == 0
>// hack: private should be treated as final, but alas
>   || (old_flags & (JVM_ACC_FINAL|JVM_ACC_STATIC)) == 0
>  ) {
> // deleted methods must be private
> return JVMTI_ERROR_UNSUPPORTED_REDEFINITION_METHOD_DELETED;
>   }
> 
> I read it as we allow ADD and DELETE for PRIVATE && (STATIC || FINAL) methods.
> (Rephrase: We allow PRIVATE FINAL or PRIVATE STATIC methods.)
> As private should always be treated final then we can simplify the above to:
>   We allow and and delete PRIVATE INSTANCE or PRIVATE STATIC methods
>   which is equal to just "PRIVATE methods”.

Sergeui - you are right - I misread this today.
And I played with the tests a bit - thanks Dan - and it matches the way you 
read this.

So today, private methods that are not marked as final in the source code can 
not be added -
I tried that variation by modifying the RedefineMethodAddInvokeTarget_1.java 
and changing
private final void myMethod1() to private void myMethod1() and got an 
UnsupportedOperationException.

So - private methods are not marked as ACC_FINAL today so I think the 
simplification doesn’t
apply, so we are left with the ability to ADD or DELETE
  PRIVATE && (STATIC || FINAL) methods  - at least that is what we support 
today.
> 
>> With the current implementation, I am not sure if deletion works for private 
>> methods - do we
>> have a test for that? Or could you add one as part of this exercise?
> 
> Yes, we have one j.l.instrument test: RedefineMethodDelInvoke.sh.
I tried the test. And it works because of the requirement that FINAL or STATIC 
are set,
which therefore means no vtable entry.
> 
> 
>> Today we create a vtable entry for private methods (my misunderstanding ~ 
>> 2006ish). After discussions
>> with David I no longer believe we need those.
>> Today, klassVtable::adjust_method_entries has an assertion
>>   assert(!old_method->is_deleted(), “vtable methods may not be deleted”)
>> 
>> I may have read the code incorrectly - but I would expect to hit this 
>> assertion if you had a private
>> method you were deleting that was not final and not static.
>> 
>> option 1) we could explicitly tighten the restrictions to match what we have 
>> implemented
>> option 2) we could make this work by changing 
>> klassVtable.cpp::update_inherited_vtable
>>   handling of private fields to be done the way it handles final fields.
>> option 3) I read the code incorrectly?
> 
> If we create a vtable entry for private methods then we should hit the assert 
> above.
> If we no longer need this then we have no problem.
We do create a vtable entry for private methods; however if FINAL or STATIC is 
set, then
we do not create a vtable entry.

That is why we don’t ever get here.

thanks,
Karen
> 
> Thanks,
> Serguei
> 
>> thanks,
>> Karen
>> 
>>> On Feb 21, 2018, at 10:40 AM, Daniel D. Daugherty 
>>> <daniel.daughe...@oracle.com <mailto:daniel.daughe...@oracle.com>> wrote:
>>> 
>>> On 2/21/18 2:45 AM, serguei.spit...@oracle.com 
>>> <mailto:serguei.spit...@oracle.com> wrote:
>>>> On 2/20/18 23:01, David Holmes wrote: 
>>>>> On 21/02/2018 4:50 PM, serguei.spit...@oracle.com 
>>>>> <mailto:serguei.spit...@oracle.com> wrote: 
>>>>>> Hi Karen and David, 
>>>>>> 
>>>>>> 
>>>>>> On 2/20/18 19:52, David Holmes wrote: 
>>>>>>> Hi Karen, 
>>>>>>> 
>>>>>>> On 21/02/2018 1:54 AM, Karen Kinnear wrote: 
>>>>>>>> Folks, 
>>

Re: JVMTI retransformation and addition of private methods

[Karen Kinnear]

Dan,

Thank you for all the background digging. This is really helpful.

Serguei - do you know what tests exist for this behavior?

The way I read the source code - we currently allow ADD and DELETE for
PRIVATE OR STATIC OR FINAL methods. Did I read that correctly?

With the current implementation, I am not sure if deletion works for private 
methods - do we
have a test for that? Or could you add one as part of this exercise?

Today we create a vtable entry for private methods (my misunderstanding ~ 
2006ish). After discussions
with David I no longer believe we need those.
Today, klassVtable::adjust_method_entries has an assertion
  assert(!old_method->is_deleted(), “vtable methods may not be deleted”)

I may have read the code incorrectly - but I would expect to hit this assertion 
if you had a private
method you were deleting that was not final and not static.

option 1) we could explicitly tighten the restrictions to match what we have 
implemented
option 2) we could make this work by changing 
klassVtable.cpp::update_inherited_vtable
  handling of private fields to be done the way it handles final fields.
option 3) I read the code incorrectly?

thanks,
Karen

> On Feb 21, 2018, at 10:40 AM, Daniel D. Daugherty 
> <daniel.daughe...@oracle.com> wrote:
> 
> On 2/21/18 2:45 AM, serguei.spit...@oracle.com 
> <mailto:serguei.spit...@oracle.com> wrote:
>> On 2/20/18 23:01, David Holmes wrote: 
>>> On 21/02/2018 4:50 PM, serguei.spit...@oracle.com 
>>> <mailto:serguei.spit...@oracle.com> wrote: 
>>>> Hi Karen and David, 
>>>> 
>>>> 
>>>> On 2/20/18 19:52, David Holmes wrote: 
>>>>> Hi Karen, 
>>>>> 
>>>>> On 21/02/2018 1:54 AM, Karen Kinnear wrote: 
>>>>>> Folks, 
>>>>>> 
>>>>>> As part of the Valhalla EG discussions for JVMTI changes for nestmates 
>>>>>> (thank you Serguei and David), 
>>>>>> IBM brought up a request that we update the JVMTI documentation to 
>>>>>> reflect that we allow addition 
>>>>>> of private methods. 
>>>>>> 
>>>>>> Is there a reason we do not document this? I’m inviting those who were 
>>>>>> involved at the time - please include 
>>>>>> others if needed. 
>>>> 
>>>> I support documenting this in the JVMTI spec and had a plan to fix it in 
>>>> 11. 
>>>> However, it is not clear to me yet if we have a consensus on it. 
>>> 
>>> I would like to see a detailed analysis of the implications of allowing 
>>> this. I _think_ it is safe but ... 
>> 
>> Valid concern. 
>> Also, I'd love to collect more details on the initial motivation to relax 
>> the JVMTI spec. 
>> Most likely we had no CCC/CSR filed on this change. 
>> 
>> 
>>>>> This issue is tracked by: 
>>>>> 
>>>>> https://bugs.openjdk.java.net/browse/JDK-8192936 
>>>>> <https://bugs.openjdk.java.net/browse/JDK-8192936> 
>>>>> 
>>>>> "RI does not follow the JVMTI RedefineClasses spec that is too strict in 
>>>>> the definition" 
>>>> 
>>>> Yes, this is the one. 
>>>> Thank you, David, for posting the link. 
>>>> 
>>>> 
>>>>> As I wrote there ... It is not at all clear how JDK-6404550 morphed into 
>>>>> "Permit the adding or deleting of private final/static methods with 
>>>>> redefine" - nor why those changes failed to make any change to the spec 
>>>>> itself. It is also unclear whether the add/delete is restricted to 
>>>>> final/static methods or any private method? I can see that the intent was 
>>>>> to only allow something that would not perturb the vtable for existing 
>>>>> instances. 
>>>> 
>>>> I agree, there is a confusion somewhere. 
>>>> Is it possible, the JDK-6404550 in JIRA is a different bug than the one in 
>>>> the Bugtraq system? 
>>>> 
>>>> The JDK-6404550 in JIRA has a different synopsis: 
>>>> https://bugs.openjdk.java.net/browse/JDK-6404550 
>>>> <https://bugs.openjdk.java.net/browse/JDK-6404550> 
>>>>   Cannot implement late attach in NetBeans Profiler due to missing 
>>>> functionality in JVMTI 
>>> 
>>> Digging deeper ... to fix the problem described in that bug they augmented 
>>> JVM TI to allow private method redefinition as an alternate to the "native 
>>> rebinding"

JVMTI retransformation and addition of private methods

[Karen Kinnear]

Folks,

As part of the Valhalla EG discussions for JVMTI changes for nestmates (thank 
you Serguei and David),
IBM brought up a request that we update the JVMTI documentation to reflect that 
we allow addition
of private methods.

Is there a reason we do not document this? I’m inviting those who were involved 
at the time - please include
others if needed.

thanks,
Karen

Re: Unified JVM Logging and diagnostic options LogVMOutput, LogFile, DisplayVMOutput

[Karen Kinnear]

Just a note - unified logging belongs to runtime.

thanks,
Karen

> On Dec 6, 2017, at 1:33 AM, David Holmes  wrote:
> 
> On 1/12/2017 11:15 AM, Man Cao wrote:
>> Thanks David for the quick response!
>> Yes, I understand completely removing defaultStream and tty is probably 
>> infeasible.
>> If deprecating the diagnostic options (LogVMOutput, LogFile, 
>> DisplayVMOutput) is the intent, could someone create a bug/RFE to track it? 
>> They should probably first be added to the special_jvm_flags table 
> 
> I think there is too much yet to be done in the conversion process to file 
> such a RFE - to be frank we don't have active RFE's for the remaining 
> conversions, rather they are being tackled case by case when people are 
> working in the area.
> 
>> in arguments.cpp, so there will be a warning when people use them. The case 
>> of conflicting -Xlog::file=test.log and -XX:LogFile=test.log is also a 
>> concern, the JVM should at least issue a warning about it. In addition, it 
>> would make it easier for us to convince production teams to stop using these 
>> options.
> 
> I have filed:
> 
> https://bugs.openjdk.java.net/browse/JDK-8193117
> 
> JDK-8193117 Issue a warning if legacy logging and Unified Logging are told to 
> use the same file
> 
> Cheers,
> David
> -
> 
>> Thanks,
>> Man
>> On Thu, Nov 30, 2017 at 4:48 PM, David Holmes > > wrote:
>>Hi Man,
>>Adding serviceability as UL is a serviceability feature.
>>On 1/12/2017 10:23 AM, Man Cao wrote:
>>Hello,
>>We (Java Platform Team at Google) found that in JDK9, the file
>>generated by
>>-XX:+LogVMOutput no longer contains GC log messages, because the
>>unified
>>JVM logging framework does not use the defaultStream and
>>fileStream classes
>>and the tty variable. Similarly, related options such as LogFile,
>>DisplayVMOutput have no effect on the messages printed by the
>>unified
>>logging framework.
>>The main problem for us is that most of our production Java
>>servers use the
>>options "-XX:+LogVMOutput -XX:-DisplayVMOutput", to save the GC
>>logs and
>>other VM messages. These flags would no longer work for JDK9's
>>JVM logging
>>framework. In addition, the file generated by -XX:+LogVMOutput
>>may contain
>>information that is not printed by the unified logging framework.
>>What is worse is that if LogFile and the output of unified logging
>>framework point to the same file, the file may contain partial
>>or corrupted
>>messages from the unified logging framework. For example,
>>consider the
>>following command:
>>java -Xlog::file=test.log -XX:+UnlockDiagnosticVMOptions
>>-XX:+LogVMOutput
>>-XX:LogFile=test.log
>>Then test.log would miss some of the messages that would be
>>printed at the
>>beginning if you run "java -Xlog".
>>So our questions are:
>>1. Do you consider this is a bug for the unified logging
>>framework that
>>should be fixed? Should the unified logging framework respect these
>>diagnostic options?
>>My understanding is that UL is intended to replace the legacy
>>"logging" flags and works independently of them. So no, UL should
>>not respect these flags.
>>2. Is there a plan to deprecate these diagnostic options
>>(LogVMOutput,
>>LogFile, DisplayVMOutput, etc.)? Will the defaultStream,
>>fileStream classes
>>and the tty variable eventually removed from HotSpot?
>>I believe that is the general intent - though complete removal is
>>not feasible as UL can't always be used in all contexts. We have
>>migrated various subsystems to UL and all new logging should be
>>using UL. However I don't believe we actually have active RFEs to
>>aggressively complete the switchover.
>>Just my 2c.
>>David
>>Thanks,
>>Man

Re: RFR: 8141341: CDS should be disabled if JvmtiExport::should_post_class_file_load_hook() is true

[Karen Kinnear]

Jiangli,

Thank you so much for picking up this bug fix, I very much appreciate that.

I agree with not quitting the vm on agent attach.
I actually think we also want the agent attach to succeed - see below.
> On Jul 7, 2016, at 9:56 PM, Ioi Lam  wrote:
> 
> (Adding serviceability-dev@openjdk.java.net)
> 
> Hi Jiangli,
> 
> I think the two blocks of
> 
>if (RequireSharedSpaces) {
>  tty->print_cr("An error has occurred while loading shared class.");
>  tty->print_cr("Tool agent requires sharing to be disabled.");
>  vm_exit(1);
>}
> 
> should be removed.
I agree with this part. I was expecting the else to return a null 
instanceklasshandle as if the shared class was not found,
but no forcing of using the shared spaces if there is an agent.
> 
> If the agent is specified in the command line, the JVM start would be 
> aborted. This is already handled by your changes to 
> src/share/vm/memory/metaspace.cpp
Yes, here it makes sense to do the fail_continue such that RequireSharedSpaces 
will prevent starting the JVM.
> 
> If the agent is attached later, after the VM has started, I think we should 
> not quit the VM.
Totally agree - we should not quit the VM.

> Consider this scenario -- a production server could be running for a long 
> time without issues using -Xshare:on, then when the user attaches a 
> diagnostic agent the JVM suddenly quits.  I think it's better to cause the 
> agent attachment to fail with something like
> 
> jvmtiError
> JvmtiEnv::SetEventNotificationMode(jvmtiEventMode mode, jvmtiEvent 
> event_type, jthread event_thread,   ...) {
> 
> ...
>  if (event_type == JVMTI_EVENT_CLASS_FILE_LOAD_HOOK && enabled) {
> +   if (RequireSharedSpaces && !universe::is_bootstraping()) {
> + tty->print_cr("Tool agent that requires 
> JVMTI_EVENT_CLASS_FILE_LOAD_HOOK cannot be dynamically loaded after JVM has 
> started");
> + return JVMTI_ERROR_ILLEGAL_ARGUMENT;
> +   }
>record_class_file_load_hook_enabled();
>  }
> …
> }
> 
I was expecting the vm to continue running, letting the agent attach (customers 
really want to use these).
Jiangli - if I read your code correctly, that is what you do if UseSharedSpaces 
but not if RequireSharedSpaces.
I would propose that you do the same behavior for both cases once we are up and 
running - i.e. get rid of the
two blocks Ioi suggested removing and just fall through as if the file was not 
found in the archive regardless of RequireSharedSpaces. 
> 
> Thanks
> - Ioi
Jiangli: 

Minor code review comments:

metaspace.cpp line 3181: CFHL-> CFLH

metaspace.cpp: 
   I appreciated your moving the if (UseSharedSpaces) into the #INCLUDE_CDS.
   I think it would make sense to do a bit more of that - e.g. cds_address 
appears to only be
   use locally, so it also could be inside the #if INCLUDE_CDS. 
   Would it make sense to have the entire if (DumpSharedSpaces) etc. all be 
inside a single #if INCLUDE_CDS?

thanks,
Karen

> 
> 
> 
> On 7/7/16 6:08 PM, Jiangli Zhou wrote:
>> Please review the following webrev that disables CDS when 
>> JvmtiExport::should_post_class_file_load_hook() is enabled at runtime.
>> 
>>   webrev: http://cr.openjdk.java.net/~jiangli/8141341/webrev.00/
>>   bug: JDK-8141341 
>> 
>> With -Xshare:on
>> If -Xshare:on is used and JvmtiExport::should_post_class_file_load_hook() is 
>> required, the VM now reports "Tool agent requires sharing to be disabled” 
>> and terminates. This is the same behavior as jdk8.
>> 
>> With -Xshare:auto
>> The change in meatspace.cpp is to detect the case where 
>> JvmtiExport::should_post_class_file_load_hook() is enabled very early during 
>> JVM initialization. In that case, we disable CDS entirely, including all 
>> shared classes, symbols, and Strings objects.
>> 
>> The change in systemDictionary.cpp is to detect the cases where 
>> JvmtiExport::should_post_class_file_load_hook() is enabled late, which 
>> include agent dynamically attached at runtime. In those cases, JVM does not 
>> allow loading classes from the shared archive. The shared symbols and 
>> Strings can still be used.
>> 
>> Thanks,
>> Jiangli
>> 
>> 
> 

Re: CFV: New Serviceability Group Lead: Staffan Larsen

[Karen Kinnear]

Vote: yes
> On Jul 7, 2016, at 7:48 PM, Daniel D. Daugherty  
> wrote:
> 
> I hereby nominate Staffan Larsen to Serviceability Group Lead [1].
> 
> Staffan is a Reviewer in the jdk9 and jdk8u Projects, a member of the
> Serviceability team, and has so far committed > 145 changesets. He is the
> Serviceability architect since 2011, has been working on HotSpot for
> more than five years. Before joining the HotSpot team he worked with
> the JRockit JVM from 1999.
> 
> Votes are due by July 25, 2016 18:00 PDT.
> 
> Only current Members of the Serviceability Group [2] are eligible
> to vote on this nomination.  Votes must be cast in the open by
> replying to this mailing list.
> 
> For Simple Majority voting instructions, see [3].
> 
> Thanks,
> Daniel Daugherty
> 
> [1]: http://openjdk.java.net/bylaws#group-lead
> [2]: http://openjdk.java.net/census#serviceability
> [3]: http://openjdk.java.net/groups#lead-vote

Re: RFR(XXS): 8149803: Adjust lock rankings for some Event-based tracing locks

[Karen Kinnear]

Markus,

It would be helpful if you could find a place to record a comment about the 
JfrStream_lock - that the rank needs to
be between safe point and JfrBuffer_lock. That way, should we do cleanup in 
this area we will know the constraints.

thanks,
Karen

> On Feb 24, 2016, at 2:30 PM, Karen Kinnear <karen.kinn...@oracle.com> wrote:
> 
> Markus,
> 
> See below.
>> On Feb 23, 2016, at 11:58 PM, David Holmes <david.hol...@oracle.com> wrote:
>> 
>> Hi Markus,
>> 
>> On 23/02/2016 8:00 PM, Markus Gronlund wrote:
>>> Thanks for taking a look David,
>>> 
>>> I have verified the new lock rankings by running Kitchensink and by code 
>>> inspection.
>>> 
>>> The original intent was to try to make the tracing locks transparent much 
>>> like the way ttyLocker is today ("event" ranking). I did this in an effort 
>>> to provide "seamless" tracing usages from higher ranking locks (leafs in 
>>> particular). As you point out, the mechanics for accomplishing this becomes 
>>> a bit of a force-fit in light of the existing lock_types enum. I still 
>>> needed lock ordering since ttyLocker is being used under some of these 
>>> locks; in addition, there is a relative order between two of these locks 
>>> (that was the reason for special+1 - and yes it lands on the same ordinal 
>>> as suspend_resume (but I didn't want to use that designation since it is 
>>> totally separate)).
>>> 
>>> I also elaborated on expanding the lock_types enum:
>>> 
>>>   enum lock_types {
>>>event,
>>>special,
>>>suspend_resume,
>>>traceleaf,
>>>trace,
>>>leaf,
>>>safepoint   = leaf   +  10,
>>>barrier = safepoint  +   1,
>>>nonleaf = barrier+   1,
>>>max_nonleaf = nonleaf+ 900,
>>>native  = max_nonleaf+   1
>>>   };
>>> 
>>> This seems also to work but I am not sure about any disruptions this might 
>>> cause. Would mean much more investigation to do any changes here.
>> 
>> I don't think this should cause any disruptions unless we have hard-coded 
>> the enum values somewhere - which we do not seem to do. It also avoids the 
>> potential problem of having a lock with rank special+1 being considered to 
>> have rank suspend_resume in the rank checking code:
>> 
>>   if (this->rank() != Mutex::native &&
>>   this->rank() != Mutex::suspend_resume &&
>>   locks != NULL && locks->rank() <= this->rank() &&
>>   ...
>> 
>>> On the topic of the "special" rank comment mentioning guarantees not to 
>>> block, that would be true for JfrBuffer_lock, but not for JfrStream_lock 
>>> (could be blocked on the former). So this is not correct - thanks for 
>>> pointing this out.
>>> 
>>> I am having a rethink about this:
>>> 
>>> Reducing the lock rankings underneath the ordinary leaf ranking would be 
>>> for facility yes. However, doing so might cause an unwanted effect:
>>> 
>>> It would become easier to generated trace events while holding these other 
>>> locks. A better pattern for overall performance would be to instead save 
>>> the necessary information, release the lock as soon as possible, and 
>>> generate an event post-lock release (if possible). I have not yet seen a 
>>> real case where this is not possible to do - I might need to revisit this 
>>> if that becomes a real problem in the future.
>>> 
>>> Even though it might not be the primary vehicle for enforcement, we could 
>>> capitalize on the lock ordering scheme to avoid generating events 
>>> underneath other locks unnecessarily.
>>> 
>>> So I am retracting the lock ranking "special" suggestion.
>>> 
>>> What I actually only want/need is this:
>>> 
>>> -  def(JfrStream_lock   , Mutex,   nonleaf, true,
>>> Monitor::_safepoint_check_never);
>>> +  def(JfrStream_lock   , Mutex,   leaf+1,   true,
>>> Monitor::_safepoint_check_never);
>>> 
>>> The lock ordering assertions will today handle nonleaf for this lock, but 
>>> only because of the assertion bailing on 
>>> SafepointSynchronize::is_at_safepoint(). I would like to reduce the ranking 
>>> from nonleaf to leaf+1 to gracefully handle 
>>> Safepoi

Re: RFR(XXS): 8149803: Adjust lock rankings for some Event-based tracing locks

[Karen Kinnear]

Markus,

See below.
> On Feb 23, 2016, at 11:58 PM, David Holmes  wrote:
> 
> Hi Markus,
> 
> On 23/02/2016 8:00 PM, Markus Gronlund wrote:
>> Thanks for taking a look David,
>> 
>> I have verified the new lock rankings by running Kitchensink and by code 
>> inspection.
>> 
>> The original intent was to try to make the tracing locks transparent much 
>> like the way ttyLocker is today ("event" ranking). I did this in an effort 
>> to provide "seamless" tracing usages from higher ranking locks (leafs in 
>> particular). As you point out, the mechanics for accomplishing this becomes 
>> a bit of a force-fit in light of the existing lock_types enum. I still 
>> needed lock ordering since ttyLocker is being used under some of these 
>> locks; in addition, there is a relative order between two of these locks 
>> (that was the reason for special+1 - and yes it lands on the same ordinal as 
>> suspend_resume (but I didn't want to use that designation since it is 
>> totally separate)).
>> 
>> I also elaborated on expanding the lock_types enum:
>> 
>>enum lock_types {
>> event,
>> special,
>> suspend_resume,
>> traceleaf,
>> trace,
>> leaf,
>> safepoint   = leaf   +  10,
>> barrier = safepoint  +   1,
>> nonleaf = barrier+   1,
>> max_nonleaf = nonleaf+ 900,
>> native  = max_nonleaf+   1
>>};
>> 
>> This seems also to work but I am not sure about any disruptions this might 
>> cause. Would mean much more investigation to do any changes here.
> 
> I don't think this should cause any disruptions unless we have hard-coded the 
> enum values somewhere - which we do not seem to do. It also avoids the 
> potential problem of having a lock with rank special+1 being considered to 
> have rank suspend_resume in the rank checking code:
> 
>if (this->rank() != Mutex::native &&
>this->rank() != Mutex::suspend_resume &&
>locks != NULL && locks->rank() <= this->rank() &&
>...
> 
>> On the topic of the "special" rank comment mentioning guarantees not to 
>> block, that would be true for JfrBuffer_lock, but not for JfrStream_lock 
>> (could be blocked on the former). So this is not correct - thanks for 
>> pointing this out.
>> 
>> I am having a rethink about this:
>> 
>> Reducing the lock rankings underneath the ordinary leaf ranking would be for 
>> facility yes. However, doing so might cause an unwanted effect:
>> 
>> It would become easier to generated trace events while holding these other 
>> locks. A better pattern for overall performance would be to instead save the 
>> necessary information, release the lock as soon as possible, and generate an 
>> event post-lock release (if possible). I have not yet seen a real case where 
>> this is not possible to do - I might need to revisit this if that becomes a 
>> real problem in the future.
>> 
>> Even though it might not be the primary vehicle for enforcement, we could 
>> capitalize on the lock ordering scheme to avoid generating events underneath 
>> other locks unnecessarily.
>> 
>> So I am retracting the lock ranking "special" suggestion.
>> 
>> What I actually only want/need is this:
>> 
>> -  def(JfrStream_lock   , Mutex,   nonleaf, true,
>> Monitor::_safepoint_check_never);
>> +  def(JfrStream_lock   , Mutex,   leaf+1,   true,
>> Monitor::_safepoint_check_never);
>> 
>> The lock ordering assertions will today handle nonleaf for this lock, but 
>> only because of the assertion bailing on 
>> SafepointSynchronize::is_at_safepoint(). I would like to reduce the ranking 
>> from nonleaf to leaf+1 to gracefully handle 
>> SafepointSynchronize::is_synchronizing() and other states as well.
> 
> I didn't quite follow all that. :) If this simpler suggestion meets your 
> requirements and adheres to the intent of the lock-ranking protocol then that 
> is good.
I think what you are saying is that actually you do not adhere to the 
lock-ranking protocol. Rather that, when not at a safe point you adhere to the
lock ranking protocol, but when at a safe point, the safe point check 
(is_a_safepoint)  skips the lock ranking check.
I think the rest of your comment is that with the new tracepoints in the safe 
point code, which need this lock while holding the Safepoint_lock (i.e. while 
getting
to a safepoint or leaving a safe point but not during a safe point), so you 
need to ensure that both the JfrBuffer_lock and JfrStream_lock can be acquired
while not actually at a safe point, and while doing a lock ordering check, 
while holding the Safepoint_lock. And you are maintaining the relative
relationship of JfrBuffer_lock with JfrStream_lock.

So you are just changing JfrStream_lock from non leaf to leaf+1.

If that is what you are saying, I am good with the change. Ship it once 
sufficiently tested.

thanks,
Karen

> 
> Thanks,
> David
> -
> 
>> Thanks for your 

Re: RFR (S) 8049304: race between VM_Exit and _sync_FutileWakeups->inc()

[Karen Kinnear]

Dan,

Can you possibly change the two "in a parallel" to "in parallel" ?

thanks,
Karen

On Sep 2, 2015, at 1:06 PM, Tom Benson wrote:

> Looks good to me!
> Tnx,
> Tom
> 
> On 9/2/2015 12:40 PM, Daniel D. Daugherty wrote:
>> Just for the record, here are the comment context diffs:
>> 
>> $ diff -c src/share/vm/runtime/perfMemory.cpp{.cr2,}*** 
>> src/share/vm/runtime/perfMemory.cpp.cr2 Tue Sep  1 19:39:45 2015
>> --- src/share/vm/runtime/perfMemory.cpp Wed Sep  2 09:35:48 2015
>> ***
>> *** 70,76 
>>// objects that are currently being used by running JavaThreads
>>// or the StatSampler. This method is invoked while we are not at
>>// a safepoint during a VM abort so leaving the PerfData objects
>> !   // around may also help diagnose the failure.
>>//
>>if (SafepointSynchronize::is_at_safepoint() && !StatSampler::is_active()) 
>> {
>>  PerfDataManager::destroy();
>> --- 70,78 
>>// objects that are currently being used by running JavaThreads
>>// or the StatSampler. This method is invoked while we are not at
>>// a safepoint during a VM abort so leaving the PerfData objects
>> !   // around may also help diagnose the failure. In rare cases,
>> !   // PerfData objects are used in parallel with a safepoint. See
>> !   // the work around in PerfDataManager::destroy().
>>//
>>if (SafepointSynchronize::is_at_safepoint() && !StatSampler::is_active()) 
>> {
>>  PerfDataManager::destroy();
>> 
>> 
>> $ diff -c src/share/vm/runtime/objectMonitor.cpp{.cr2,}
>> *** src/share/vm/runtime/objectMonitor.cpp.cr2  Tue Sep  1 19:23:35 2015
>> --- src/share/vm/runtime/objectMonitor.cpp  Wed Sep  2 09:37:08 2015
>> ***
>> *** 572,577 
>> --- 572,579 
>>  // That is by design - we trade "lossy" counters which are exposed to
>>  // races during updates for a lower probe effect.
>>  TEVENT(Inflated enter - Futile wakeup);
>> + // This PerfData object can be used in a parallel with a safepoint.
>> + // See the work around in PerfDataManager::destroy().
>>  OM_PERFDATA_OP(FutileWakeups, inc());
>>  ++nWakeups;
>> 
>> ***
>> *** 744,749 
>> --- 746,753 
>>  // *must* retry  _owner before parking.
>>  OrderAccess::fence();
>> 
>> + // This PerfData object can be used in a parallel with a safepoint.
>> + // See the work around in PerfDataManager::destroy().
>>  OM_PERFDATA_OP(FutileWakeups, inc());
>>}
>> 
>> 
>> Dan
>> 
>> 
>> On 9/2/15 10:03 AM, Daniel D. Daugherty wrote:
>>> On 9/2/15 9:40 AM, Tom Benson wrote:
 Hi Dan,
 OK.  I didn't review what was added in round 1 once you said it was all 
 removed for round 2.  8^)
>>> 
>>> Not "all", but I did remove "most" of the round 1 changes :-)
>>> The changes I kept are called in the list below.
>>> 
>>> 
 It would be great if what you have in your first paragraph below was added 
 to the comments.   I think the existing comment in perfMemory_exit implies 
 we're safe to remove the PerfData objects without fear of them being in 
 use because we're at a safepoint.
>>> 
>>> I think I'll add this sentence to the comment in perfMemory_exit():
>>> 
>>>// In rare cases, PerfData objects are used in parallel with a
>>>// safepoint. See the work around in PerfDataManager::destroy().
>>> 
>>> 
 Perhaps better to have it (the new comment) in PerfDataManager::destroy(), 
  because it seems so weird to have a sleep in the VM thread during a 
 safepoint, even in a shutdown path.
>>> 
>>> I think the PerfDataManager::destroy() comment is clear about
>>> the race we're trying avoid. Again, if you have specific wording
>>> changes to suggest to make it more clear... I'll take them. :-)
>>> 
>>> I think I'll also add this comment:
>>> 
>>>// This PerfData object can be used in a parallel with a safepoint.
>>>// See the work around in PerfDataManager::destroy().
>>> 
>>> above these lines in src/share/vm/runtime/objectMonitor.cpp:
>>> 
>>> 575 OM_PERFDATA_OP(FutileWakeups, inc());
>>> 747 OM_PERFDATA_OP(FutileWakeups, inc());
>>> 
>>> 
 Any interest in asserting that you're at a safepoint in 
 PerfDataManager::destroy?   Just a thought.
>>> 
>>> I'd rather not add an assert() at this time.
>>> 
>>> Are you good with the above comment additions? Do you need to
>>> see another webrev when I make those changes?
>>> 
>>> Dan
>>> 
>>> 
 Tom
 
 On 9/2/2015 11:15 AM, Daniel D. Daugherty wrote:
> On 9/2/15 8:49 AM, Tom Benson wrote:
>> Hi,
>> I'm a bit confused on one point... Since you now only call 
>> PerfDataManager::destroy at a safepoint, why do you still have the 
>> comment about 'the race'  and the sleep?
> 
> Because the two "futile wakeup" counter updates in the monitor
> subsystem can execute in parallel with a safepoint. The JavaThread
> state is "blocked" so the safepoint subsystem will see the JavaThread
> 

Re: RFR (S) 8049304: race between VM_Exit and _sync_FutileWakeups->inc()

[Karen Kinnear]

Dan,

I did not do a complete review - I have been following the conversation and you 
all seem to have this completely
in hand so I didn't jump in. Yell if need more.

thanks,
Karen

On Sep 2, 2015, at 3:27 PM, Daniel D. Daugherty wrote:

> On 9/2/15 12:50 PM, Karen Kinnear wrote:
>> Dan,
>> 
>> Can you possibly change the two "in a parallel" to "in parallel" ?
> 
> Nice catch. Will fix.
> 
> Did you do a complete review or did that glaring typo just
> jump out at you?
> 
> Dan
> 
> 
>> 
>> thanks,
>> Karen
>> 
>> On Sep 2, 2015, at 1:06 PM, Tom Benson wrote:
>> 
>>> Looks good to me!
>>> Tnx,
>>> Tom
>>> 
>>> On 9/2/2015 12:40 PM, Daniel D. Daugherty wrote:
>>>> Just for the record, here are the comment context diffs:
>>>> 
>>>> $ diff -c src/share/vm/runtime/perfMemory.cpp{.cr2,}*** 
>>>> src/share/vm/runtime/perfMemory.cpp.cr2 Tue Sep  1 19:39:45 2015
>>>> --- src/share/vm/runtime/perfMemory.cpp Wed Sep  2 09:35:48 2015
>>>> ***
>>>> *** 70,76 
>>>>// objects that are currently being used by running JavaThreads
>>>>// or the StatSampler. This method is invoked while we are not at
>>>>// a safepoint during a VM abort so leaving the PerfData objects
>>>> !   // around may also help diagnose the failure.
>>>>//
>>>>if (SafepointSynchronize::is_at_safepoint() && 
>>>> !StatSampler::is_active()) {
>>>>  PerfDataManager::destroy();
>>>> --- 70,78 
>>>>// objects that are currently being used by running JavaThreads
>>>>// or the StatSampler. This method is invoked while we are not at
>>>>// a safepoint during a VM abort so leaving the PerfData objects
>>>> !   // around may also help diagnose the failure. In rare cases,
>>>> !   // PerfData objects are used in parallel with a safepoint. See
>>>> !   // the work around in PerfDataManager::destroy().
>>>>//
>>>>if (SafepointSynchronize::is_at_safepoint() && 
>>>> !StatSampler::is_active()) {
>>>>  PerfDataManager::destroy();
>>>> 
>>>> 
>>>> $ diff -c src/share/vm/runtime/objectMonitor.cpp{.cr2,}
>>>> *** src/share/vm/runtime/objectMonitor.cpp.cr2  Tue Sep  1 19:23:35 2015
>>>> --- src/share/vm/runtime/objectMonitor.cpp  Wed Sep  2 09:37:08 2015
>>>> ***
>>>> *** 572,577 
>>>> --- 572,579 
>>>>  // That is by design - we trade "lossy" counters which are exposed to
>>>>  // races during updates for a lower probe effect.
>>>>  TEVENT(Inflated enter - Futile wakeup);
>>>> + // This PerfData object can be used in a parallel with a safepoint.
>>>> + // See the work around in PerfDataManager::destroy().
>>>>  OM_PERFDATA_OP(FutileWakeups, inc());
>>>>  ++nWakeups;
>>>> 
>>>> ***
>>>> *** 744,749 
>>>> --- 746,753 
>>>>  // *must* retry  _owner before parking.
>>>>  OrderAccess::fence();
>>>> 
>>>> + // This PerfData object can be used in a parallel with a safepoint.
>>>> + // See the work around in PerfDataManager::destroy().
>>>>  OM_PERFDATA_OP(FutileWakeups, inc());
>>>>}
>>>> 
>>>> 
>>>> Dan
>>>> 
>>>> 
>>>> On 9/2/15 10:03 AM, Daniel D. Daugherty wrote:
>>>>> On 9/2/15 9:40 AM, Tom Benson wrote:
>>>>>> Hi Dan,
>>>>>> OK.  I didn't review what was added in round 1 once you said it was all 
>>>>>> removed for round 2.  8^)
>>>>> Not "all", but I did remove "most" of the round 1 changes :-)
>>>>> The changes I kept are called in the list below.
>>>>> 
>>>>> 
>>>>>> It would be great if what you have in your first paragraph below was 
>>>>>> added to the comments.   I think the existing comment in perfMemory_exit 
>>>>>> implies we're safe to remove the PerfData objects without fear of them 
>>>>>> being in use because we're at a safepoint.
>>>>> I think I'll add this sentence to the comment in perfMemory_exit():
>>>>> 
>>>>>// In rare cases, PerfData objects are used in parall

Re: Low-Overhead Heap Profiling

[Karen Kinnear]

Thanks Jeremy - that helps -  not such a big deal in terms of maintaining - so 
back to the bigger discussion.

thanks,
Karen

On Jun 26, 2015, at 1:34 AM, Jeremy Manson wrote:

 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 karen.kinn...@oracle.com 
 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 tprinte...@twitter.com 
 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 (orthogonal to the main point, but I thought I’d 
 mention it anyway

Re: Low-Overhead Heap Profiling

[Karen Kinnear]

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 tprinte...@twitter.com 
 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 (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.
 
 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).
 
 
 
 (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 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?
 
 
 I was not concerned with the read and subtraction, I was more concerned with 
 the conditional that follows them (intel has great branch prediction).
 
 And a personal pet peeve (based on past experience): How many “free” 
 instructions do you have to add before they are not free any more?
 
 
 
 
 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 
 
 
 The allocating Thread is passed to the slow (outside-TLAB) alloc path so it 
 would be trivial to update the per-thread allocation stats from there too (in 
 fact, it does; see below).
 
 
 
 (and hypothetically disabled TLABs).
 
 
 Anyone cares? :-)
 
 
 
 
 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.
 
 
 Thanks for the tip. I’ll look into this...
 
 and no one has complained.
 
 I'm not worried about the ease of implementation here, because we've 
 

Re: [9] RFR (M) 8054888: Runtime: Add Diagnostic Command that prints the class hierarchy

[Karen Kinnear]

Chris,

Just saw this. I was thinking the instance in the heap since you might have 
multiple instances of a single subclass.
Good point that it might change and so the correlation would be gone.

thanks,
Karen

On Feb 6, 2015, at 5:09 PM, Chris Plummer wrote:

 Hi Karen,
 
 Can you clarify what you mean by address of the j.l.ClassLoader class. Is 
 that the Klass* for the ClassLoader subclass, or the actual address of the 
 ClassLoader instance in the heap, which can change.
 
 Chris
 
 On 2/6/15 1:21 PM, Karen Kinnear wrote:
 Chris,
 
 So I was curious - I was thinking you would printing the hex address of the 
 j.l.ClassLoader class rather than the cld so that if folks were to look
 at a heap dump later it might be meaningful to them. Is it unlikely that 
 they would ever want to correlate those?
 
 
 On Feb 6, 2015, at 3:15 PM, Chris Plummer wrote:
 
 I was also thinking it might be a good idea to indicate what the hex value 
 is, although still have figured out the best way of doing 
 this. Maybe just a simple comment before the output. Keep in mind that 
 eventually other DCMDS will also include the cld to help uniquiely identify 
 classes across dcmd output. Also keep in mind your earlier suggestion:
 
 java.lang.Object/0x12345600
 |--java.io.Serializable/0x12345601
 |--java.util.RandomAccess/0x12345602
 |--java.lang.Iterable/0x12345603
 | |--java.util.Collection/0x12345604
 | | |--java.util.List/0x12345605
 |--java.util.AbstractCollection/0x12345606
 | |  implements java.util.Collection/0x12345604
 | |  implements java.lang.Iterable/0x12345603
 | |--java.util.AbstractList/0x12345607
 | |implements java.util.List/0x12345605
 | | |--java.util.Arrays$ArrayList/0x12345608
 | | |implements java.io.Serializable/0x12345601
 | | |implements java.util.RandomAccess/0x12345602
 
 With additions to GC.class_stats:
 
 Index Super ClassLoaderClassName
 1-1 0x0007c0034c48 java.lang.Object/0x12345600
 2 1 0x0007c0034c48 java.util.List/0x12345605
 331 0x0007c0034c48 java.util.AbstractList/0x12345607
 
 And GC.class_histogram:
 
  num #instances #bytes  class name
 --
1:   945 117736  java.lang.Object/0x12345600
2:   442  50352  java.util.List/0x12345605
3:   499  25288  java.util.AbstractList/0x12345607
 
 I think in your case you assumed we would create a unique identifier for 
 each class, but then we settled on just classname + ClassLoader identifier 
 of some sort, and the CLD* works for that. So replace your hex class ID in 
 the above with the hex CLD*.
 
 Also something Karen had said is just now starting to sink in and make 
 sense to me:
 
 |-sun.misc.Launcher$AppClassLoader/null (0xyyy)
 |-myapp/0xyyy
 
 I think the idea here is that when printing a ClassLoader subclass, you 
 include two CLDs. The first is for the ClassLoader that loaded the class, 
 and the second is the CLD for the ClassLoader subclass itself. Thus the 
 above indicates that myapp was loaded by 0xyyy, which is the 
 sun.misc.Launcher$AppClassLoader, which was loaded by the null ClassLoader. 
 I apologize - I don't remember what I said. But I am not sure we need that 
 level of complexity. I think I was asking that one of the dcmds that gives
 information could print information like 0xabc a 
 sun.misc.Launcher$AppClassLoader or a WLS.blah.GenericClassLoader if we 
 don't want to include
 that everywhere. Mostly people want to know the name of the class loader. 
 Since they don't yet have names, they want to know the type of the
 classloader. So that string a MyClassLoader would be more meaningful than 
 the 0xabc - except that if they
 have 5 of those they need the uniqueness. So personally I think I was 
 proposing the 
   java.base, 0xA/a MyClassLoader, iface).
 
 But I would defer to Staffan if he suggests something else.
 
 thanks for your patience,
 Karen
 
 With regard to the '/' format, keep in mind that we also need an indicator 
 of whether or not it's an interface, and there's also a request to include 
 the module name when that becomes available. At one point you requested the 
 following, which is what I was aiming for:
 
 java.lang.Object
 |--java.io.Serializable (java.base, 0x0007c00375f8, iface)
 |--java.util.RandomAccess (java.base, 0x0007c00375f8, iface)
 
 So maybe I can do:
 
 java.lang.Object
 |--java.io.Serializable/0x0007c00375f8 (java.base, intf)
 |--java.util.RandomAccess/0x0007c00375f8 (java.base, intf)
 ...
 |-sun.misc.Launcher$AppClassLoader/0x0007c00375f8 (java.base, 
 CLD=0x00087654320)
 |-myapp/0x00087654320
 
 So now the classname and its classloader id (which is the CLD*)  are 
 grouped together to make it easy to strip out or to search for in more than 
 one dcmd output. I think this is probably what you were striving when you 
 proposed using '/', and other DCMDs can eventually be changed

Re: [9] RFR (M) 8054888: Runtime: Add Diagnostic Command that prints the class hierarchy

[Karen Kinnear]

So I am in agreement on the CLD* - sorry for the earlier suggestion.

thanks,
Karen

On Feb 11, 2015, at 3:04 AM, Staffan Larsen wrote:

 This version looks good to me!
 
 Small comments inline.
 
 On 11 feb 2015, at 04:13, Chris Plummer chris.plum...@oracle.com wrote:
 
 Hi Staffan,
 
 Thanks for your feedback. A new incremental webrev can be found at:
 
 http://cr.openjdk.java.net/~cjplummer/8054888/webrev.01-02/
 
 Most changes are discussed inline below. Here are a couple of additional 
 changes:
 
 - Changed transitive interface to inherited interface in the output.
 - Went back to using the CLD* instead of the Klass*, except still use null 
 for the bootstrap ClassLoader.
 
 Good. 
 
 
 On 2/10/15 4:19 AM, Staffan Larsen wrote:
 Chris,
 
 In general I think this looks very good. Simple and well-commented code to 
 follow. I am missing a test, though. Please look at the 
 hotspot/test/serviceability/dcmd set of tests.
 Added.
 
 Thanks!
 
 
 
 A couple of smaller comments:
 
 
 Are Unsafe.defineAnonymousClass classes included? Should they be?
 I didn't really understand what you were talking about at first, but then 
 when I started looking at ClassLoaderStatsTest.java, I saw the following:
 
 class TestClass {
   static {
   // force creation of anonymous class (for the lambdaform)
   Runnable r = () - System.out.println(Hello);
   r.run();
   }
 }
 
 I added something similar to my test case and found a whole bunch of lines 
 like the following are suddenly added to the output:
 
 |--java.lang.invoke.LambdaForm$MH/11440528/null
 
 And there is one that seems specifically for the test case:
 
 |--DcmdTestClass$$Lambda$1/4081552/0xa529fbb0
 
 So I think they answer is yes they are added. As for whether or not they 
 should be, I really don't know. That's probably up to you. GC.class_stats 
 does include them however.
 
 I like to include them.
 
 
 BTW, I do not include array classes. They are intentionally stripped out 
 since they don't really have relevance in a Class hierarchy analysis. I can 
 easily add them back in if you want.
 
 I’m fine with skipping array classes.
 
 I think ClassHierarchyDCmd should include this code as well to restrict 
 remote access:
static const JavaPermission permission() {
  JavaPermission p = {java.lang.management.ManagementPermission,
  monitor, NULL};
  return p;
}
 Ok. I was a bit unclear as to when this was really needed.
 diagnosticCommand.hpp:278: Missing �if� in the comment
 Ok.
 vm_operations.hpp: Spelling error in �VM_PrintClassHierachry� and 
 �PrintClassHierachry�
 Ok.
 vm_operations.hpp:461: Should the complete class be surrounded by #if 
 INCLUDE_SERVICES� ?
 Yes, but I can't do anything about the reference in the VM_OPS_DO macro, at 
 least not in a straight forward manner. I think that part is benign, but 
 will find out when I do a minimal VM build.
 heapInspection.hpp:272: The constructor and destructor does not seem to be 
 used. Because of that you should also change it to a AllStatic class.
 Yeah, old code copied from HeapInspection class. I made it AllStatic and 
 removed the constructor and destructor. However, my lack of C++ experience 
 is showing here, and I haven't been happy about the existence of this 
 KlassHierarchy class. It's static with just one public API. It's purpose in 
 life is just to allow a VM operation to call that public method, but it just 
 as easily could have been a regular C function call. Likewise the two 
 private static methods in KlassHierarchy could have been C functions. There 
 is no encapsulation or subclassing going on here. If you have 
 recommendations for further improvement I'm open to suggestions. Otherwise 
 I'll leave it with just the changes mentioned.
 
 I come from a C background as well, so I don’t have much to add here. I think 
 this looks reasonable.
 
 heapInspection.cpp:339: Shouldn�t this be labeled as an �error�?
 That would probably be better. I had copied it from line 742. Should I make 
 that one an ERROR also to be consistent?
 
 Yes, please.
 
 Thanks,
 /Staffan
 
 
 thanks,
 
 Chris
 
 Thanks,
 /Staffan
 
 
 
 
 On 10 feb 2015, at 03:00, Chris Plummerchris.plum...@oracle.com  wrote:
 
 [Once again the attachment went out but the main body was stripped. Not 
 too sure what's going on, but here it is again. Sorry if you are getting 
 this twice.]
 
 I've attached updated output:
 
 � I now use the Klass* of the ClassLoader instead of the CLD*, and this 
 is documented in the help output.
 � The Klass* of the ClassLoader now immediately follows the class name, 
 and is also included when printing interface names.
 
 The webrevs can be found at:
 
 http://cr.openjdk.java.net/~cjplummer/8054888/webrev.01/
 http://cr.openjdk.java.net/~cjplummer/8054888/webrev.00-01/
 
 The first is the full webrev. The 2nd is what's changed since the last 
 webrev that was reviewed. Changes since then include:
 
 � Support for 

Re: [9] RFR (M) 8054888: Runtime: Add Diagnostic Command that prints the class hierarchy

[Karen Kinnear]

Chris,

So I was curious - I was thinking you would printing the hex address of the 
j.l.ClassLoader class rather than the cld so that if folks were to look
at a heap dump later it might be meaningful to them. Is it unlikely that they 
would ever want to correlate those?


On Feb 6, 2015, at 3:15 PM, Chris Plummer wrote:

 I was also thinking it might be a good idea to indicate what the hex value 
 is, although still have figured out the best way of doing this. Maybe just a 
 simple comment before the output. Keep in mind that eventually other DCMDS 
 will also include the cld to help uniquiely identify classes across dcmd 
 output. Also keep in mind your earlier suggestion:
 
 java.lang.Object/0x12345600
 |--java.io.Serializable/0x12345601
 |--java.util.RandomAccess/0x12345602
 |--java.lang.Iterable/0x12345603
 | |--java.util.Collection/0x12345604
 | | |--java.util.List/0x12345605
 |--java.util.AbstractCollection/0x12345606
 | |  implements java.util.Collection/0x12345604
 | |  implements java.lang.Iterable/0x12345603
 | |--java.util.AbstractList/0x12345607
 | |implements java.util.List/0x12345605
 | | |--java.util.Arrays$ArrayList/0x12345608
 | | |implements java.io.Serializable/0x12345601
 | | |implements java.util.RandomAccess/0x12345602
 
 With additions to GC.class_stats:
 
 Index Super ClassLoaderClassName
 1-1 0x0007c0034c48 java.lang.Object/0x12345600
 2 1 0x0007c0034c48 java.util.List/0x12345605
 331 0x0007c0034c48 java.util.AbstractList/0x12345607
 
 And GC.class_histogram:
 
  num #instances #bytes  class name
 --
1:   945 117736  java.lang.Object/0x12345600
2:   442  50352  java.util.List/0x12345605
3:   499  25288  java.util.AbstractList/0x12345607
 
 I think in your case you assumed we would create a unique identifier for each 
 class, but then we settled on just classname + ClassLoader identifier of some 
 sort, and the CLD* works for that. So replace your hex class ID in the above 
 with the hex CLD*.
 
 Also something Karen had said is just now starting to sink in and make sense 
 to me:
 
 |-sun.misc.Launcher$AppClassLoader/null (0xyyy)
 |-myapp/0xyyy
 
 I think the idea here is that when printing a ClassLoader subclass, you 
 include two CLDs. The first is for the ClassLoader that loaded the class, and 
 the second is the CLD for the ClassLoader subclass itself. Thus the above 
 indicates that myapp was loaded by 0xyyy, which is the 
 sun.misc.Launcher$AppClassLoader, which was loaded by the null ClassLoader. 
I apologize - I don't remember what I said. But I am not sure we need that 
level of complexity. I think I was asking that one of the dcmds that gives
information could print information like 0xabc a 
sun.misc.Launcher$AppClassLoader or a WLS.blah.GenericClassLoader if we 
don't want to include
that everywhere. Mostly people want to know the name of the class loader. Since 
they don't yet have names, they want to know the type of the
classloader. So that string a MyClassLoader would be more meaningful than the 
0xabc - except that if they
have 5 of those they need the uniqueness. So personally I think I was proposing 
the 
  java.base, 0xA/a MyClassLoader, iface).

But I would defer to Staffan if he suggests something else.

thanks for your patience,
Karen
 
 With regard to the '/' format, keep in mind that we also need an indicator of 
 whether or not it's an interface, and there's also a request to include the 
 module name when that becomes available. At one point you requested the 
 following, which is what I was aiming for:
 
 java.lang.Object
 |--java.io.Serializable (java.base, 0x0007c00375f8, iface)
 |--java.util.RandomAccess (java.base, 0x0007c00375f8, iface)
 
 So maybe I can do:
 
 java.lang.Object
 |--java.io.Serializable/0x0007c00375f8 (java.base, intf)
 |--java.util.RandomAccess/0x0007c00375f8 (java.base, intf)
 ...
 |-sun.misc.Launcher$AppClassLoader/0x0007c00375f8 (java.base, 
 CLD=0x00087654320)
 |-myapp/0x00087654320
 
 So now the classname and its classloader id (which is the CLD*)  are grouped 
 together to make it easy to strip out or to search for in more than one dcmd 
 output. I think this is probably what you were striving when you proposed 
 using '/', and other DCMDs can eventually be changed to do the same. Also, 
 once you know the CLD of the ClassLoader, you can find out the name of the 
 ClassLoader class by looking for CLD=classloaderID
 
 I can go with null for the null ClassLoader if you want. I used it's 
 ClassLoaderData* to be consistent with the other ClassLoaders. Just let me 
 know which you prefer.
 
 thanks,
 
 Chris
 
 On 2/6/15 12:52 AM, Staffan Larsen wrote:
 I think this looks good! Perhaps we should give a hint as to what the hex 
 value is? I don�t know if it is best to print this as part of a �header� 
 printed before the rest of the 

Re: [9] RFR (M) 8054888: Runtime: Add Diagnostic Command that prints the class hierarchy

[Karen Kinnear]

Chris,

I filed JDK-8068830 to capture the request I had to print the super types of
a given class - so you don't have to deal with that as part of this exercise.

thanks,
Karen

On Jan 9, 2015, at 12:53 PM, Karen Kinnear wrote:

 Thanks Frederic for suggesting two different dcmds - they could
 share a lot of the code logic.
 
 If folks generally prefer these as separate dcmds - I can file an
 rfe to add the inverted one - i.e. start at a given class/interface and
 tell me its supertypes.
 
 thanks,
 Karen
 
 On Jan 9, 2015, at 3:53 AM, Frederic Parain wrote:
 
 
 
 On 01/08/2015 10:29 PM, Chris Plummer wrote:
 Hi Karen,
 
 Comments inline.
 
 On 1/8/15 8:07 AM, Karen Kinnear wrote:
 Chris,
 
 Thank you for doing this. I had a couple of questions/comments.
 
 I like your idea of being able to start with a specific class to show
 all subclasses of.
 Ok. I'll add that.
 
 I think the way I read the code this shows the superclass hierarchy,
 not the superinterfaces. With the addition
 of default methods in interfaces, I think we have increased the value
 in seeing superinterfaces.
 It does include interfaces in the output, but not as part of any class
 hierarchy. Interfaces are just shown as regular classes that inherit
 from Object. This is the case if one interface extends another, I
 suppose because extends is just interpreted as implements in this case.
 
 So what I personally would find useful would be to be able to start
 with a specific class and
 find the superclasses and superinterfaces of that class - for the
 debugging I do, I usually am
 trying to look up and need both sets of information. Today if you run
 -XX:+TraceDefaultMethods
 there is one sample way to display all the supertypes of a single
 type, all the way up. I don't know the
 best way to make that consistent with the current output approach,
 e.g. using the |- syntax.
 
 e.g.
 Class java.util.Arrays$ArrayList requires default method processing
 java/util/Arrays$ArrayList
  java/util/AbstractList
java/util/AbstractCollection
  java/lang/Object
  java/util/Collection
java/lang/Object
java/lang/Iterable
  java/lang/Object
java/util/List
  java/lang/Object
  java/util/Collection
java/lang/Object
java/lang/Iterable
  java/lang/Object
  java/util/RandomAccess
java/lang/Object
  java/io/Serializable
java/lang/Object
 
 Do you think there could be value to others in the ability to walk up
 the hierarchy as well as to
 see superclasses and superinterfaces at least from that perspective?
 This is a inverted from how my dcmd prints the hierarchy, plus also
 includes interfaces. Inverting the hierarchy means a class is listed
 with every subclass of the class, which I don't think is desirable.
 Including interfaces has the same issue, but introduces a new issue even
 when not inverting the hierarchy. The same interface can be in more than
 one location in the hierarchy, so there is no avoiding printing it more
 than once, so we need to decide how to best include interfaces in the
 output.
 
 It seems to me that we have two very different use cases here, each one
 best served with a different output format:
 
 1 - Listing of all classes/interfaces hierarchy when the dcmd is
invoked without arguments:
   - Chris' output format as described below (with interfaces)
 2 - Investigation on a particular class or interface when a class
or interface is passed in argument to the dcmd
   - Karen's output format, much easier to work with to
  track default methods. Because the output is limited to the
  hierarchy from a single class, there's no class duplication
  in output (single parent class inheritance) and limited
  interfaces duplication.
 
 If the implementations of the two features are too different, we could
 consider having two different dcmds.
 
 My 2 cents,
 
 Fred
 
 The could just be a simple list right after the class that
 implements them:
 
 java.lang.Object
 | ...
 |--MyBaseClass
 | |  implements - MyInterface1
 | |  implements - MyInterface2
 | |--MySubClass
 |  implements - MyInterface1
 |  implements - MyInterface2
 | ...
 |--MyInterface1
 |--MyInterface2
 
 The implements  lines could be optional.
 
 I know cvm would distinguish between interfaces the Class declared it
 implemented, and those it inherited from the interfaces it declared it
 implemented. This was necessary for reflection, and I think also to
 properly build up interfaces tables. I assume hotspot does something
 similar. Is there any need for this information to be conveyed in the
 above output, or just list out every interface implemented, and not
 worry about how the class acquired it.
 Is there value in printing the defining class loader for each class -
 maybe optionally?
 This is already available with GC.class_stats, although not in the
 default output. I suppose the reality is that it might be better handled
 by this DCMD. The main

Re: [9] RFR (M) 8054888: Runtime: Add Diagnostic Command that prints the class hierarchy

[Karen Kinnear]

Staffan,

On Jan 9, 2015, at 7:38 AM, Staffan Larsen wrote:

 It’s getting difficult to get all the information into the same output: 
 hierarchy, interfaces, class loaders and modules. I took a stab at it and it 
 could look like this:
 
 java.lang.Object
 |--java.io.Serializable (java.base, 0x0007c00375f8, iface)
 |--java.util.RandomAccess (java.base, 0x0007c00375f8, iface)
 |--java.lang.Iterable (java.base, 0x0007c00375f8, iface)
 | |--java.util.Collection (java.base, 0x0007c00375f8, iface)
 | | |--java.util.List (java.base, 0x0007c00375f8, iface)
 |--java.util.AbstractCollection  (java.base, 0x0007c00375f8)
 | |  implements java.util.Collection
 | |  implements java.lang.Iterable
 | |--java.util.AbstractList (java.base, 0x0007c00375f8)
 | |implements java.util.List
 | | |--java.util.Arrays$ArrayList (java.base, 0x0007c00375f8)
 | | |implements java.io.Serializable
 | | |implements java.util.RandomAccess
  
 
 But this is pushing what can be visualized in one place.
 
 We will need to add module and class loader information somehow to all the 
 Diagnostic Commands that list classes. In addition we will need a way to see 
 how modules relate to one another. Perhaps it isn’t possible to have all the 
 information in one place, but instead make it possible to cross reference 
 between different diagnostic commands. For example, GC.class_stats could have 
 the module and class loader information, and GC.class_hierarchy would not 
 have to include it. What is missing from making that possible is a unique way 
 of identifying a class (since the name is not unique). All output would need 
 to include that unique identifier and it would be possible to cross 
 reference. The identifier has to be stable during a JVM run, but not between 
 runs.
I like where you are going with this. Since a given module is within a class 
loader, putting the the loader/module/class  information in class_stats would 
be helpful and then having
other dcmds just need the class/class loader pair for uniqueness. 

Not sure what you are using below for unique identifier.
I would be tempted to use the class loader hex value since otherwise you are 
introducing hex values that have no meaning other than as
cross-references. And the class/class loader pair is guaranteed uniqueness.
For any class loader you could list its hex value thereby giving the 
information in a single command that gives meaning to the loader value. 

java.lang.Object/null
...
|-sun.misc.Launcher$AppClassLoader/null (0xyyy)
|-myapp/0xyyy

Just a thought. If you are not enthused - I am ok with your proposal.

thanks,
Karen

 
 The above would then become:
 
 java.lang.Object/0x12345600
 |--java.io.Serializable/0x12345601
 |--java.util.RandomAccess/0x12345602
 |--java.lang.Iterable/0x12345603
 | |--java.util.Collection/0x12345604
 | | |--java.util.List/0x12345605
 |--java.util.AbstractCollection/0x12345606
 | |  implements java.util.Collection/0x12345604
 | |  implements java.lang.Iterable/0x12345603
 | |--java.util.AbstractList/0x12345607
 | |implements java.util.List/0x12345605
 | | |--java.util.Arrays$ArrayList/0x12345608
 | | |implements java.io.Serializable/0x12345601
 | | |implements java.util.RandomAccess/0x12345602
 
 With additions to GC.class_stats:
 
 Index Super ClassLoaderClassName
 1-1 0x0007c0034c48 java.lang.Object/0x12345600
 2 1 0x0007c0034c48 java.util.List/0x12345605
 331 0x0007c0034c48 java.util.AbstractList/0x12345607
 
 And GC.class_histogram:
 
  num #instances #bytes  class name
 --
1:   945 117736  java.lang.Object/0x12345600
2:   442  50352  java.util.List/0x12345605
3:   499  25288  java.util.AbstractList/0x12345607
 
 
 
 /Staffan
 
 On 9 jan 2015, at 09:53, Frederic Parain frederic.par...@oracle.com wrote:
 
 
 
 On 01/08/2015 10:29 PM, Chris Plummer wrote:
 Hi Karen,
 
 Comments inline.
 
 On 1/8/15 8:07 AM, Karen Kinnear wrote:
 Chris,
 
 Thank you for doing this. I had a couple of questions/comments.
 
 I like your idea of being able to start with a specific class to show
 all subclasses of.
 Ok. I'll add that.
 
 I think the way I read the code this shows the superclass hierarchy,
 not the superinterfaces. With the addition
 of default methods in interfaces, I think we have increased the value
 in seeing superinterfaces.
 It does include interfaces in the output, but not as part of any class
 hierarchy. Interfaces are just shown as regular classes that inherit
 from Object. This is the case if one interface extends another, I
 suppose because extends is just interpreted as implements in this case.
 
 So what I personally would find useful would be to be able to start
 with a specific class and
 find the superclasses and superinterfaces of that class - for the
 debugging I do, I usually am
 trying to look up and need both

Re: [9] RFR (M) 8054888: Runtime: Add Diagnostic Command that prints the class hierarchy

[Karen Kinnear]

Thanks Frederic for suggesting two different dcmds - they could
share a lot of the code logic.

If folks generally prefer these as separate dcmds - I can file an
rfe to add the inverted one - i.e. start at a given class/interface and
tell me its supertypes.

thanks,
Karen

On Jan 9, 2015, at 3:53 AM, Frederic Parain wrote:

 
 
 On 01/08/2015 10:29 PM, Chris Plummer wrote:
 Hi Karen,
 
 Comments inline.
 
 On 1/8/15 8:07 AM, Karen Kinnear wrote:
 Chris,
 
 Thank you for doing this. I had a couple of questions/comments.
 
 I like your idea of being able to start with a specific class to show
 all subclasses of.
 Ok. I'll add that.
 
 I think the way I read the code this shows the superclass hierarchy,
 not the superinterfaces. With the addition
 of default methods in interfaces, I think we have increased the value
 in seeing superinterfaces.
 It does include interfaces in the output, but not as part of any class
 hierarchy. Interfaces are just shown as regular classes that inherit
 from Object. This is the case if one interface extends another, I
 suppose because extends is just interpreted as implements in this case.
 
 So what I personally would find useful would be to be able to start
 with a specific class and
 find the superclasses and superinterfaces of that class - for the
 debugging I do, I usually am
 trying to look up and need both sets of information. Today if you run
 -XX:+TraceDefaultMethods
 there is one sample way to display all the supertypes of a single
 type, all the way up. I don't know the
 best way to make that consistent with the current output approach,
 e.g. using the |- syntax.
 
 e.g.
 Class java.util.Arrays$ArrayList requires default method processing
 java/util/Arrays$ArrayList
   java/util/AbstractList
 java/util/AbstractCollection
   java/lang/Object
   java/util/Collection
 java/lang/Object
 java/lang/Iterable
   java/lang/Object
 java/util/List
   java/lang/Object
   java/util/Collection
 java/lang/Object
 java/lang/Iterable
   java/lang/Object
   java/util/RandomAccess
 java/lang/Object
   java/io/Serializable
 java/lang/Object
 
 Do you think there could be value to others in the ability to walk up
 the hierarchy as well as to
 see superclasses and superinterfaces at least from that perspective?
 This is a inverted from how my dcmd prints the hierarchy, plus also
 includes interfaces. Inverting the hierarchy means a class is listed
 with every subclass of the class, which I don't think is desirable.
 Including interfaces has the same issue, but introduces a new issue even
 when not inverting the hierarchy. The same interface can be in more than
 one location in the hierarchy, so there is no avoiding printing it more
 than once, so we need to decide how to best include interfaces in the
 output.
 
 It seems to me that we have two very different use cases here, each one
 best served with a different output format:
 
 1 - Listing of all classes/interfaces hierarchy when the dcmd is
 invoked without arguments:
- Chris' output format as described below (with interfaces)
 2 - Investigation on a particular class or interface when a class
 or interface is passed in argument to the dcmd
- Karen's output format, much easier to work with to
   track default methods. Because the output is limited to the
   hierarchy from a single class, there's no class duplication
   in output (single parent class inheritance) and limited
   interfaces duplication.
 
 If the implementations of the two features are too different, we could
 consider having two different dcmds.
 
 My 2 cents,
 
 Fred
 
 The could just be a simple list right after the class that
 implements them:
 
 java.lang.Object
 | ...
 |--MyBaseClass
 | |  implements - MyInterface1
 | |  implements - MyInterface2
 | |--MySubClass
 |  implements - MyInterface1
 |  implements - MyInterface2
 | ...
 |--MyInterface1
 |--MyInterface2
 
 The implements  lines could be optional.
 
 I know cvm would distinguish between interfaces the Class declared it
 implemented, and those it inherited from the interfaces it declared it
 implemented. This was necessary for reflection, and I think also to
 properly build up interfaces tables. I assume hotspot does something
 similar. Is there any need for this information to be conveyed in the
 above output, or just list out every interface implemented, and not
 worry about how the class acquired it.
 Is there value in printing the defining class loader for each class -
 maybe optionally?
 This is already available with GC.class_stats, although not in the
 default output. I suppose the reality is that it might be better handled
 by this DCMD. The main puprose of GC.class_stats is to print statistics
 (counts and sizes), so printing the ClassLoader name there is probably
 not appropriate, but then it's not really appropriate for
 VM.class_hierarchy either. I'm

Re: [9] RFR (M) 8054888: Runtime: Add Diagnostic Command that prints the class hierarchy

[Karen Kinnear]

Chris,

Thank you for doing this. I had a couple of questions/comments.

I like your idea of being able to start with a specific class to show all 
subclasses of.

I think the way I read the code this shows the superclass hierarchy, not the 
superinterfaces. With the addition
of default methods in interfaces, I think we have increased the value in seeing 
superinterfaces.

So what I personally would find useful would be to be able to start with a 
specific class and
find the superclasses and superinterfaces of that class - for the debugging I 
do, I usually am
trying to look up and need both sets of information. Today if you run 
-XX:+TraceDefaultMethods
there is one sample way to display all the supertypes of a single type, all the 
way up. I don't know the
best way to make that consistent with the current output approach, e.g. using 
the |- syntax.

e.g.
Class java.util.Arrays$ArrayList requires default method processing
java/util/Arrays$ArrayList
  java/util/AbstractList
java/util/AbstractCollection
  java/lang/Object
  java/util/Collection
java/lang/Object
java/lang/Iterable
  java/lang/Object
java/util/List
  java/lang/Object
  java/util/Collection
java/lang/Object
java/lang/Iterable
  java/lang/Object
  java/util/RandomAccess
java/lang/Object
  java/io/Serializable
java/lang/Object

Do you think there could be value to others in the ability to walk up the 
hierarchy as well as to
see superclasses and superinterfaces at least from that perspective? 

Is there value in printing the defining class loader for each class - maybe 
optionally?
If so, I'm wondering if there might be value in future for the jigsaw project 
adding the module for each class - maybe optionally as well?
Love opinions on that  - especially from the serviceability folks 

thanks,
Karen


On Jan 7, 2015, at 6:29 PM, Chris Plummer wrote:

 Hi,
 
 Please review the following changes for the addition of the 
 VM.class_hierarchy DCMD. Please read the bug first for some background 
 information.
 
 Webrev: http://cr.openjdk.java.net/~cjplummer/8054888/webrev.00/
 Bug: https://bugs.openjdk.java.net/browse/JDK-8054888
 
 I expect there will be further restructuring or additional feature work. More 
 discussion on that below. I'm not sure if that additional work will be done 
 later with a separately bug filed or with this initial commit. That's one 
 thing I want to work out with this review.
 
 Currently the bulk of the DCMD is implemented in heapInspection.cpp. The  
 main purpose of this file is to implement the GC.class_stats and 
 GC.class_histogram DCMDs. Both of them require walking the java heap to count 
 live objects of each type, thus the name heapInspection.cpp. This new 
 VM.class_hierarchy DCMD does not require walking the heap, but is implemented 
 in this file because it leverages the existing KlassInfoTable and related 
 classes (KlassInfoEntry, KlassInfoBucket, and KlassClosure).
 
 KlassInfoTable makes it easy to build a database of all loaded classes, save 
 additional info gathered for each class, iterate over them quickly, and also 
 do quick lookups. This exactly what I needed for this DCMD, thus the reuse. 
 There is some downside to this. For starters, heapInspection.cpp is not the 
 proper place for a DCMD that has nothing to do with heap inspection. Also, 
 KlassInfoEntry is being overloaded now to support 3 different DCMDs, as is 
 KlassInfoTable. As a result each has a few fields and methods that are not 
 used for all 3 DCMDs. Some subclassing might be in order here, but I'm not 
 sure if it's worth it. Opinions welcomed. If I am going to refactor, I would 
 prefer that be done as a next step so I'm not disturbing the existing DCMDs 
 with this first implementation.
 
 I added some comments to code only used for GC.class_stats and 
 GC.class_histogram. I did this when trying to figure them out so I could 
 better understand how to implement VM.class_hierarchy. I can take them out if 
 you think they are not appropriate for this commit.
 
 One other item I like to discuss is whether it is worth adding a class name 
 argument to this DCMD. That would cause just the superclasses and subclasses 
 of the named class to be printed. If you think that is useful, I think it can 
 be added without too much trouble.
 
 At the moment not much testing has been done other than running the DCMD and 
 looking at the output. I'll do more once it's clear the code has settled. I 
 would like to know if there are any existing tests for GC.class_stats and 
 GC.class_histogram (there are none in the test directory). If so, possibly 
 one could serve as the basis for a new test for VM.class_hierarchy.
 
 thanks,
 
 Chris

Re: RFR round 0 JDK8u backport of ObjectMonitor-JVM/TI hang fix (8028073)

[Karen Kinnear]

Dan,

Code looks good.  This makes sense to me. Thank you for the detailed comments 
and testing.

thanks,
Karen

p.s. sorry - you would think getting three copies of the review request would 
mean
I would not completely overlook this in my emails :-)
On Feb 21, 2014, at 10:40 PM, Daniel D. Daugherty wrote:

 Greetings,
 
 This is a code review request for the JDK8u-hs-dev backport of the
 following ObjectMonitor-JVM/TI hang fix:
 
8028073 race condition in ObjectMonitor implementation causing deadlocks
https://bugs.openjdk.java.net/browse/JDK-8028073
 
 Here is the JDK8u-hs-dev webrev URL:
 
 http://cr.openjdk.java.net/~dcubed/8028073-webrev/0-jdk8u-hs-dev/
 
 This is _almost_ a straight forward backport of the JDK9 fix. The only
 difference to the fix was discussed at the end of the JDK9 review and
 was determined to only be needed in versions of HotSpot without the
 fix for 8028280:
 
 http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2014-February/010745.html
 
 8028280 has not yet been backported to JDK8u-hs-dev.
 
 The easiest way to review the backport is to download the two patch
 files from the webrevs and compare them with something like:
 
jfilemerge -r -w 8028073_exp.patch 8028073_exp_for_jdk8u_hs.patch
 
 The same testing has been performed on the JDK8u-hs-dev version as
 with the JDK9-hs-runtime version.
 
 Thanks, in advance, for any comments, questions or suggestions.
 
 Dan
 
 
 On 2/1/14 11:38 AM, Daniel D. Daugherty wrote:
  Greetings,
 
  I have a fix ready for the following bug:
 
  8028073 race condition in ObjectMonitor implementation causing deadlocks
  https://bugs.openjdk.java.net/browse/JDK-8028073
 
  On the surface, this is a very simple fix that relocates a few lines of
  code, relocates and rewrites the comments associated with that code and
  adds several new comments.
 
  Of course, in reality, the issue is much more complicated, but I'm
  hoping to make it easy for anyone not acquainted with this issue to
  understand what's going on.
 
  Here are the JDK9 webrev URLs:
 
  OpenJDK:
  http://cr.openjdk.java.net/~dcubed/8028073-webrev/0-jdk9-hs-runtime/
 
  Oracle internal:
  http://javaweb.us.oracle.com/~ddaugher/8028073-webrev/0-jdk9-hs-runtime/
 
  The simple summary:
 
  - since Java Monitors and JVM/TI RawMonitors share a ParkEvent,
it is possible for a JVM/TI monitor event handler to accidentally
consume a ParkEvent.unpark() call meant for Java Monitor layer
  - the original code fix was made on 2005.07.04 using this bug ID:
https://bugs.openjdk.java.net/browse/JDK-5030359
  - it's the right fix, but it's in the wrong place
  - the fix needs to be after the JVMTI_EVENT_MONITOR_WAITED
event handler is called because it is that event handler
that can cause the hang
 
 
  Testing
  ---
 
  - a new StessMonitorWait test has been created that reliably
reproduces the hang in JDK[6789]; see the bug's gory details
for the specific versions where the hang has been reproduced
- the test reliably reproduces the hang in 5 seconds on my
  T7600 running Solaris 10u11 X86; 1 minute runs reproduce
  the hang reliably on other machines
- 12 hour stress run of the new test on Linux-X64, MacOS X-X64,
  Solaris-SPARCV9, Solaris-X64, and Win7-X86 with the JPRT
  bits did not reproduce the hang
  - JPRT test job
  - VM/SQE Adhoc test job on Server VM, fastdebug bits on Linux-X86,
Linux-X64, MacOS X-X64, Solaris-SPARCV9, Solaris-X64, Windows-X86,
and Windows-X64:
- vm.quick
- Kitchensink (bigapps)
- Weblogic+medrec (bigapps)
- runThese (bigapps)
 
 
  The Gory Details Start Here
  ---
 
  This is the old location of block of code that's being moved:
 
  src/share/vm/runtime/objectMonitor.cpp:
 
  1440 void ObjectMonitor::wait(jlong millis, bool interruptible, TRAPS) {
  snip
  1499exit (true, Self) ;// exit the monitor
  snip
  1513if (node._notified != 0  _succ == Self) {
  1514   node._event-unpark();
  1515}
 
 
  This is the new location of block of code that's being moved:
 
  src/share/vm/runtime/objectMonitor.cpp:
 
  1452 void ObjectMonitor::wait(jlong millis, bool interruptible, TRAPS) {
  snip
  1601  if (JvmtiExport::should_post_monitor_waited()) {
  1602JvmtiExport::post_monitor_waited(jt, this, ret == OS_TIMEOUT);
  snip
  1604if (node._notified != 0  _succ == Self) {
  snip
  1620  node._event-unpark();
  1621}
 
 
  The Risks
  -
 
  - The code now executes only when the JVMTI_EVENT_MONITOR_WAITED event
is enabled:
- previously it was always executed
- while the old code was not effective for the hang that is being
  fixed with this bug, it is possible that the old code prevented
  a different bug in the successor protocol from manifesting
- thorough analysis of the successor protocol did not reveal a
  case where the old code was needed 

Re: code review round 0 for ObjectMonitor-JVM/TI hang fix (8028073)

[Karen Kinnear]

Dan,

Thank you so much. My bad - I was looking at a jdk8 repo, not a jdk9 one.

So I agree that the JDK9 fix as is works. Code change reviewed.

For JDK8:
I don't believe we were planning to backport this to 8 given risks of changes 
in this area.

I did reach the same conclusion you did, that the WaitSetLock acquirers who 
already own
the lock don't have this issue, but those that don't already own the lock do 
have
the problem, and the timed wait could trigger this.
And that a JDK8 fix would take the change out of the jvmti conditional, or need 
the 8028280
fix, which I also believe we do not plan to backport.

thank you for the detailed walk-through,
Karen

On Feb 10, 2014, at 1:55 PM, Daniel D. Daugherty wrote:

 On 2/9/14 8:37 PM, David Holmes wrote:
 trimming content ...
 
 On 8/02/2014 9:45 AM, Daniel D. Daugherty wrote:
 On 2/7/14 2:56 PM, Karen Kinnear wrote:
 3. Did I read the code correctly that the Thread::SpinAcquire can make
 a timed park
 call on the same thread's _ParkEvent? And that this is used to get on
 and off the wait queue,
 i.e. to acquire the WaitSetLock?
Is there the same risk that a notify might be eaten here also?
 
 As far as I can see, Thread::SpinAcquire() does not use a ParkEvent
 
 It sure does:
 
 void Thread::SpinAcquire (volatile int * adr, const char * LockName) {
  if (Atomic::cmpxchg (1, adr, 0) == 0) {
 return ;   // normal fast-path return
  }
 
  // Slow-path : We've encountered contention -- Spin/Yield/Block strategy.
  TEVENT (SpinAcquire - ctx) ;
  int ctr = 0 ;
  int Yields = 0 ;
  for (;;) {
 while (*adr != 0) {
++ctr ;
if ((ctr  0xFFF) == 0 || !os::is_MP()) {
   if (Yields  5) {
 // Consider using a simple NakedSleep() instead.
 // Then SpinAcquire could be called by non-JVM threads
 Thread::current()-_ParkEvent-park(1) ;
 
 U... that's not the code I'm seeing...
 
 src/share/vm/runtime/thread.cpp:
 
  4417  void Thread::SpinAcquire (volatile int * adr, const char * LockName) {
  4418if (Atomic::cmpxchg (1, adr, 0) == 0) {
  4419   return ;   // normal fast-path return
  4420}
  4421
  4422// Slow-path : We've encountered contention -- Spin/Yield/Block 
 strategy.
  4423TEVENT (SpinAcquire - ctx) ;
  4424int ctr = 0 ;
  4425int Yields = 0 ;
  4426for (;;) {
  4427   while (*adr != 0) {
  4428  ++ctr ;
  4429  if ((ctr  0xFFF) == 0 || !os::is_MP()) {
  4430 if (Yields  5) {
  4431   os::naked_short_sleep(1);
  4432 } else {
  4433   os::NakedYield() ;
  4434   ++Yields ;
  4435 }
  4436  } else {
  4437 SpinPause() ;
  4438  }
  4439   }
  4440   if (Atomic::cmpxchg (1, adr, 0) == 0) return ;
  4441}
  4442  }
 
 Mr Simms recently changed the above code via:
 
 changeset:   5832:5944dba4badc
 user:dsimms
 date:Fri Jan 24 09:28:47 2014 +0100
 summary: 8028280: ParkEvent leak when running modified runThese which 
 only loads classes
 
 os::naked_short_sleep() is new:
 
 - BSD/MacOS X, Linux - uses nanosleep()
 - Solaris - uses usleep()
 - Windows - uses Sleep()
 
 The fix for 8028280 was pushed to JDK9/hs-rt on 2014.01.24 and to JDK9/hs
 on 2014.01.29. I don't see any signs that Mr Simm's fix will be backported
 to JDK8u/HSX-25u (yet) so this part of the review thread might impact the
 backport of my fix to earlier releases.
 
 
 So considering Karen's question ... I can't tell for certain. :(
 
 I do not think the SpinAcquire on grabbing the wait-set lock to add to the 
 wait-set can be an issue because we will only park in response to the actual 
 wait, and hence only get unparked due to a notify/notifyAll, but at this 
 point we still own the monitor so no notify/notifyAll is possible.
 
 However, for the removal from the wait-set a more complex analysis is 
 needed. To do the SpinAcquire we must still be flagged as TS_WAIT - which 
 means we have not been notified, but must be returning due to a timeout (or 
 spurious wakeup?). In such circumstances could we be _succ? I don't think so 
 but I'll leave it to Dan to confirm that part :)
 
 So for HSX-25 and probably older...
 
 There are four Thread::SpinAcquire() calls in the objectMonitor code:
 
Thread::SpinAcquire (_WaitSetLock, WaitSet - add) ;
Thread::SpinAcquire (_WaitSetLock, WaitSet - unlink) ;
Thread::SpinAcquire (_WaitSetLock, WaitSet - notify) ;
Thread::SpinAcquire (_WaitSetLock, WaitSet - notifyall) ;
 
 We can easily rule out the notify and notifyAll uses since the
 current thread owns the Java-level monitor and there are no events
 to post in this part of the notify() or notifyAll() protocols.
 
 For the WaitSet - add use, the current thread owns the Java-level
 monitor and the thread has not been added as a waiter yet so another
 thread cannot do the notify-exit-make-successor part of the protocol
 yet.
 
 For the WaitSet

hg: hsx/hotspot-rt/hotspot: 8026066: ICCE for invokeinterface static

[karen . kinnear]

Changeset: 7a58803b5069
Author:acorn
Date:  2013-12-03 08:36 -0800
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/7a58803b5069

8026066: ICCE for invokeinterface static
Reviewed-by: coleenp, lfoltan, hseigel

! src/share/vm/interpreter/linkResolver.cpp
! src/share/vm/interpreter/linkResolver.hpp
! test/TEST.groups
! test/runtime/8024804/RegisterNatives.java

hg: hsx/hotspot-rt/hotspot: 8028438: static superclass method masks default methods

[karen . kinnear]

Changeset: 379f11bc04fc
Author:acorn
Date:  2013-12-03 11:13 -0800
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/379f11bc04fc

8028438: static superclass method masks default methods
Reviewed-by: hseigel, lfoltan, coleenp

! src/share/vm/classfile/defaultMethods.cpp
! src/share/vm/interpreter/linkResolver.cpp
! src/share/vm/oops/instanceKlass.cpp
! src/share/vm/oops/instanceKlass.hpp
! src/share/vm/oops/klassVtable.cpp

hg: hsx/hotspot-rt/hotspot: 8027229: ICCE expected for =2 maximally specific default methods.

[karen . kinnear]

Changeset: fce21ac5968d
Author:acorn
Date:  2013-11-13 07:31 -0800
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/fce21ac5968d

8027229: ICCE expected for =2 maximally specific default methods.
Summary: Need to process defaults for interfaces for invokespecial
Reviewed-by: lfoltan, hseigel, coleenp, jrose

! src/share/vm/classfile/classFileParser.cpp
! src/share/vm/classfile/defaultMethods.cpp
! src/share/vm/interpreter/linkResolver.cpp
! src/share/vm/interpreter/linkResolver.hpp
! src/share/vm/oops/klassVtable.cpp

hg: hsx/hotspot-rt/hotspot: 2 new changesets

[karen . kinnear]

Changeset: fdd464c8d62e
Author:acorn
Date:  2013-10-30 09:11 -0700
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/fdd464c8d62e

8027304: Lambda: inheriting abstract + 1 default - default, not ICCE
Reviewed-by: hseigel, zgu

! src/share/vm/classfile/defaultMethods.cpp

Changeset: 4fe7815b04f5
Author:acorn
Date:  2013-10-30 09:26 -0700
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/4fe7815b04f5

Merge

hg: hsx/hotspot-rt/hotspot: 8026299: invokespecial gets ICCE when it should get AME.

[karen . kinnear]

Changeset: 2f8728d92483
Author:acorn
Date:  2013-10-14 21:52 -0400
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/2f8728d92483

8026299: invokespecial gets ICCE when it should get AME.
Reviewed-by: ccheung, coleenp

! src/share/vm/interpreter/linkResolver.cpp
! src/share/vm/interpreter/linkResolver.hpp

hg: hsx/hotspot-rt/hotspot: 8026185: nsk/jvmit/GetMethodDeclaringClass/declcls001 failed

[karen . kinnear]

Changeset: d25557d03ec0
Author:acorn
Date:  2013-10-09 17:57 -0400
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/d25557d03ec0

8026185: nsk/jvmit/GetMethodDeclaringClass/declcls001 failed
Summary: Missed initialization. Thanks Coleen.
Reviewed-by: coleenp, minqi

! src/share/vm/oops/instanceKlass.cpp

hg: hsx/hotspot-rt/hotspot: 8026022: Verifier: allow anon classes to invokespecial host class/intf methods.

[karen . kinnear]

Changeset: c72075c2883e
Author:acorn
Date:  2013-10-08 16:58 -0400
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/c72075c2883e

8026022: Verifier: allow anon classes to invokespecial host class/intf methods.
Reviewed-by: coleenp, bharadwaj

! src/share/vm/classfile/verifier.cpp

hg: hsx/hotspot-rt/hotspot: 8009130: Lambda: Fix access controls, loader constraints.

[karen . kinnear]

Changeset: ac9cb1d5a202
Author:acorn
Date:  2013-10-07 12:20 -0400
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/ac9cb1d5a202

8009130: Lambda: Fix access controls, loader constraints.
Summary: New default methods list with inherited superinterface methods
Reviewed-by: minqi, sspitsyn, coleenp

! src/share/vm/classfile/classFileParser.cpp
! src/share/vm/classfile/defaultMethods.cpp
! src/share/vm/code/dependencies.cpp
! src/share/vm/interpreter/linkResolver.cpp
! src/share/vm/interpreter/linkResolver.hpp
! src/share/vm/memory/heapInspection.hpp
! src/share/vm/oops/instanceKlass.cpp
! src/share/vm/oops/instanceKlass.hpp
! src/share/vm/oops/klassVtable.cpp
! src/share/vm/oops/klassVtable.hpp
! src/share/vm/oops/method.cpp
! src/share/vm/oops/method.hpp
! src/share/vm/prims/jni.cpp
! src/share/vm/prims/jvmtiRedefineClasses.cpp
! src/share/vm/prims/methodHandles.cpp
! src/share/vm/runtime/reflectionUtils.cpp
! src/share/vm/runtime/reflectionUtils.hpp
! src/share/vm/runtime/vmStructs.cpp

hg: hsx/hotspot-rt/hotspot: 8011311: Private interface methods. Default conflicts:ICCE. no erased_super_default.

[karen . kinnear]

Changeset: 36b97be47bde
Author:acorn
Date:  2013-10-01 08:10 -0400
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/36b97be47bde

8011311: Private interface methods. Default conflicts:ICCE. no 
erased_super_default.
Reviewed-by: coleenp, bharadwaj, minqi

! src/share/vm/classfile/classFileParser.cpp
! src/share/vm/classfile/defaultMethods.cpp
! src/share/vm/classfile/defaultMethods.hpp
! src/share/vm/interpreter/linkResolver.cpp
! src/share/vm/oops/instanceKlass.cpp
! src/share/vm/oops/klassVtable.cpp

hg: hsx/hotspot-rt/hotspot: 3 new changesets

[karen . kinnear]

Changeset: 3a1df0dce3e5
Author:acorn
Date:  2013-08-30 15:15 -0400
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/3a1df0dce3e5

8023872: Verification error in generated lambda classes
Summary: skip verification for generated lambda classes
Reviewed-by: kamg, dholmes

! src/share/vm/classfile/verifier.cpp
! src/share/vm/runtime/globals.hpp

Changeset: 735f94656acc
Author:acorn
Date:  2013-08-30 12:56 -0700
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/735f94656acc

Merge


Changeset: 2918c7e21a3a
Author:acorn
Date:  2013-08-30 15:42 -0700
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/2918c7e21a3a

Merge

hg: hsx/hotspot-rt/hotspot: 8020489: VM crash when non-existent interface called by invokespecial

[karen . kinnear]

Changeset: 915cc4f3fb15
Author:acorn
Date:  2013-08-28 08:15 -0400
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/915cc4f3fb15

8020489: VM crash when non-existent interface called by invokespecial
Reviewed-by: kamg, coleenp

! src/share/vm/classfile/defaultMethods.cpp

hg: hsx/hotspot-rt/hotspot: 2 new changesets

[karen . kinnear]

Changeset: 91b93f523ec6
Author:acorn
Date:  2013-08-26 11:35 -0400
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/91b93f523ec6

8012294: remove generic handling for default methods
Reviewed-by: kamg, coleenp

! src/share/vm/classfile/classFileParser.cpp
! src/share/vm/classfile/defaultMethods.cpp
- src/share/vm/classfile/genericSignatures.cpp
- src/share/vm/classfile/genericSignatures.hpp
! src/share/vm/runtime/globals.hpp

Changeset: d80493ee6430
Author:acorn
Date:  2013-08-27 01:21 -0700
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/d80493ee6430

Merge

- src/share/vm/classfile/genericSignatures.cpp
- src/share/vm/classfile/genericSignatures.hpp
! src/share/vm/runtime/globals.hpp

Re: RFR(XXS): Event based tracing framework needs a mutex for thread groups

[Karen Kinnear]

Looks good.

thanks,
Karen

On Aug 21, 2013, at 7:11 AM, Markus Grönlund wrote:

 Greetings,
  
 Kindly asking for reviews for the following:
  
 Bugid: http://bugs.sun.com/view_bug.do?bug_id=8023457
  
 Webrev: http://cr.openjdk.java.net/~mgronlun/8023457/webrev01/
  
 Thanks
 Markus

Re: RFR(XS): 8020547 : Event based tracing needs a UNICODE string type (hsx24)

[Karen Kinnear]

Markus,

Looks good.

(Ok, I miss the comment about j.l.Thread :-)
ship it!
Karen

On Jul 16, 2013, at 11:48 AM, Markus Grönlund wrote:

 Greetings,
  
 Kindly asking for reviews for the following change in hsx24:
  
 Bugid: http://bugs.sun.com/view_bug.do?bug_id=8020547
  
 Webrev: http://cr.openjdk.java.net/~mgronlun/8020547/webrev01/
  
 tracetypes.xml will declare a STRING type in order to support larger strings 
 compared to existing UTF8 type (which is limited to a length of u2,  max 
 65535 chars).
  
 I have also removed some stale comments.
  
 Thank you
 Markus

Re: RFR(S): 8020107: Avoid crashes in WatcherThread

[Karen Kinnear]

Code looks good. Thank you for the assertions and comments.

Cleanly done, as usual for your changes Rickard.

thanks,
Karen

On Jul 17, 2013, at 8:22 AM, Rickard Bäckman wrote:

 Sorry everyone, wrong bugnr in the topic.
 It should be 8020701.
 
 /R
 
 On Jul 17, 2013, at 1:58 PM, Rickard Bäckman wrote:
 
 Hi all,
 
 can I please have reviews for the following change?
 
 We are adding a mechanism for avoiding some crashes in the WatcherThread 
 using different OS-specific methods. 
 
 Webrev: http://cr.openjdk.java.net/~rbackman/8020701/webrev/
 
 Thanks
 /R
 

hg: hsx/hotspot-rt/hotspot: 2 new changesets

[karen . kinnear]

Changeset: 50257d6f5aaa
Author:acorn
Date:  2013-07-09 14:02 -0400
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/50257d6f5aaa

8013635: VM should no longer create bridges for generic signatures.
Summary: Requires: 8013789: Compiler bridges, 8015402: metafactory
Reviewed-by: sspitsyn, coleenp, bharadwaj

! src/share/vm/classfile/defaultMethods.cpp
! src/share/vm/runtime/globals.hpp

Changeset: 22baec423e2f
Author:acorn
Date:  2013-07-09 22:48 +0200
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/22baec423e2f

Merge

hg: hsx/hotspot-rt/hotspot: 3 new changesets

[karen . kinnear]

Changeset: d9b08d62b95e
Author:acorn
Date:  2013-05-02 10:58 -0400
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/d9b08d62b95e

8010783: assert(s-refcount() != 0) failed: for create_overpasses
Reviewed-by: kvn, dcubed

! src/share/vm/classfile/bytecodeAssembler.cpp

Changeset: b7f3bf2ba33b
Author:acorn
Date:  2013-05-06 10:20 -0700
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/b7f3bf2ba33b

Merge

- agent/doc/c2replay.html

Changeset: f916d5986c86
Author:acorn
Date:  2013-05-06 12:36 -0700
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/f916d5986c86

Merge

Re: RFR: 7150256/8004095: Add back Remote Diagnostic Commands

[Karen Kinnear]

Frederic,

Code looks good - actually it looks very clean. Ship it.

Couple of minor comments that don't require re-review:

1. nmtDCmd.hpp/cpp - copyrights 2012 - 2012, 2013

2. jmm.h
  line 213: True is - True if

3. diagnosticFramework.hpp
  Thank you for the comments!
  line 298 rational - rationale

4. diagnosticCommand.cpp
  lines 105/109 - what prints if p._name is null?

thanks,
Karen

On Apr 30, 2013, at 12:26 PM, frederic parain wrote:

 Hi all,
 
 This is a second request for review to add back
 Remote Diagnostic Commands.
 
 This work adds a new platform MBean providing
 remote access to the diagnostic command framework
 via JMX (already accessible locally with the jcmd
 tool).
 
 There's two CR number because this work is made of two
 parts pushed to two different repositories.
 
 JDK changeset CR 7150256
 http://cr.openjdk.java.net/~fparain/7150256/webrev.06/
 
 HotSpot changeset: CR 8004095
 http://cr.openjdk.java.net/~fparain/8004095/webrev.06/
 
 Questions from previous review have been answered
 in initial review threads. Changesets also include
 some minor changes coming from internal audit and
 feedback sent in private e-mails.
 
 However, one issue is still pending: some unit tests
 use a hard coded port number, which could cause test
 failures if several instances of the same test are
 run on the same machine. I propose to postpone the
 fix of this issue after the JDK8 feature freeze
 (leaving for vacations soon, I won't have time to
 fix tests before the feature freeze).
 
 Thanks,
 
 Fred
 
 -- 
 Frederic Parain - Oracle
 Grenoble Engineering Center - France
 Phone: +33 4 76 18 81 17
 Email: frederic.par...@oracle.com

Re: RFR: 8008453: JvmtiClassFileReconstituter does not recognize default methods

[Karen Kinnear]

Staffan,

Code looks good. Many thanks for fixing this - sorry we missed this when
adding the default method handling.

thanks,
Karen

On May 2, 2013, at 1:39 PM, serguei.spit...@oracle.com wrote:

 On 5/2/13 8:26 AM, Staffan Larsen wrote:
 
 On 2 maj 2013, at 16:33, Robert Field robert.fi...@oracle.com wrote:
 
 On 05/02/13 04:08, Staffan Larsen wrote:
 
 On 2 maj 2013, at 12:58, serguei.spit...@oracle.com wrote:
 
 
 If I understand correctly, the default methods are generated by the VM 
 and have no presence in class file.
 Is that right?
 
 Yes, that is my understanding.
 
 Hmmm, maybe this is what you are saying, but, default methods most 
 certainly have a presence in class files.  The overpass methods, however, 
 are internally generated by the VM.
 
 Yes, I think what I meant is that the default methods are not present in the 
 class that implements the interface, but they are present in the interface 
 class.
 
 Good, it is more clear now.
 Robert, thank you for the comment!
 
 Thanks,
 Serguei
 
 
 /Staffan
 
 
 -Robert
 
 
 Then the fix looks good.
 
 Thanks!
 
 A typo - be included:
  631   // and should not b included in the total count
 
 I'll fix before pushing.
 
 /Staffan
 
 
 
 Thanks,
 Serguei
 
 
 On 5/2/13 1:51 AM, Staffan Larsen wrote:
 JvmtiClassFileReconstituter does not recognize some methods as generated 
 default methods when re-creating a class for retransformation. The new 
 default methods are handled by generating overpass methods inside the 
 JVM, when reconstituting a class, these methods should be skipped as 
 they aren't part of the original class.
 
 webrev: http://cr.openjdk.java.net/~sla/8008453/webrev.00/
 bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8008453 (not 
 available yet)
 
 Testing done: java/lang/instrument, com/sun/jdi, nsk.quick-jvmti.testlist
 
 I have not written a regression test since this was found by running the 
 existing java/lang/instrument/IsModifiableClassAgent.java test.
 
 Thanks,
 /Staffan
 
 
 
 
 

Re: RFR(M): 8012182: Add information about class loading and unloading to event based tracing framework (hs24) - updated

[Karen Kinnear]

Markus,

Looks good.

Minor comments:

1. systemDictionary.cpp
Does the post_class_load_event internal logic want to be #ifdef 
INCLUDE_TRACE/#endif
so it builds with embedded?

2. tracestream.hpp
  print_val (both)
  - don't you need a ResourceMark rm to use as_C_string() and 
name_and_sig_as_C_string()?

thanks,
Karen

p.s. guess I didn't track this closely enough - I thought the class load event 
was already in hs24? 
On Apr 16, 2013, at 5:35 PM, Markus Grönlund wrote:

 Hi again,
  
 The changeset for this has been updated slightly to reflect underlying 
 changes in hs24.
  
 Still looking for reviews for this change to add information about class 
 loading and unloading to the event based tracing framework for HS24.
  
 BugID:
 http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8012182
  
 Webrev (updated):
 http://cr.openjdk.java.net/~mgronlun/8012182/webrev02/
  
 Thanks in advance
 Markus
  
  
  
  
 From: Markus Grönlund 
 Sent: den 15 april 2013 10:17
 To: hotspot-runtime-...@openjdk.java.net; serviceability-dev@openjdk.java.net
 Subject: RFR(M): 8012182: Add information about class loading and unloading 
 to event based tracing framework (hs24)
  
 Greetings,
  
 Kindly asking for reviews for the change to add class load and unload 
 information to the event based tracing framework to HS24.
  
 BugID:
 http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8012182
  
 Webrev:
 http://cr.openjdk.java.net/~mgronlun/8012182/webrev01/
  
 Thanks
 Markus

hg: hsx/hotspot-rt/hotspot: 2 new changesets

[karen . kinnear]

Changeset: 2c7663baeb67
Author:acorn
Date:  2013-03-20 11:43 -0400
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/2c7663baeb67

8010017: lambda: reflection get(Declared)Methods support for default methods.
Summary: Don't expose vm generated overpass (bridges to default methods).
Reviewed-by: dholmes, fparain

! src/share/vm/prims/jvm.cpp

Changeset: 79259e97a072
Author:acorn
Date:  2013-03-20 12:20 -0400
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/79259e97a072

Merge

Re: Code Review fix for 6799919 Recursive calls to report_vm_out_of_memory are handled incorrectly

[Karen Kinnear]

Thank you for fixing this Ron and to both you and Dan for figuring out a way to 
simulate this problem
to test the fix.

So what does happen with the UseOSErrorReporting option?
If we know the answer, perhaps the comment could not state that we have to 
figure this out later?
 
On Feb 20, 2013, at 12:46 PM, Daniel D. Daugherty wrote:

 On 2/20/13 10:37 AM, Coleen Phillimore wrote:
 On 2/20/2013 12:05 PM, Daniel D. Daugherty wrote:
 On 2/20/13 9:57 AM, Daniel D. Daugherty wrote:
 On 2/20/13 9:34 AM, Coleen Phillimore wrote:
 
 This looks good.
 
 Thanks for the review! Don't know if Ron is having the same connectivity
 problems I'm having this morning, but my access into OWAN is going up
 and down...
 
 
 It looks like the webrev was updated to get rid of the unused variable, 
 so that is good.
 
 The webrev was not updated.
 
 Yes, I see that now.  Mikael has a much better eye than I do.
 
 
 Is there a test for ErrorHandlerTest in our repository already?
 
 ErrorHandlerTest? Is there yet another possible test that we don't
 know about?
 
 OK. I know that test by a different name:
 
 $ rgrep ErrorHandlerTest agent make src test
 src/share/vm/runtime/globals.hpp:  notproduct(uintx, ErrorHandlerTest, 0,   
  \
 src/share/vm/prims/jni.cpp:  
 NOT_PRODUCT(test_error_handler(ErrorHandlerTest));
 test/runtime/6888954/vmerrors.sh:-XX:ErrorHandlerTest=${i} -version 
  ${i2}.out 21
 test/runtime/6888954/vmerrors.sh:# If ErrorHandlerTest is ignored 
 (product build), stop.
 test/runtime/6888954/vmerrors.sh:echo ErrorHandlerTest=$i 
 failed ($f)
 
 Ron had previously explored using vmerror.sh to exercise the
 vm_exit_out_of_memory() code path as one of his early experiments.
 He also did some testing using the ErrorHandlerTest command line
 option. In neither case did it seem possible to get multi-threaded
 test cases up and running. Perhaps both he and I missed something.
 
 It also looks like Ron didn't record the specifics of his testing
 with either vmerrors.sh or the ErrorHandlerTest command line option
 in the bug report. I'll have to rattle his cage about that.
 
 
 My question was mostly if we had a jtreg test in hotspot/test repository 
 that exercised this ErrorHandlerTest option.   The second part of the 
 question is whether we should have a test because it'll look like it failed. 
   Maybe this is more of a question for Christian Tornqvist and SQE and is 
 not tied to this specific change.
 
 I talked to Ron about trying to test this also and we couldn't come up with 
 a good strategy either.   We need a good way to test out of C heap memory 
 without actually allocating lots of memory and running out of C heap.  Such 
 tests don't run well.  Maybe we can do something in the future with this 
 ErrorHandlerTest option to have the VM return NULL or assert for various 
 malloc calls triggered through some heuristic.   Having the experiments to 
 date recorded somewhere would be great.
 
 See the READ_ME file attached to the bug report for the craziness that
 Ron and I had to go through to properly test this. Some of what we came
 up with should be useful as a diagnostic option, but that should be done
 as a separate bug fix.
Totally agree, this should be a separate discussion.

thanks,
Karen
 
 Dan
 
 
 
 Thanks,
 Coleen
 
 Dan
 
 
 Dan
 
 
 
 Thanks,
 Coleen
 
 On 2/19/2013 6:48 PM, Daniel D. Daugherty wrote:
 Greetings, 
 
 I'm sponsoring this code review request from Ron Durbin. This change 
 is targeted at JDK8/HSX-25 in the RT_Baseline repo. 
 
 Dan 
 
 
 I have a proposed fix for the following bug: 
 
 6799919 Recursive calls to report_vm_out_of_memory are handled 
 incorrectly 
 http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6799919 
 https://jbs.oracle.com/bugs/browse/JDK-6799919 
 
 This is one of those bug fixes where the commit message nicely describes 
 the change: 
 
 6799919: Recursive calls to report_vm_out_of_memory are handled 
 incorrectly 
 Summary: report_vm_out_of_memory() should allow VMError.report_and_die() 
 to handle multiple out of native memory errors. 
 Reviewed-by: dcubed, other-reviewers 
 Contributed-by ron.dur...@oracle.com 
 
 Here is the webrev URL: 
 
 http://cr.openjdk.java.net/~dcubed/for_rdurbin/6799919-webrev/0-hsx25 
 
 Testing: 
- See the READ_ME file attached to the JDK-6799919 for the gory 
 details 
  of the testing needed to reproduce this failure and verify the fix 
- regular JPRT test job is in process 
 
 Comments, questions and suggestions are welcome. 
 
 Ron 
 
 
 
 
 

Re: Code Review fix for 6799919 Recursive calls to report_vm_out_of_memory are handled incorrectly

[Karen Kinnear]

Thanks for the explanation. The guarantee should let us know we are
in that situation.

thanks,
Karen

On Feb 20, 2013, at 1:48 PM, Daniel D. Daugherty wrote:

 Karen,
 
 Thanks for the review!
 
 
 On 2/20/13 11:39 AM, Karen Kinnear wrote:
 Thank you for fixing this Ron and to both you and Dan for figuring out a way 
 to simulate this problem
 to test the fix.
 
 So what does happen with the UseOSErrorReporting option?
 If we know the answer, perhaps the comment could not state that we have to 
 figure this out later?
 
 Since I had asked Ron to put in both the comment and the guarantee,
 I'll field that question. The UseOSErrorReporting option in
 VMError.report_and_die() is meant to handle the case where
 report_and_die() is called for every frame during some error
 reporting stack walk; Coleen made that change in report_and_die()
 years ago...
 
 Based on my analysis of how the UseOSErrorReporting option is used,
 I don't expect it to come into play when report_vm_out_of_memory()
 calls VMError.report_and_die(), but I'm paranoid so I asked Ron to
 add a guarantee() so we got some failure indication just in case
 we returned from VMError.report_and_die() at some point in the
 future. I didn't want us to return to the report_vm_out_of_memory()
 caller nor did I want us to simply call abort() like the old code.
 
 Dan
 
 
 
  
 On Feb 20, 2013, at 12:46 PM, Daniel D. Daugherty wrote:
 
 On 2/20/13 10:37 AM, Coleen Phillimore wrote:
 On 2/20/2013 12:05 PM, Daniel D. Daugherty wrote:
 On 2/20/13 9:57 AM, Daniel D. Daugherty wrote:
 On 2/20/13 9:34 AM, Coleen Phillimore wrote:
 
 This looks good.
 
 Thanks for the review! Don't know if Ron is having the same connectivity
 problems I'm having this morning, but my access into OWAN is going up
 and down...
 
 
 It looks like the webrev was updated to get rid of the unused variable, 
 so that is good.
 
 The webrev was not updated.
 
 Yes, I see that now.  Mikael has a much better eye than I do.
 
 
 Is there a test for ErrorHandlerTest in our repository already?
 
 ErrorHandlerTest? Is there yet another possible test that we don't
 know about?
 
 OK. I know that test by a different name:
 
 $ rgrep ErrorHandlerTest agent make src test
 src/share/vm/runtime/globals.hpp:  notproduct(uintx, ErrorHandlerTest, 0, 
\
 src/share/vm/prims/jni.cpp:  
 NOT_PRODUCT(test_error_handler(ErrorHandlerTest));
 test/runtime/6888954/vmerrors.sh:-XX:ErrorHandlerTest=${i} 
 -version  ${i2}.out 21
 test/runtime/6888954/vmerrors.sh:# If ErrorHandlerTest is ignored 
 (product build), stop.
 test/runtime/6888954/vmerrors.sh:echo ErrorHandlerTest=$i 
 failed ($f)
 
 Ron had previously explored using vmerror.sh to exercise the
 vm_exit_out_of_memory() code path as one of his early experiments.
 He also did some testing using the ErrorHandlerTest command line
 option. In neither case did it seem possible to get multi-threaded
 test cases up and running. Perhaps both he and I missed something.
 
 It also looks like Ron didn't record the specifics of his testing
 with either vmerrors.sh or the ErrorHandlerTest command line option
 in the bug report. I'll have to rattle his cage about that.
 
 
 My question was mostly if we had a jtreg test in hotspot/test repository 
 that exercised this ErrorHandlerTest option.   The second part of the 
 question is whether we should have a test because it'll look like it 
 failed.   Maybe this is more of a question for Christian Tornqvist and SQE 
 and is not tied to this specific change.
 
 I talked to Ron about trying to test this also and we couldn't come up 
 with a good strategy either.   We need a good way to test out of C heap 
 memory without actually allocating lots of memory and running out of C 
 heap.  Such tests don't run well.  Maybe we can do something in the future 
 with this ErrorHandlerTest option to have the VM return NULL or assert for 
 various malloc calls triggered through some heuristic.   Having the 
 experiments to date recorded somewhere would be great.
 
 See the READ_ME file attached to the bug report for the craziness that
 Ron and I had to go through to properly test this. Some of what we came
 up with should be useful as a diagnostic option, but that should be done
 as a separate bug fix.
 Totally agree, this should be a separate discussion.
 
 thanks,
 Karen
 
 Dan
 
 
 
 Thanks,
 Coleen
 
 Dan
 
 
 Dan
 
 
 
 Thanks,
 Coleen
 
 On 2/19/2013 6:48 PM, Daniel D. Daugherty wrote:
 Greetings, 
 
 I'm sponsoring this code review request from Ron Durbin. This change 
 is targeted at JDK8/HSX-25 in the RT_Baseline repo. 
 
 Dan 
 
 
 I have a proposed fix for the following bug: 
 
 6799919 Recursive calls to report_vm_out_of_memory are handled 
 incorrectly 
 http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6799919 
 https://jbs.oracle.com/bugs/browse/JDK-6799919 
 
 This is one of those bug fixes where the commit message nicely 
 describes 
 the change

hg: jdk8/tl/jdk: 2 new changesets

[karen . kinnear]

Changeset: 4f520ce7ba3f
Author:acorn
Date:  2013-02-13 16:09 -0500
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/4f520ce7ba3f

8007888: jdk fix default method: VerifyError: Illegal use of nonvirtual
Summary: Recognize VM generated method in old verifier. With 8004967
Reviewed-by: coleenp, acorn
Contributed-by: bharadwaj.yadava...@oracle.com

! src/share/javavm/export/jvm.h
! src/share/native/common/check_code.c

Changeset: e6f34051c60c
Author:acorn
Date:  2013-02-13 16:15 -0500
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/e6f34051c60c

Merge

Re: JVM/TI code review request (XS and M) (7182152)

[Karen Kinnear]

Dan,

All 3 versions of the code looks good. Thank you for enabling the printing for 
product since
this type of problem is so hard to duplicate.

A small note, I think it would have been easier for the internal code logic
for the CPCE::check_no_old_or_obsolete_entries to reverse the true/false,
but no need to change. I would appreciate the comment from 
is_interesting_method_entry copied to check_no_old_or_obsolete_entries
about virtual and final that f2 contains a method ptr instead of a vtable index.

In the jdk8 version in cpCache.cpp you've added the is_valid checks for 
metadata.
For a future cleanup, do we need f2_as_vfinal_method and 
is_interesting_method_entry
to do that as well? 

Is redefineclasses supported in the MinimalVM? 

thanks,
Karen

On Feb 1, 2013, at 2:55 PM, Daniel D. Daugherty wrote:

 Greetings,
 
 I have a fix for the following JVM/TI bug:
 
7182152 Instrumentation hot swap test incorrect monitor count
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7182152
https://jbs.oracle.com/bugs/browse/JDK-7182152
 
 The fix for the bug in the product code is one line:
 
 src/share/vm/oops/klassVtable.cpp:
 
 @@ -992,18 +1020,50 @@
   // RC_TRACE macro has an embedded ResourceMark
   RC_TRACE(0x0020, (itable method update: %s(%s),
 new_method-name()-as_C_string(),
 new_method-signature()-as_C_string()));
 }
 -break;
 +// cannot 'break' here; see for-loop comment above.
   }
   ime++;
 }
   }
 }
 
 and is applicable to JDK7u10/HSX-23.6 and JDK7u14/HSX-24. Coleen
 already fixed the bug as part of the Perm Gen Removal (PGR) project
 in HSX-25. Yes, we found a 1-line bug fix buried in the monster PGR
 changeset. Many thanks to Coleen for her help in this bug hunt!
 
 The rest of the code in the webrevs are:
 
 - additional JVM/TI tracing code backported from Coleen's PGR changeset
 - additional JVM/TI tracing code added by me and forward ported to HSX-25
 - a new -XX:TraceRedefineClasses=16384 flag value for finding these
  elusive old or obsolete methods
 - exposure of some printing code to the PRODUCT build so that the new
  tracing is available in a PRODUCT build
 
 You might be wondering why the new tracing code is exposed in a PRODUCT
 build. Well, it appears that more and more PRODUCT bits deployments are
 using JVM/TI RedefineClasses() and/or RetransformClasses() at run-time
 to instrument their systems. This bug (7182152) was only intermittently
 reproducible in the WLS environment in which it occurred so I made the
 tracing available in a PRODUCT build to assist in the hunt.
 
 Raj from the WLS team has also verified that the HSX-23.6 version of
 fix resolves the issue in his environment. Thanks Raj!
 
 Here are the URLs for the three webrevs:
 
 http://cr.openjdk.java.net/~dcubed/7182152-webrev/0-hsx23.6/
 http://cr.openjdk.java.net/~dcubed/7182152-webrev/0-hsx24/
 http://cr.openjdk.java.net/~dcubed/7182152-webrev/0-hsx25/
 
 I have run the following test suites from the JPDA stack on the
 JDK7u10/HSX-23.6 version of the fix with -XX:TraceRedefineClasses=16384
 specified:
 
sdk-jdi
sdk-jdi_closed
sdk-jli
vm-heapdump
vm-hprof
vm-jdb
vm-jdi
vm-jdwp
vm-jvmti
vm-sajdi
 
 The tested configs are:
 
{Solaris-X86, WinXP}
  X {Client VM, Server VM}
  X {-Xmixed, -Xcomp}
  X {product, fastdebug}
 
 With the 1-liner fix in place, the new tracing code does not find any
 instances of this failure mode in any of the above test suites. Without
 the the 1-liner fix in place, the new tracing code finds one instance
 of this failure mode in the above test suites:
 
test/java/lang/instrument/IsModifiableClassAgent.java
 
 There are two new tests that will be pushed to the JDK repos using
 a different bug ID (not yet filed):
 
test/com/sun/jdi/RedefineAbstractClass.sh
 test/java/lang/instrument/RedefineSubclassWithTwoInterfaces.sh
 
 There will be a separate review request for the new tests.
 
 I'm currently running the JPDA stack of tests on the JDK7u14/HSX-24
 and JDK8-B75/HSX-25 versions of the fix. That testing will likely
 take all weekend to complete.
 
 Thanks, in advance, for any comments and/or suggestions.
 
 Dan
 

hg: hsx/hotspot-rt/hotspot: 63 new changesets

[karen . kinnear]

Changeset: e51c9860cf66
Author:jmasa
Date:  2012-12-03 15:09 -0800
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/e51c9860cf66

8005082: NPG: Add specialized Metachunk sizes for reflection and anonymous 
classloaders
Reviewed-by: johnc, coleenp

! src/share/vm/classfile/classLoaderData.cpp
! src/share/vm/memory/binaryTreeDictionary.cpp
! src/share/vm/memory/collectorPolicy.cpp
! src/share/vm/memory/metachunk.cpp
! src/share/vm/memory/metachunk.hpp
! src/share/vm/memory/metaspace.cpp
! src/share/vm/memory/metaspace.hpp
! src/share/vm/runtime/globals.hpp

Changeset: 1de1b145f6bc
Author:jmasa
Date:  2012-12-26 15:05 -0800
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/1de1b145f6bc

8005486: NPG: Incorrect assertion in ChunkManager::list_index()
Reviewed-by: coleenp

! src/share/vm/memory/metaspace.cpp

Changeset: b735136e0d82
Author:johnc
Date:  2013-01-02 11:32 -0800
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/b735136e0d82

8004132: SerialGC: ValidateMarkSweep broken when running GCOld
Summary: Remove bit-rotten ValidateMarkSweep functionality and flag.
Reviewed-by: johnc, jmasa
Contributed-by: tamao tao@oracle.com

! 
src/share/vm/gc_implementation/concurrentMarkSweep/compactibleFreeListSpace.cpp
! src/share/vm/gc_implementation/g1/g1MarkSweep.cpp
! src/share/vm/gc_implementation/parallelScavenge/psMarkSweepDecorator.cpp
! src/share/vm/gc_implementation/parallelScavenge/psParallelCompact.cpp
! src/share/vm/gc_implementation/parallelScavenge/psParallelCompact.hpp
! src/share/vm/gc_implementation/shared/markSweep.cpp
! src/share/vm/gc_implementation/shared/markSweep.hpp
! src/share/vm/gc_implementation/shared/markSweep.inline.hpp
! src/share/vm/memory/genMarkSweep.cpp
! src/share/vm/memory/space.cpp
! src/share/vm/memory/space.hpp
! src/share/vm/runtime/globals.hpp
! src/share/vm/utilities/debug.cpp

Changeset: 37f7535e5f18
Author:johnc
Date:  2012-12-21 11:45 -0800
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/37f7535e5f18

8001424: G1: Rename certain G1-specific flags
Summary: Rename G1DefaultMinNewGenPercent, G1DefaultMaxNewGenPercent, and 
G1OldCSetRegionLiveThresholdPercent to G1NewSizePercent, G1MaxNewSizePercent, 
and G1MixedGCLiveThresholdPercent respectively. The previous names are no 
longer accepted.
Reviewed-by: brutisso, ysr

! src/share/vm/gc_implementation/g1/collectionSetChooser.cpp
! src/share/vm/gc_implementation/g1/g1CollectorPolicy.cpp
! src/share/vm/gc_implementation/g1/g1CollectorPolicy.hpp
! src/share/vm/gc_implementation/g1/g1_globals.hpp

Changeset: d275c3dc73e6
Author:johnc
Date:  2013-01-03 16:28 -0800
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/d275c3dc73e6

8004816: G1: Kitchensink failures after marking stack changes
Summary: Reset the marking state, including the mark stack overflow flag, in 
the event of a marking stack overflow during serial reference processing.
Reviewed-by: jmasa

! src/share/vm/gc_implementation/g1/concurrentMark.cpp
! src/share/vm/gc_implementation/g1/concurrentMark.hpp
! src/share/vm/gc_implementation/g1/concurrentMarkThread.cpp

Changeset: ca0a78017dc7
Author:brutisso
Date:  2012-12-30 08:47 +0100
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/ca0a78017dc7

8005396: Use ParNew with only one thread instead of DefNew as default for CMS 
on single CPU machines
Reviewed-by: jmasa, jcoomes

! src/share/vm/gc_implementation/concurrentMarkSweep/cmsCollectorPolicy.cpp
! src/share/vm/gc_implementation/parNew/parNewGeneration.cpp
! src/share/vm/gc_implementation/parNew/parNewGeneration.hpp
! src/share/vm/memory/collectorPolicy.cpp
! src/share/vm/memory/tenuredGeneration.cpp
! src/share/vm/runtime/arguments.cpp

Changeset: e0ab18eafbde
Author:brutisso
Date:  2013-01-04 11:10 +0100
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/e0ab18eafbde

8003820: Deprecate untested and rarely used GC combinations
Summary: Log warning messages for DefNew+CMS and ParNew+SerialOld
Reviewed-by: ysr, jwilhelm, jcoomes

! src/share/vm/runtime/arguments.cpp
! src/share/vm/runtime/arguments.hpp

Changeset: c98b676a98b4
Author:brutisso
Date:  2013-01-04 21:33 +0100
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/c98b676a98b4

8003822: Deprecate the incremental mode of CMS
Reviewed-by: johnc, jwilhelm

! src/share/vm/runtime/arguments.cpp

Changeset: 6e9174173e00
Author:jmasa
Date:  2013-01-04 17:04 -0800
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/6e9174173e00

8000325: Change default for CMSClassUnloadingEnabled to true
Reviewed-by: stefank, ysr

! src/share/vm/runtime/globals.hpp

Changeset: 0b54ffe4c2d3
Author:jmasa
Date:  2013-01-04 17:04 -0800
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/0b54ffe4c2d3

8005672: Clean up some changes to GC logging with GCCause's
Reviewed-by: johnc, ysr

! 

Re: RFR (XS): 8005592: ClassLoaderDataGraph::_unloading incorrectly defined as nonstatic in vmStructs

[Karen Kinnear]

Mikael,

I like the assertion that you added.
I believe that this field isn't used by SA - so if you could double-check that 
in SA - if that is the case, please
just remove it from vmStructs before checking in.

thanks,
Karen

On Dec 28, 2012, at 5:24 PM, Mikael Vidstedt wrote:

 
 Please review the following change.
 
 Background:
 
 The _unloading field is a static field in ClassLoaderDataGraph (in 
 classLoaderData.hpp) and should therefore be defined using static_field, as 
 opposed to nonstatic_field, in vmStructs.
 
 Apart from changing from nonstatic_field to static_field I also added an 
 assert in the CHECK_NONSTATIC_VM_STRUCT_ENTRY macro to make sure any field 
 offsets are within the bounds of the corresponding structs. The assert 
 triggers for _unloading before the change to static_field.
 
 The change passes JPRT.
 
 http://cr.openjdk.java.net/~mikael/8005592/webrev.00/
 
 Thanks,
 Mikael
 

Re: RFR: 8004095: Add support for JMX interface to Diagnostic Framework and Commands,

[Karen Kinnear]

Frederic,

Finally getting to a code review. I like the design and the code looks very 
clean.

I had a couple of questions please:

1. in the EFP and diagnosticCommand.hpp, why do GC.run and GC. run_finalization 
not need any special Java permissions?
Is this backward compatibility with existing ways to do this? 

2. jmm.h
what happens if you run with code built with an older jmm.h in another process 
- are there cases where there would
be a misinterpretation of dcmdInfo, dcmdArgInfo because the new arguments were 
not added at the end?

3. diagnosticFramework.hpp
who deallocates the JavaPermission strings?
Don't we have the same issue with the name, description and impact already?
That they never get freed?

5. diagnosticFramework.hpp line 216 DCmdParser parse - DCmdParser parses
line 220 relatively - relative

6. EFP/diagnosticCommand.cpp - this is the only major discussion point
  what is the plan for ManagementAgent.start/start_local, stop? 
  Are we planning to add a new permission?
  Seems like a remote stop of the agent stops you in your tracks - i.e. you can 
now not remotely restart
  - at one point we discussed a restart rather than a stop, which might allow 
a remote requestor to
change arguments without losing the ability to connect

7. diagnosticCommand.cpp
line 259 are disabled - is disabled

8. VMManagementImpl.c - for 7150256
line 394 least one the - least one of the

DiagnosticCommandMBean.java - for 7150256
line 76 doesn't - don't

DiagnosticCommandImpl.java - for 7150256
I tend to catch all exceptions rather than be specific - if they all would 
result
in a failure, or e.g. ignoring a diagnostic command - that allows for later 
code changes
that still get caught

9. sorry - copyrights will all want to be 2013 now

thanks,
Karen
On Dec 17, 2012, at 10:01 AM, Frederic Parain wrote:

 Staffan,
 
 Thank you for the review, I've fixed the code to address all
 the issue you reported. The new webrev is here:
 
 http://cr.openjdk.java.net/~fparain/8004095/webrev.01/
 
 Regards,
 
 Fred
 
 On 12/17/12 01:27 PM, Staffan Larsen wrote:
 Frederic,
 
 Looks good! Just a few small comments below.
 
 diagnosticCommand.cpp:255: When DisableExplicitGC is set, it would be great 
 if the SystemGC command printed on error message so that the caller knows 
 that the GC didn't happen as intended. I also think we should add an entry 
 to GCCause for GCs caused by the DCmd (but that isn't new to this change).
 
 diagnosticFramework.hpp:423: nit: misspelled enabled in the method name
 
 diagnosticFramework.cpp:435: seems some testing code has slipped in
 
 diagnosticFramework.cpp:509: nit: missing spaces around ''. (Consider 
 adding () to clarify the precedence.)
 
 Thanks,
 /Staffan
 
 On 17 dec 2012, at 12:26, Frederic Parain frederic.par...@oracle.com wrote:
 
 Hi,
 
 Please review the following change for CR 8004095.
 
 This changeset is the JDK side of two parts project.
 
 The goal of this feature is to allow remote invocations of
 Diagnostic Commands via JMX.
 
 
 Diagnostic Commands are actually invoked from the jcmd
 tool, which is a local tool using the attachAPI. It was
 not possible to remotely invoke these commands.
 This project creates a new PlatformMBean, remotely
 accessible via the Platform MBean Server, providing
 access to Diagnostic Commands too.
 
 The JDK side of this project, including the new
 Platform MBean, is covered in CR 7150256.
 
 This changeset contains the modifications in the
 JVM to support the new API.
 
 There are two types of modifications:
  1 - extension of the diagnostic command framework
  2 - extension of existing diagnostic commands
 
 Modifications of the framework:
 
 The first modification of the framework is the mechanism
 to communicate with the DiagnosticCommandsMBean defined
 in the JDK. Communications are performed via the jmm.h
 interface. In addition to a few new entries in the
 jmm structure, several data structures have been declared
 to export diagnostic command meta-data to the JDK.
 
 The second modification of the framework consists in an
 export control mechanism. Diagnostic Commands are executed
 inside the JVM, they have access to critical information
 and can perform very disruptive operations. Even if the
 DiagnosticCommandsMBean includes some security checks
 using Java Permissions, it sometime better to simply
 make a diagnostic command not available from the JMX
 API. The export control mechanism gives to diagnostic
 command developers full control on how the command will
 be accessible. When a diagnostic command is registered
 to the framework, a list of interfaces allowed to
 export this command must be provided. The current
 implementation defines three interfaces: JVM internal,
 attachAPI and JMX. A diagnostic command can be
 exported to any combination of these interfaces.
 
 Modifications of existing diagnostic commands:
 
 Because this feature allows remote invocations, it
 is important to be able to control who is invoking
 and 

hg: hsx/hotspot-rt/hotspot: 7199207: NPG: Crash in PlaceholderTable::verify after StackOverflow

[karen . kinnear]

Changeset: aefb345d3f5e
Author:acorn
Date:  2013-01-10 17:38 -0500
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/aefb345d3f5e

7199207: NPG: Crash in PlaceholderTable::verify after StackOverflow
Summary: Reduce scope of placeholder table entries to improve cleanup
Reviewed-by: dholmes, coleenp

! src/share/vm/classfile/placeholders.cpp
! src/share/vm/classfile/placeholders.hpp
! src/share/vm/classfile/systemDictionary.cpp
! src/share/vm/utilities/exceptions.hpp

hg: hsx/hotspot-rt/hotspot: 8005689: InterfaceAccessFlagsTest failures in Lambda-JDK tests

[karen . kinnear]

Changeset: adc176e95bf2
Author:acorn
Date:  2013-01-09 11:39 -0500
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/adc176e95bf2

8005689: InterfaceAccessFlagsTest failures in Lambda-JDK tests
Summary: Fix verifier for new interface access flags
Reviewed-by: acorn, kvn
Contributed-by: bharadwaj.yadava...@oracle.com

! src/share/vm/classfile/classFileParser.cpp

Re: Code Review fix for 8005044 remove crufty '_g' support from HS runtime code

[Karen Kinnear]

Sounds like time to file a separate bug to track that issue down (particularly 
the bsd issue)
and move on with this one.

Thank you so much Dan for the history and details,
Karen

On Dec 19, 2012, at 11:30 AM, Daniel D. Daugherty wrote:

 Adding the other aliases back in...
 
 Harold,
 
 This question:
 
  Another nit: Is it worth checking that all of
  /hotspot/libjvm.so could get copied to buf? (i.e.:  that
  buflen-len  strlen(/hotspot/libjvm.so)
 
 is straight forward and the short answer is: yes, that
 code makes assumptions that /hotspot/libjvm.so will
 fit into 'buf'. It should not do that.
 
 It is Ron's call as to whether he wants to fix this existing
 bug as part of this work. It is easy enough to fix the obvious
 part of the bug, but... as is normal with HotSpot, there
 is more going on here than meets the eye...
 
 The slightly longer answer is that it probably doesn't
 matter because the path being created here is a fake.
 As long as the /hotspot/l part gets copied, then the
 callers of os::jvm_path() will be able to handle the
 fakery.
 
 Of course, there is an even longer answer. Read on if
 you want gory details (and a history lesson).
 
 This gamma launcher stuff is giving me an absolute headache.
 Unfortunately, there is a lot of history involved here...
 
 Here are the variations:
 
 src/os/bsd/vm/os_bsd.cpp:
 
1715   if (Arguments::created_by_gamma_launcher()) {
1716 // Support for the gamma launcher.  Typical value for buf is
1717 // JAVA_HOME/jre/lib/arch/vmtype/libjvm.  If /jre/lib/ 
 appears at
1718 // the right place in the string, then assume we are installed in 
 a JDK and
1719 // we're done.  Otherwise, check for a JAVA_HOME environment 
 variable and
1720 // construct a path to the JVM being overridden.
 
 snip
 
1762 // If the path exists within JAVA_HOME, add the JVM library 
 name
1763 // to complete the path to JVM being overridden.  Otherwise 
 fallback
1764 // to the path to the current library.
1765 if (0 == access(buf, F_OK)) {
1766   // Use current module name libjvm
1767   len = strlen(buf);
1768   snprintf(buf + len, buflen-len, /libjvm%s, 
 JNI_LIB_SUFFIX);
1769 } else {
1770   // Fall back to path of current library
1771   rp = realpath(dli_fname, buf);
1772   if (rp == NULL)
1773 return;
1774 }
 
 src/os/linux/vm/os_linux.cpp:
 
2206   if (Arguments::created_by_gamma_launcher()) {
2207 // Support for the gamma launcher.  Typical value for buf is
2208 // JAVA_HOME/jre/lib/arch/vmtype/libjvm.so.  If 
 /jre/lib/ appears at
2209 // the right place in the string, then assume we are installed in 
 a JDK and
2210 // we're done.  Otherwise, check for a JAVA_HOME environment 
 variable and fix 
2211 // up the path so it looks like libjvm.so is installed there 
 (append a
2212 // fake suffix hotspot/libjvm.so).
 
 snip
 
2243 if (0 == access(buf, F_OK)) {
2244   // Use current module name libjvm.so
2245   len = strlen(buf);
2246   snprintf(buf + len, buflen-len, /hotspot/libjvm.so);
2247 } else {
2248   // Go back to path of .so
2249   rp = realpath(dli_fname, buf);
2250   if (rp == NULL)
2251 return;
2252 }
 
 src/os/solaris/vm/os_solaris.cpp:
 
2496   if (Arguments::created_by_gamma_launcher()) {
2497 // Support for the gamma launcher.  Typical value for buf is
2498 // JAVA_HOME/jre/lib/arch/vmtype/libjvm.so.  If 
 /jre/lib/ appears at
2499 // the right place in the string, then assume we are installed in 
 a JDK and
2500 // we're done.  Otherwise, check for a JAVA_HOME environment 
 variable and fix
2501 // up the path so it looks like libjvm.so is installed there 
 (append a
2502 // fake suffix hotspot/libjvm.so).
 
 snip
 
2539 if (0 == access(buf, F_OK)) {
2540   // Use current module name libjvm.so
2541   len = strlen(buf);
2542   snprintf(buf + len, buflen-len, /hotspot/libjvm.so);
2543 } else {
2544   // Go back to path of .so
2545   realpath((char *)dlinfo.dli_fname, buf);
2546 }
 
 src/os/windows/vm/os_windows.cpp:
 
1732   buf[0] = '\0';
1733   if (Arguments::created_by_gamma_launcher()) {
1734  // Support for the gamma launcher. Check for an
1735  // JAVA_HOME environment variable
1736  // and fix up the path so it looks like
1737  // libjvm.so is installed there (append a fake suffix
1738  // hotspot/libjvm.so).
1739  char* java_home_var = ::getenv(JAVA_HOME);
1740  if (java_home_var != NULL  java_home_var[0] != 0) {
1741 
1742 strncpy(buf, 

Re: code review for second hotspot FDS gobjcopy work around (7165598)

[Karen Kinnear]

Dan,

Code looks good.

Thank you for the work-around. 

So is this HDR_FLAGS workaround also needed in all the other jdk repositories?

And I assume the windows/makefiles/defs.make change is because you determined
that the failing tests actually pre-dated the zipped debuginfo. Did you find 
the root cause for that
yet, or is that an orthogonal topic?

And this also appears to have additional modifications for the earlier 
workaround add_gnu_debuglink.
I has assumed in reviewing 7165060 that you only used that for makefiles that 
built with DOF sections,
i.e. libjvm.so and that you had chosen not to use the simpler add_gnu_debuglink 
in all cases, even though
it is generally less risky. Did you change your approach here? Your comment was 
changed from SUNW_DOF to
SUNW_*. That makes sense to me. And are you also changing that for
the other jdk repositories?

thanks,
Karen

On May 23, 2012, at 3:59 PM, Daniel D. Daugherty wrote:

 Greetings,
 
 This is a hotspot code review request for the second of a pair of
 Full Debug Symbols gobjcopy work arounds on Solaris. The first
 hotspot FDS gobjcopy work around was reviewed using bug 7165060
 and that fixed the dtrace test failures.
 
 The gobjcopy utility also crashes due to empty sections with the
 SHF_ALLOC flagset on Solaris X64 objects. This causes build
 failures.
 
 The first new temporary work around tool is add_gnu_debuglink
 and it was added by 7165060.The second new temporary work around
 tool is:
 
 fix_empty_sec_hdr_flags - removes the SHF_ALLOC flag from empty
sections in ELF objects.
 
 These temporary work arounds are only needed until the proper
 Solaris 10 Update 6 patches are made available. The two patches
 are independent of one another which is why there are two
 separate temporary work arounds. However, we're putting the
 temporary work arounds in place because the 7u6/HSX-23.2 project
 window is closing fast.
 
 Here is the webrev URL for the HSX-24 version:
 
 http://cr.openjdk.java.net/~dcubed/fds_revamp/7165598-webrev/0/
 
 This fix will also be backported to 7u6/HSX-23.2 and I expect the
 changes to virtually identical.
 
 Thanks, in advance, for any reviews!
 
 Dan
 
 

Re: code review for JDK FDS gobjcopy work arounds (7170449)

[Karen Kinnear]

Looks good. Thank you.

thanks,
Karen

On May 23, 2012, at 3:50 PM, Daniel D. Daugherty wrote:

 Greetings,
 
 This is a JDK code review request for a pair of Full Debug Symbols
 gobjcopy work arounds on Solaris. The gobjcopy utility on Solaris 10
 corrupts the SUNW_* sections on objects. This has caused dtrace test
 failures and Monitoring  Management test failures. The gobjcopy
 utility crashes due to empty sections with the SHF_ALLOC flagset on
 Solaris X64 objects. This causes build failures.
 
 There are two new temporary work around tools:
 
 add_gnu_debuglink - adds the .gnu_debuglink section to an ELF
object without corrupting the other sections in the object.
 
 fix_empty_sec_hdr_flags - removes the SHF_ALLOC flag from empty
sections in ELF objects.
 
 These temporary work arounds are only needed until the proper
 Solaris 10 Update 6 patches are made available. The two patches
 are independent of one another which is why there are two
 separate temporary work arounds. However, we're putting the
 temporary work arounds in place because the 7u6 project window
 is closing fast.
 
 Here is the webrev URL for the JDK8-TL version:
 
 http://cr.openjdk.java.net/~dcubed/fds_revamp/7170449-webrev/0/
 
 This fix will also be backported to 7u6-TL and I expect the
 changes to virtually identical.
 
 Thanks, in advance, for any reviews!
 
 Dan
 
 

Re: code review request for FDS/aurora bug fix (7168520)

[Karen Kinnear]

Dan,

Code looks good. Thank you for figuring out this work-around.

thanks,
Karen

On May 17, 2012, at 10:14 AM, Daniel D. Daugherty wrote:

 Greetings,
 
 This is a code review request for the following P1 bug:
 
7168520 1/3 No jdk8 TL Nightly linux builds due to broken link in
b39-2012-05-13_231
 
 Here is the URL for the webrev:
 
http://cr.openjdk.java.net/~dcubed/fds_revamp/7168520-webrev/0/
 
 This fix has already been reviewed internally and is in the process of
 being pushed to the JDK8 TL repo. Since the bug impacts our internal
 aurora tool, we're moving very quickly on this issue. If this code
 review request generates any additional changes, then we'll use another
 bug ID to make those changes.
 
 Gory details below...
 
 In the original FDS implementation, per-VM libjsig.debuginfo symlinks were
 created that refer to ../libjsig.debuginfo. When the ZIP_DEBUGINFO_FILES
 feature is enabled (the default), the jre/lib/arch/libjsig.debuginfo
 files are ZIPed into jre/lib/arch/libjsig.diz. This makes the per-VM
 libjsig.debuginfo symlinks dangling because the ../libjsig.debuginfo
 files are ZIP'ed. Aurora doesn't tolerate dangling symlinks so this fix
 puts a work around in place.
 
 The work around ZIPs the per-VM libjsig.debuginfo symlinks into a per-VM
 libjsig.diz file. Aurora has no problem with ZIP files. After the work
 around is in place, the default libjsig.diz layout is:
 
 jre/lib/arch/libjsig.diz - ZIP'ed real libjsig.debuginfo
 jre/lib/arch/client/libjsig.diz  - ZIP'ed symlink
 (libjsig.debuginfo - 
 ../libjsig.debuginfo)
 jre/lib/arch/server/libjsig.diz  - ZIP'ed symlink
 (libjsig.debuginfo - 
 ../libjsig.debuginfo)
 
 Dan
 

Re: RFR: 7148497: javax.management.MBeanAttributeInfo.hashCode throws NullPointerException

[Karen Kinnear]

Frederic,

Looks good. And I like the refactoring.

A question - does the Descriptor field have the same potential for being null? 
e.g.
in MBeanFeatureInfo.java new line 160 - do you need to check getDescriptor() == 
null as well?
in MBeanConstructorinfo.java line 197, do you need the alternative null 
checking?
MBeanAttributeInfo.java line 293?
(there may be other lines, I didn't do a thorough search)

thanks,
Karen



On Apr 13, 2012, at 9:01 AM, Staffan Larsen wrote:

 This is much easier to read!
 
 In MBeanFeatureInfo.hasSameName() this check is not needed:
 
 274 if(info.getName() == null) {
 275return false;
 276 }
 
 since String.equals() with a null argument works as expected (return false). 
 The same is true for the implementation in hasSameDescription().
 
 When calling hasSameName() and hasSameDescription() the this. is not 
 needed, but perhaps using it makes the intent clearer?
 
 Thanks,
 /Staffan
 
 
 
 
 On 13 apr 2012, at 14:45, Frederic Parain wrote:
 
 Thanks Staffan and Dmitry for your feedback,
 here's a new webrev with a refactored code for
 name and descriptions fields which are common to
 most MBean*Info classes:
 
 http://cr.openjdk.java.net/~fparain/7148497/webrev.01/
 
 Thanks,
 
 Fred
 
 On 4/12/12 8:18 AM, Dmitry Samersoff wrote:
 Frederic,
 
 I'm second to Staffan.
 
 Looks good to me but would prefer to have it refactored, e.g. create a
 method equalOrNull();
 
 -Dmitry
 
 
 On 2012-04-10 18:33, Staffan Larsen wrote:
 Looks good! (Although I wish there was a better way to write code like
 this).
 
 /Staffan
 
 On 10 apr 2012, at 15:50, Frederic Parain wrote:
 
 Greetings,
 
 This is a request for review for CR
 7148497: javax.management.MBeanAttributeInfo.hashCode throws
 NullPointerException
 http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7148497
 
 Even if the initial CR only refers to MBeanAttributeInfo.hashCode(),
 the problem is not limited to the MBeanAttributeInfo.hashCode(), but
 is common to all javax.management.MBean*Info classes:
 MBeanAttributeInfo, MBeanConstructorInfo, MBeanFeatureInfo,
 MBeanInfo, MBeanNotificationInfo, MBeanOperationInfo and
 MBeanParameterInfo.
 
 The root cause of the problem is that these classes can be
 instantiated with some null fields (mainly, but not limited to, the
 name and type fields), and these fields are dereferenced
 unconditionally in some methods. Unconditional dereferencing is used
 in the hashCode method but also in the equals() method.
 
 The proposed fix improves implementation of equals() and hashCode()
 method to handle cases where some fields are null without throwing
 a NPE.
 
 Webrev:
 http://cr.openjdk.java.net/~fparain/7148497/webrev.00/
 
 Thanks,
 
 Fred
 
 --
 Frederic Parain - Oracle
 Grenoble Engineering Center - France
 Phone: +33 4 76 18 81 17
 Email: frederic.par...@oracle.com
 
 
 
 
 
 -- 
 Frederic Parain - Oracle
 Grenoble Engineering Center - France
 Phone: +33 4 76 18 81 17
 Email: frederic.par...@oracle.com
 
 

Re: RFR: 7148497: javax.management.MBeanAttributeInfo.hashCode throws NullPointerException

[Karen Kinnear]

Thank you for checking Frederic.

thanks,
Karen

On Apr 13, 2012, at 10:43 AM, Frederic Parain wrote:

 Karen,
 
 As far as I can tell, getDescriptor() never returns null.
 
 In the javax.management package, getDescriptor() calls
 ImmutableDescriptor.nonNullDescriptor(descriptor).clone()
 which returns a reference to a singleton empty Descriptor
 instance if the descriptor field is null.
 
 The getDescriptor() method is redefined in some classes of
 the javax.management.modelmbean package, but these implementations
 never return null either.
 
 So, unless I missed some code, it is safe to use getDescriptor()
 without null check.
 
 Fred
 
 On 4/13/12 3:54 PM, Karen Kinnear wrote:
 Frederic,
 
 Looks good. And I like the refactoring.
 
 A question - does the Descriptor field have the same potential for being 
 null? e.g.
 in MBeanFeatureInfo.java new line 160 - do you need to check getDescriptor() 
 == null as well?
 in MBeanConstructorinfo.java line 197, do you need the alternative null 
 checking?
 MBeanAttributeInfo.java line 293?
 (there may be other lines, I didn't do a thorough search)
 
 thanks,
 Karen
 
 
 
 On Apr 13, 2012, at 9:01 AM, Staffan Larsen wrote:
 
 This is much easier to read!
 
 In MBeanFeatureInfo.hasSameName() this check is not needed:
 
 274 if(info.getName() == null) {
 275return false;
 276 }
 
 since String.equals() with a null argument works as expected (return 
 false). The same is true for the implementation in hasSameDescription().
 
 When calling hasSameName() and hasSameDescription() the this. is not 
 needed, but perhaps using it makes the intent clearer?
 
 Thanks,
 /Staffan
 
 
 
 
 On 13 apr 2012, at 14:45, Frederic Parain wrote:
 
 Thanks Staffan and Dmitry for your feedback,
 here's a new webrev with a refactored code for
 name and descriptions fields which are common to
 most MBean*Info classes:
 
 http://cr.openjdk.java.net/~fparain/7148497/webrev.01/
 
 Thanks,
 
 Fred
 
 On 4/12/12 8:18 AM, Dmitry Samersoff wrote:
 Frederic,
 
 I'm second to Staffan.
 
 Looks good to me but would prefer to have it refactored, e.g. create a
 method equalOrNull();
 
 -Dmitry
 
 
 On 2012-04-10 18:33, Staffan Larsen wrote:
 Looks good! (Although I wish there was a better way to write code like
 this).
 
 /Staffan
 
 On 10 apr 2012, at 15:50, Frederic Parain wrote:
 
 Greetings,
 
 This is a request for review for CR
 7148497: javax.management.MBeanAttributeInfo.hashCode throws
 NullPointerException
 http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7148497
 
 Even if the initial CR only refers to MBeanAttributeInfo.hashCode(),
 the problem is not limited to the MBeanAttributeInfo.hashCode(), but
 is common to all javax.management.MBean*Info classes:
 MBeanAttributeInfo, MBeanConstructorInfo, MBeanFeatureInfo,
 MBeanInfo, MBeanNotificationInfo, MBeanOperationInfo and
 MBeanParameterInfo.
 
 The root cause of the problem is that these classes can be
 instantiated with some null fields (mainly, but not limited to, the
 name and type fields), and these fields are dereferenced
 unconditionally in some methods. Unconditional dereferencing is used
 in the hashCode method but also in the equals() method.
 
 The proposed fix improves implementation of equals() and hashCode()
 method to handle cases where some fields are null without throwing
 a NPE.
 
 Webrev:
 http://cr.openjdk.java.net/~fparain/7148497/webrev.00/
 
 Thanks,
 
 Fred
 
 --
 Frederic Parain - Oracle
 Grenoble Engineering Center - France
 Phone: +33 4 76 18 81 17
 Email: frederic.par...@oracle.com
 
 
 
 
 
 --
 Frederic Parain - Oracle
 Grenoble Engineering Center - France
 Phone: +33 4 76 18 81 17
 Email: frederic.par...@oracle.com
 
 
 
 
 -- 
 Frederic Parain - Oracle
 Grenoble Engineering Center - France
 Phone: +33 4 76 18 81 17
 Email: frederic.par...@oracle.com
 

Re: RFR: 7160924: jvmti: GetPhase returns incorrect phase before VMInit event is issued

[Karen Kinnear]

Dmitry,

I hear your point, there is a comment in the VM Initialization Event that In 
the case of VM start-up
failure, this event will not be sent.

That said, the goal of the VM initialization event is to inform the agent that 
it is free
to call any JNI or JMVTI function. Many agents want to be able to start 
tracking the VM as early as possible.

So there is a trade-off in how early you send this event and risks of VM 
failures after we send it.
To maximize benefits to users of the API, the current trade-off is to declare 
the VM initialized, i.e. in live phase,
and to post the VM Initialization event  as soon as we meet the agents' needs.

Yes, it is possible for the VM to exit later, both on additional subsystem 
startup operations and of course
due to things like out of memory errors at any time. So the consistency is 
that if we have a failure
in starting up the VM for the operations critical to jvmti agents, we exit 
without sending the event. For
failures later during startup and beyond, we don't. Bootstrapping is a very 
delicate process and 
we want to optimize for the agents starting as early as they can for the 
success cases.

While longer term we would like to improve our ability to recover from some of 
the memory errors,
I think the trade-off of empowering the agents to be functional as early as we 
can is the right balance
at this time.

So - I would support the bug fix as is.

thanks,
Karen

On Apr 12, 2012, at 9:12 AM, Dmitry Samersoff wrote:

 Rickard,
 
 As far as I understand the code, after your changes we will
 post JVMTI event
 
 JvmtiExport::post_vm_initialized();
 
 ever if later JVM aborts on some error.
 
 I'm not sure it's a good idea because in couple of other
 places we abort VM without sending the event, so
 behavior of agent become inconsistent.
 
 -Dmitry
 
 
 
 On 2012-04-12 16:12, Rickard Bäckman wrote:
 Hi,
 
 can I get review for this small change? The issue is that if we are
 running tracing startup code inbetween setting the phase to
 JVMTI_PHASE_LIVE and posting the VMInit event.
 
 Webrev:
 http://cr.openjdk.java.net/~rbackman/7160924/webrev/
 
 CR: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7160924
 
 Thanks
 /R
 
 
 -- 
 Dmitry Samersoff
 Java Hotspot development team, SPB04
 * There will come soft rains ...

hg: hsx/hotspot-rt/hotspot: 19 new changesets

[karen . kinnear]

Changeset: 1139f6b1cbd4
Author:jcoomes
Date:  2012-03-20 19:36 -0700
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/1139f6b1cbd4

7154724: jdk7u4 test properties missing from jprt.properties
Reviewed-by: brutisso

! make/jprt.properties

Changeset: 0e9e3cecdc81
Author:mgerdin
Date:  2012-03-21 08:34 +0100
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/0e9e3cecdc81

7152791: wbapi tests fail on cygwin
Summary: Detect cygwin prescence when setting up PLATFORM. Translate cygwin 
style paths before passing them on to jtreg.
Reviewed-by: jcoomes, brutisso

! test/Makefile

Changeset: 8a729074feae
Author:nloodin
Date:  2012-03-16 16:14 +0100
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/8a729074feae

7154517: Build error in hotspot-gc without precompiled headers
Reviewed-by: jcoomes, brutisso

! src/share/vm/gc_implementation/parallelScavenge/psPromotionManager.inline.hpp

Changeset: 64bf7c8270cb
Author:johnc
Date:  2012-03-12 14:59 -0700
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/64bf7c8270cb

7147724: G1: hang in SurrogateLockerThread::manipulatePLL
Summary: Attempting to initiate a marking cycle when allocating a humongous 
object can, if a marking cycle is successfully initiated by another thread, 
result in the allocating thread spinning until the marking cycle is complete. 
Eliminate a deadlock between the main ConcurrentMarkThread, the SurrogateLocker 
thread, the VM thread, and a mutator thread waiting on the 
SecondaryFreeList_lock (while free regions are going to become available) by 
not manipulating the pending list lock during the prologue and epilogue of the 
cleanup pause.
Reviewed-by: brutisso, jcoomes, tonyp

! src/share/vm/gc_implementation/g1/concurrentMarkThread.cpp
! src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp
! src/share/vm/gc_implementation/g1/vm_operations_g1.cpp
! src/share/vm/gc_implementation/g1/vm_operations_g1.hpp

Changeset: 21595f05bc93
Author:tonyp
Date:  2012-03-23 10:53 -0400
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/21595f05bc93

7146246: G1: expose some of the -XX flags that drive which old regions to 
collect during mixed GCs
Summary: Make two G1 cmd line flags available in product builds: 
G1HeapWastePercent (previously called: G1OldReclaimableThresholdPercent) and 
G1MixedGCCountTarget (previous called: G1MaxMixedGCNum). Also changed the 
default of the former from 1% to 5% and the default for 
G1OldCSetRegionLiveThresholdPercent to 90%.
Reviewed-by: azeemj, jwilhelm, johnc

! src/share/vm/gc_implementation/g1/collectionSetChooser.cpp
! src/share/vm/gc_implementation/g1/g1CollectorPolicy.cpp
! src/share/vm/gc_implementation/g1/g1_globals.hpp

Changeset: cc74fa5a91a9
Author:brutisso
Date:  2012-03-23 15:28 +0100
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/cc74fa5a91a9

7103665: HeapWord*ParallelScavengeHeap::failed_mem_allocate(unsigned 
long,bool)+0x97
Summary: Make sure that MutableNUMASpace::ensure_parsability() only calls 
CollectedHeap::fill_with_object() with valid sizes and make sure 
CollectedHeap::filler_array_max_size() returns a value that can be converted to 
an int without overflow
Reviewed-by: azeemj, jmasa, iveresov

! src/share/vm/gc_implementation/shared/mutableNUMASpace.cpp
! src/share/vm/gc_interface/collectedHeap.cpp
! src/share/vm/gc_interface/collectedHeap.hpp

Changeset: 0c49af52ff2c
Author:jwilhelm
Date:  2012-03-26 13:22 +0200
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/0c49af52ff2c

Merge


Changeset: f7c4174b33ba
Author:jiangli
Date:  2012-03-13 13:50 -0400
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/f7c4174b33ba

7109878: The instanceKlass EnclosingMethhod attribute fields can be folded into 
the _inner_class field.
Summary: Fold instanceKlass::_enclosing_method_class_index and 
instanceKlass::_enclosing_method_method_index into the 
instanceKlass::_inner_classes array.
Reviewed-by: never, coleenp
Contributed-by: Jiangli Zhou jiangli.z...@oracle.com

! agent/src/share/classes/sun/jvm/hotspot/oops/InstanceKlass.java
! src/share/vm/classfile/classFileParser.cpp
! src/share/vm/classfile/classFileParser.hpp
! src/share/vm/memory/dump.cpp
! src/share/vm/oops/instanceKlass.cpp
! src/share/vm/oops/instanceKlass.hpp
! src/share/vm/oops/instanceKlassKlass.cpp
! src/share/vm/prims/jvm.cpp
! src/share/vm/prims/jvmtiClassFileReconstituter.cpp
! src/share/vm/prims/jvmtiRedefineClasses.cpp
! src/share/vm/runtime/reflection.cpp

Changeset: 21b94feb697c
Author:collins
Date:  2012-03-13 15:37 -0700
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/21b94feb697c

Merge


Changeset: 6522ad563f99
Author:dlong
Date:  2012-03-17 17:31 -0400
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/6522ad563f99

Merge

! src/share/vm/oops/instanceKlass.cpp

Changeset: 0698f5ef5535
Author:dlong

Re: RFR: 7145419 - Ignore events from oracle packages in test.

[Karen Kinnear]

Looks good Rickard - thanks for the fixes.

Karen

On Mar 14, 2012, at 5:25 AM, Rickard Bäckman wrote:

 Hi,
 
 I would like to get reviews for CR 7145419, where we have a test that 
 sometimes fails when getting events from background threads. The test should 
 ignore those events.
 
 Webrev: http://cr.openjdk.java.net/~rbackman/7145419/webrev/
 
 Thanks
 /R

Re: RR(XS) 7149181:sun/management/jmxremote/startstop/JMXStartStopTest.sh failing on all platforms

[Karen Kinnear]

Looks good. Thanks Dmitry.

Karen

On Feb 28, 2012, at 6:54 AM, Dmitry Samersoff wrote:

 Hi Everyone,
 
 Please review:
 
 http://cr.openjdk.java.net/~dsamersoff/7149181/webrev.01/
 
 Disable test until hotspot and jdk changes meet together.
 
 -Dmitry
 
 -- 
 Dmitry Samersoff
 Java Hotspot development team, SPB04
 * There will come soft rains ...

Re: Diagnostic command fixes

[Karen Kinnear]

Looks good. Thank you.

And I will deal with the early event work when we add more events in terms of
exactly how early we need to initialize the tracing subsystem.

thanks,
Karen

On Feb 15, 2012, at 6:38 AM, Nils Loodin wrote:

 Thank you all for suggesting even more improvements!
 
 Updated the webrev and put it here: 
 http://cr.openjdk.java.net/~nloodin/7145243/webrev.02/
 
 Regards,
 Nils Loodin
 
 On Feb 14, 2012, at 18:29 , Nils Loodin wrote:
 
 Thanks all for suggestions on improvements.
 I have an updated webrev here:
 http://cr.openjdk.java.net/~nloodin/7145243/webrev.01/
 
 Regards
 Nils Loodin
 
 PS: Note - Frederic Parain has graciously taken upon himself to sponsor 
 this. Many thanks!
 
 DS
 
 On Feb 13, 2012, at 22:58 , Nils Loodin wrote:
 
 Hey all!
 
 The new diagnostic command parser needs some additional specializations for 
 time and bytes, here included.
 Also a few fixes for crashes for some combinations of commandlines.
 
 Tested by throwing a lot of different arguments on the parser, also by 
 running the tests in sun/tools/jcmd.
 
 http://cr.openjdk.java.net/~nloodin/7145243/webrev.00/
 
 I would also need a sponsor to get this in..
 
 Regards
 Nils Loodin
 
 

Re: Diagnostic command fixes

[Karen Kinnear]

Thank you Nils :-)

I had a question on the DCmdArgumentNanoTimeArgument - I think I am not 
reading
this correctly:

Couple of questions please:


1) What is this doing?
 if (idx == len) {
143 
// only accept missing unit if the value is 0
144 
if (_value._time != 0) {
145 
  THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(),
146 
Integer parsing error nanotime value: unit required);
147 
} else {
148 
  _value._nanotime = 0;
149 
  strcpy(_value._unit, ns);
150 
  return;

I haven't seen the specification at this level of detail, but I thought that if
you entered a digit value with no units that you got ns by default - i.e. we 
need to
be able to parse units, but they are not required.

2) What does this comment mean:

// WARNING StringArrayArgument can only be used as an option, it cannot be
// used as an argument with the DCmdParser
... DCmdArgumentStringArrayArgument*

-- what can't you do with it more specifically please? Do you mean you can't
pass it as an argument to parse_value?

3) diagnosticFramework.cpp: copyright 2012NAME -- oops

thanks,
Karen

On Feb 14, 2012, at 12:33 PM, Nils Loodin wrote:

 Forgot an important point - the testing
 
 First I ran all the testing we have for the tracing framework that makes use 
 of these commands.
 I have also debugged through with frivolous command lines to make sure we 
 don't do anything bad, like crash.
 Also I've manually tried to use all the combinations of the command line 
 arguments I can think of that should work, and that shouldn't. 
 
 Regards
 Nils Loodin
 
 On Feb 14, 2012, at 18:29 , Nils Loodin wrote:
 
 Thanks all for suggestions on improvements.
 I have an updated webrev here:
 http://cr.openjdk.java.net/~nloodin/7145243/webrev.01/
 
 Regards
 Nils Loodin
 
 PS: Note - Frederic Parain has graciously taken upon himself to sponsor 
 this. Many thanks!
 
 DS
 
 On Feb 13, 2012, at 22:58 , Nils Loodin wrote:
 
 Hey all!
 
 The new diagnostic command parser needs some additional specializations for 
 time and bytes, here included.
 Also a few fixes for crashes for some combinations of commandlines.
 
 Tested by throwing a lot of different arguments on the parser, also by 
 running the tests in sun/tools/jcmd.
 
 http://cr.openjdk.java.net/~nloodin/7145243/webrev.00/
 
 I would also need a sponsor to get this in..
 
 Regards
 Nils Loodin
 
 

Re: Diagnostic command fixes

[Karen Kinnear]

Frederic, Nils,

Just fix the copyright then.

responses inline ...

On Feb 14, 2012, at 1:27 PM, Frederic Parain wrote:

 
 
 On 14/02/2012 18:55, Karen Kinnear wrote:
 Thank you Nils :-)
 
 I had a question on the DCmdArgumentNanoTimeArgument - I think I am
 not reading
 this correctly:
 
 Couple of questions please:
 
 I haven't seen the specification at this level of detail, but I thought
 that if
 you entered a digit value with no units that you got ns by default -
 i.e. we need to
 be able to parse units, but they are not required.
 
 This is code matches the behavior of the JRockit parser, if the
 specification has changed to consider digit value with no units as
 ns, then we can remove this logic. Personally, I'd prefer this new
 semantic.
This was my misunderstanding then. I also looked to see how hotspot
handles time value defaults, since my goal is to have dcmd parsing be
a logical extension of how hotspot handles things today. 
Right now the command-line arguments do NOT have a default unit -
in fact many of the names explicitly include Millis. 

So I would leave this alone as requiring units. 
 
 2) What does this comment mean:
 
 // WARNING StringArrayArgument can only be used as an option, it cannot be
 // used as an argument with the DCmdParser
 ... DCmdArgumentStringArrayArgument*
 
 -- what can't you do with it more specifically please? Do you mean you can't
 pass it as an argument to parse_value?
 
 With the DCmdParser, options are defined with a key=value syntax and
 can be specified in any order. arguments don't have key, just a value
 and are identified by their position on the command line (options are
 ignored when positions are computed). There's actually no way to specify
 that an argument can be passed at multiple positions.
 StringArrayArgument is an argument type defined to accept multiple
 strings. As it is not possible to specify the multiple position for
 an argument, StringArrayArgument can only used as an option, each
 string being preceded by the same key.
Thank you - I remember when you explained this to me when you
first added the distinction between options and arguments.

thanks,
Karen

 
 Fred
 
 -- 
 Frederic Parain - Oracle
 Grenoble Engineering Center - France
 Phone: +33 4 76 18 81 17
 Email: frederic.par...@oracle.com

hg: hsx/hotspot-rt/hotspot: 7114376: Make system dictionary hashtable bucket array size configurable

[karen . kinnear]

Changeset: b2cd0ee8f778
Author:acorn
Date:  2012-01-30 23:27 -0500
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/b2cd0ee8f778

7114376: Make system dictionary hashtable bucket array size configurable
Summary: 7u4 new experimental flag -XX:PredictedClassLoadedCount=#
Reviewed-by: dholmes, phh, dcubed

! agent/src/share/classes/sun/jvm/hotspot/memory/LoaderConstraintTable.java
! agent/src/share/classes/sun/jvm/hotspot/memory/SystemDictionary.java
! src/share/vm/classfile/dictionary.cpp
! src/share/vm/classfile/systemDictionary.cpp
! src/share/vm/classfile/systemDictionary.hpp
! src/share/vm/runtime/globals.hpp
! src/share/vm/runtime/vmStructs.cpp
! src/share/vm/utilities/hashtable.hpp

Re: RFR: 7133124 Remove redundant packages from JAR command line

[Karen Kinnear]

Code change looks good.
I assume you want to put this fix back to JDK8 as well.

thank you for the quick fix!
Karen

On Jan 26, 2012, at 4:44 AM, Rickard Bäckman wrote:

 Hi,
 
 We have a problem with some versions of jar reporting errors when trying to 
 run jar cf some.jar com/test com/test/foo
 
 This fix removes the redundant subdirectories from the command.
 
 Webrev is here:
 http://cr.openjdk.java.net/~rbackman/7133124/webrev/
 
 Thanks
 /Rickard

Re: RFR: 7133124 Remove redundant packages from JAR command line

[Karen Kinnear]

Thanks Rickard.

My guess is that if it works with all version of jar which you've tested, then 
it will
be easier going forward to maintain the file consistently, and it causes no 
harm.

thanks,
Karen

On Jan 26, 2012, at 7:03 AM, Rickard Bäckman wrote:

 On 01/26/2012 12:53 PM, Karen Kinnear wrote:
 Code change looks good.
 I assume you want to put this fix back to JDK8 as well.
 
 thank you for the quick fix!
 Karen
 
 Thanks for the review Karen,
 
 I'm not sure about JDK8, I think it build with JDK1.7.0? Which has a fixed 
 version of jar.
 
 I'll try to find out if we need to fix it in 8 as well.
 
 /R
 
 
 On Jan 26, 2012, at 4:44 AM, Rickard Bäckman wrote:
 
 Hi,
 
 We have a problem with some versions of jar reporting errors when trying to 
 run jar cfsome.jar  com/test com/test/foo
 
 This fix removes the redundant subdirectories from the command.
 
 Webrev is here:
 http://cr.openjdk.java.net/~rbackman/7133124/webrev/
 
 Thanks
 /Rickard
 
 

Re: Request for Review: 7120511: Add diagnostic commands

[Karen Kinnear]

Frederic,

Code looks good.

A couple of minor comments/questions:
1) diagnosticCommand.cpp HeapDumpDCmd::execute
   - There is a comment in attachListener under dump_heap that would
  be helpful to have here as well: // Request a full GC before heap dump if 
live_objects ...

2) I wonder if it would be helpful for the  new jcmds that share functionality 
with existing mechanisms
if it is worth a comment in the code noting where else this logic is called 
- might
help future bug fixers do thorough testing.

3) attachListener jcmd
   You've added an out-cr() after throwing the exception here, but I wonder if
   you actually want it whether or not there was an exception thrown here -
   e.g. inside execute I see a number of places where there are print calls for
   exceptions and raw_print called - do those want the cr as well?
   Or are there cases where there is nothing printed so you would not want 
that? e.g. runFinalization?

thanks,
Karen

On Jan 5, 2012, at 10:19 AM, Frederic Parain wrote:

 This changeset aims to add a first set of diagnostic commands
 to the HotSpot JVM. It also includes minor modifications to
 the diagnostic command framework implementation to ease
 development of new diagnostic commands.
 
 The webrev is here:
 
 http://cr.openjdk.java.net/~fparain/7120511/webrev.00/
 
 
 Here's the list of new diagnostic commands:
 
 Thread.print
Print all threads with stacktraces.
 
 GC.class_histogram
Provides statistics about the Java heap usage
 
 GC.heap_dump
Generate a HPROF format dump of the Java heap
 
 GC.run_finalization
Call java.lang.System.runFinalization().
 
 GC.run
Call java.lang.System.gc().
 
 VM.uptime
Print VM uptime.
 
 VM.flags
Print VM flag options and their current values.
 
 VM.system_properties
Print system properties
 
 VM.command_line
Print the command line used to start this VM instance.
 
 
 Thanks,
 
 Fred
 
 -- 
 Frederic Parain - Oracle
 Grenoble Engineering Center - France
 Phone: +33 4 76 18 81 17
 Email: frederic.par...@oracle.com
 

Re: code review request for Redefine and Retransform Classes memory leak (7121600, 7122253)

[Karen Kinnear]

Dan,

All versions of hotspot changes look good.

For the JDK and test side:
1) minor: JPLISAgent.c - new lines 1210-1212 might just be an } else {
2) new lines 1311-1312
Why do you break if an error occurred? 
If there is a reason to stop freeing memory, would that also apply if
you already had had an error? So you would want to check a new 
cleanuperrorOccurred
regardless of pre-existing error?
But I suspect leaving out the break would make more sense.

thanks,
Karen

On Dec 21, 2011, at 2:09 PM, Daniel D. Daugherty wrote:

 Greetings,
 
 This is a code review request for day one memory leaks in
 the java.lang.instrument.Instrumentation.redefineClasses()
 and JVM/TI RetransformClasses() APIs.
 
 Since one leak is in the JDK and the other leak is in the
 VM, I'm sending out separate webrevs for each repo. I'm also
 attacking these leaks in three releases in parallel so there
 is a pair of webrevs for: JDK6u29, JDK7u4 and JDK8. Yes, I'm
 trying to get this all done before Christmas!
 
 Here are the bug links:
 
7121600 2/3 Instrumentation.redefineClasses() leaks class bytes
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7121600
http://monaco.sfbay.sun.com/detail.jsp?cr=7121600
 
7122253 2/3 Instrumentation.retransformClasses() leaks class bytes
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7121600
http://monaco.sfbay.sun.com/detail.jsp?cr=7122253
 
 I don't know why the bugs.sun.com links aren't showing up yet...
 
 I think it is best to review the JDK7u4/HSX-23-B06 webrevs first.
 
 Here are the webrevs for the JDK6u29/HSX-20 version of the fixes:
 
http://cr.openjdk.java.net/~dcubed/7121600-webrev/1-jdk6u29/
http://cr.openjdk.java.net/~dcubed/7122253-webrev/0-hsx20/
 
 I expect the OpenJDK6 version of the fixes to very similar if not
 identical to the JDK6u29 version. I haven't been tracking the
 OpenJDK6 project as closely as I used to so I don't know whether
 these fixes should go back there also.
 
 Here are the webrevs for the JDK7u4/HSX-23-B06 version of the fixes:
 
http://cr.openjdk.java.net/~dcubed/7121600-webrev/1-jdk7u4/
http://cr.openjdk.java.net/~dcubed/7122253-webrev/0-hsx23-b06/
 
 The JDK7u4 JLI code has some other, unrelated fixes relative to
 the JDK6u29 JLI code. That required a very minor change in my fix
 to JPLISAgent.c to insulate against unexpected JVM/TI phase values
 in a slightly different way than the original JDK7u4 code.
 
 Also, JDK7u4 was updated to the HSX-23-B08 snapshot last night, but
 I baselined and tested the fix against HSX-23-B06 so I'm sending out
 the webrev for what I actually used.
 
 Here are the webrevs for the JDK8/HSX-23-B08 version of the fixes:
 
http://cr.openjdk.java.net/~dcubed/7121600-webrev/1-jdk8/
http://cr.openjdk.java.net/~dcubed/7122253-webrev/0-hsx23-b08/
 
 The JDK7u4 and JDK8 versions of the fix for 7121600 are identical.
 The HSX-23-B06 and HSX-23-B08 versions of the fix for 7122253 are
 also identical.
 
 I've run the following test suites:
 
 - VM/NSK jvmti, VM/NSK jvmti_unit
 - VM/NSK jdwp
 - SDK/JDK com/sun/jdi, SDK/JDK closed/com/sun/jdi, VM/NSK jdi
 - SDK/JDK java/lang/instrument
 - VM/NSK hprof, VM/NSK jdb, VM/NSK sajdi
 - VM/NSK heapdump
 - SDK/JDK misc_attach, SDK/JDK misc_jvmstat, SDK/JDK misc_tools
 
 on the following configs:
 
 - {Client VM, Server VM} x {product, fastdebug} x {-Xmixed, -Xcomp}
 - Solaris X86 JDK6u29/HSX-20 (done - no regressions)
 - Solaris X86 JDK7u4/HSX-23-B06 (done - no regressions)
 - WinXP JDK7u4/HSX-23-B06 (in process, no regressions so far)
 - Solaris X86 JDK8/HSX-23-B08 (just started)
 - WinXP JDK8/HSX-23-B08 (not yet started)
 
 Thanks, in advance, for any feedback...
 
 Dan
 

Re: code review request for Redefine and Retransform Classes memory leak (7121600, 7122253)

[Karen Kinnear]

Dan,

Thank you for the quick fix - I echo Coleen's sentiments of wondering how
you managed to find the problem(s).

On Dec 21, 2011, at 5:41 PM, Daniel D. Daugherty wrote:

 Karen,
 
 Thanks for the quick review! Replies in-lined below...
 
 
 On 12/21/11 2:47 PM, Karen Kinnear wrote:
 Dan,
 
 All versions of hotspot changes look good.
 
 Thanks! Are you OK if I don't wait for someone from the new
 Serviceability team on this review? Serguei has already left
 for the holidays... and I told him to ignore any e-mails from
 me :-)
Yes - go for it. Given the holiday timing and how critical this fix
is - you have reviews from Coleen and from me.
 
 
 For the JDK and test side:
 1) minor: JPLISAgent.c - new lines 1210-1212 might just be an } else {
 
 Nice catch! This line:
 
 1212 if (!errorOccurred) {
 
 should change back to:
 
 1212 else {
 
 I missed that when I was backing out some more complicated
 stuff that I gave up on.
 
 Fixed in all three versions.
 
 
 2) new lines 1311-1312
 Why do you break if an error occurred?
 
 Because I was trying to match the existing style
 too much. The for-loop from lines 1235-1276 does
 that: if I have an error, then break...
 
 
 If there is a reason to stop freeing memory, would that also apply if
 you already had had an error? So you would want to check a new 
 cleanuperrorOccurred
 regardless of pre-existing error?
 But I suspect leaving out the break would make more sense.
 
 For this block:
 
 1304 /*
 1305  * Only check for error if we didn't already 
 have one
 1306  * so we don't overwrite errorOccurred.
 1307  */
 1308 if (!errorOccurred) {
 1309 errorOccurred = checkForThrowable(jnienv);
 1310 jplis_assert(!errorOccurred);
 1311 if (errorOccurred) {
 1312 break;
 1313 }
 1314 }
 
 I'm going to delete lines 1311-1313 so we'll just try to keep
 freeing as many of the memory arrays as we can...
Thanks - I prefer that solution.
 
 Fixed in all three versions.
 
 On a related note, jplis_assert() appears to be enabled even
 in product bits. Which means if one of the many jplis_assert()
 calls fails, then the agent terminates. I don't know the history
 behind it being enabled in all bits, but I figure that's a
 different issue for the Serviceability team to mull on...
I wondered about that, but it seemed consistent with existing
usage  - so I agree.
 
 Thanks again for the review.
 
 Dan
thanks,
Karen
 
 
 
 
 thanks,
 Karen
 
 On Dec 21, 2011, at 2:09 PM, Daniel D. Daugherty wrote:
 
 Greetings,
 
 This is a code review request for day one memory leaks in
 the java.lang.instrument.Instrumentation.redefineClasses()
 and JVM/TI RetransformClasses() APIs.
 
 Since one leak is in the JDK and the other leak is in the
 VM, I'm sending out separate webrevs for each repo. I'm also
 attacking these leaks in three releases in parallel so there
 is a pair of webrevs for: JDK6u29, JDK7u4 and JDK8. Yes, I'm
 trying to get this all done before Christmas!
 
 Here are the bug links:
 
7121600 2/3 Instrumentation.redefineClasses() leaks class bytes
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7121600
http://monaco.sfbay.sun.com/detail.jsp?cr=7121600
 
7122253 2/3 Instrumentation.retransformClasses() leaks class bytes
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7121600
http://monaco.sfbay.sun.com/detail.jsp?cr=7122253
 
 I don't know why the bugs.sun.com links aren't showing up yet...
 
 I think it is best to review the JDK7u4/HSX-23-B06 webrevs first.
 
 Here are the webrevs for the JDK6u29/HSX-20 version of the fixes:
 
http://cr.openjdk.java.net/~dcubed/7121600-webrev/1-jdk6u29/
http://cr.openjdk.java.net/~dcubed/7122253-webrev/0-hsx20/
 
 I expect the OpenJDK6 version of the fixes to very similar if not
 identical to the JDK6u29 version. I haven't been tracking the
 OpenJDK6 project as closely as I used to so I don't know whether
 these fixes should go back there also.
 
 Here are the webrevs for the JDK7u4/HSX-23-B06 version of the fixes:
 
http://cr.openjdk.java.net/~dcubed/7121600-webrev/1-jdk7u4/
http://cr.openjdk.java.net/~dcubed/7122253-webrev/0-hsx23-b06/
 
 The JDK7u4 JLI code has some other, unrelated fixes relative to
 the JDK6u29 JLI code. That required a very minor change in my fix
 to JPLISAgent.c to insulate against unexpected JVM/TI phase values
 in a slightly different way than the original JDK7u4 code.
 
 Also, JDK7u4 was updated to the HSX-23-B08 snapshot last night, but
 I baselined and tested the fix against HSX-23-B06 so I'm sending out
 the webrev for what I actually used.
 
 Here are the webrevs for the JDK8/HSX-23-B08 version of the fixes:
 
http://cr.openjdk.java.net/~dcubed

Re: code review for JVM/TI DynamicCodeGenerated event fix (7039447)

[Karen Kinnear]

Sorry, I didn't know you were going to redo this. I too have memories
of os::free of null having issues. And I also have memories of code checking
tools complaining about calls to os::free of null.

So, if you could either go back to the way it was, or only call
os::free if it is not null I would be much more comfortable.

thanks,
Karen

On 04/28/11 09:52, Daniel D. Daugherty wrote:

On 4/28/2011 7:37 AM, Daniel D. Daugherty wrote:

I'm going to take another look at what I can do to clean this
up a bit...


This version should address your concerns and Dmitry's also.
Especially since Dmitry suggested the specific code in the
post_dynamic_code_generated_internal() call. I just added
the comment and reformatted it a bit...

Dan


--- a/src/share/vm/prims/jvmtiImpl.cpp  Sat Apr 23 00:33:38 2011 -0400
+++ b/src/share/vm/prims/jvmtiImpl.cpp  Thu Apr 28 07:44:01 2011 -0600
@@ -38,6 +38,7 @@
#include runtime/handles.inline.hpp
#include runtime/interfaceSupport.hpp
#include runtime/javaCalls.hpp
+#include runtime/os.hpp
#include runtime/serviceThread.hpp
#include runtime/signature.hpp
#include runtime/vframe.hpp
@@ -939,10 +940,15 @@
  nmethodLocker::lock_nmethod(nm, true /* zombie_ok */);
  return event;
}
+
JvmtiDeferredEvent JvmtiDeferredEvent::dynamic_code_generated_event(
  const char* name, const void* code_begin, const void* code_end) {
  JvmtiDeferredEvent event = 
JvmtiDeferredEvent(TYPE_DYNAMIC_CODE_GENERATED);

-  event._event_data.dynamic_code_generated.name = name;
+  // Need to make a copy of the name since we don't know how long
+  // the event poster will keep it around after we enqueue the
+  // deferred event and return. strdup() failure is handled in
+  // the post() routine below.
+  event._event_data.dynamic_code_generated.name = os::strdup(name);
  event._event_data.dynamic_code_generated.code_begin = code_begin;
  event._event_data.dynamic_code_generated.code_end = code_end;
  return event;
@@ -968,12 +974,17 @@
  nmethodLocker::unlock_nmethod(nm);
  break;
}
-case TYPE_DYNAMIC_CODE_GENERATED:
+case TYPE_DYNAMIC_CODE_GENERATED: {
  JvmtiExport::post_dynamic_code_generated_internal(
-_event_data.dynamic_code_generated.name,
+// if strdup failed give the event a default name
+(_event_data.dynamic_code_generated.name == NULL)
+  ? unknown_code : _event_data.dynamic_code_generated.name,
_event_data.dynamic_code_generated.code_begin,
_event_data.dynamic_code_generated.code_end);
+  // release our copy
+  os::free(_event_data.dynamic_code_generated.name);
  break;
+}
default:
  ShouldNotReachHere();
  }

Re: code review for JVM/TI DynamicCodeGenerated event fix (7039447)

[Karen Kinnear]

Thank you very much. It is safer to err on the side of paranoia.

thanks,
Karen

On 04/28/11 10:38, Daniel D. Daugherty wrote:

On 4/28/2011 8:06 AM, Karen Kinnear wrote:

Sorry, I didn't know you were going to redo this. I too have memories
of os::free of null having issues. And I also have memories of code 
checking

tools complaining about calls to os::free of null.

So, if you could either go back to the way it was, or only call
os::free if it is not null I would be much more comfortable.


I did a quick survey of calls to os::free() in src/share/vm/*/*
and found that there are many places that check for NULL before
calling os::free(). Of course, there are also places that don't.

The general style in JVM/TI code is to check for NULL before
calling os::free() unless you're sure that the variable is
non-NULL. Being the paranoid person that I am, I'm going to
tweak this simple fix one more time...

I forgot to mention that the previous version and this version
allow me to drop the changes to jvmtiImpl.hpp.

This version still avoids assigning a static string to the
name field so Dmitry should be happy about that. However,
I'm adding a check for non-NULL before calling os::free()
which both Dmitry and David H insist is not necessary. In
this case, I'm going to fall back on the argument that
checking for NULL before call os::free() is consistent with
the style of paranoia used in JVM/TI.

Yes, I will make a new webrev available just to dot the i's
and cross the t's.

Dan


--- a/src/share/vm/prims/jvmtiImpl.cpp  Sat Apr 23 00:33:38 2011 -0400
+++ b/src/share/vm/prims/jvmtiImpl.cpp  Thu Apr 28 08:23:56 2011 -0600
@@ -38,6 +38,7 @@
#include runtime/handles.inline.hpp
#include runtime/interfaceSupport.hpp
#include runtime/javaCalls.hpp
+#include runtime/os.hpp
#include runtime/serviceThread.hpp
#include runtime/signature.hpp
#include runtime/vframe.hpp
@@ -939,10 +940,15 @@
  nmethodLocker::lock_nmethod(nm, true /* zombie_ok */);
  return event;
}
+
JvmtiDeferredEvent JvmtiDeferredEvent::dynamic_code_generated_event(
  const char* name, const void* code_begin, const void* code_end) {
  JvmtiDeferredEvent event = 
JvmtiDeferredEvent(TYPE_DYNAMIC_CODE_GENERATED);

-  event._event_data.dynamic_code_generated.name = name;
+  // Need to make a copy of the name since we don't know how long
+  // the event poster will keep it around after we enqueue the
+  // deferred event and return. strdup() failure is handled in
+  // the post() routine below.
+  event._event_data.dynamic_code_generated.name = os::strdup(name);
  event._event_data.dynamic_code_generated.code_begin = code_begin;
  event._event_data.dynamic_code_generated.code_end = code_end;
  return event;
@@ -968,12 +974,19 @@
  nmethodLocker::unlock_nmethod(nm);
  break;
}
-case TYPE_DYNAMIC_CODE_GENERATED:
+case TYPE_DYNAMIC_CODE_GENERATED: {
  JvmtiExport::post_dynamic_code_generated_internal(
-_event_data.dynamic_code_generated.name,
+// if strdup failed give the event a default name
+(_event_data.dynamic_code_generated.name == NULL)
+  ? unknown_code : _event_data.dynamic_code_generated.name,
_event_data.dynamic_code_generated.code_begin,
_event_data.dynamic_code_generated.code_end);
+  if (_event_data.dynamic_code_generated.name != NULL) {
+// release our copy
+os::free((void *)_event_data.dynamic_code_generated.name);
+  }
  break;
+}
default:
  ShouldNotReachHere();
  }

Re: code review for JVM/TI DynamicCodeGenerated event fix (7039447)

[Karen Kinnear]

Code looks good. And thank you for freeing memory :-)

thanks,
Karen

On Apr 27, 2011, at 3:46 PM, Daniel D. Daugherty wrote:


Greetings,

I have a fix for a bug in JVM/TI DynamicCodeGenerated event posting:

  7039447 2/1 java profiling is broken in build 139 (garbage in
  function name)

The bug has actually been tracked back to HSX-21-B04/JDK7-B134, but
the failure mode can be intermittent. Here is the webrev URL:

  http://cr.openjdk.java.net/~dcubed/7039447-webrev/0/

Here is my proposed commit message:

7039447: 2/1 java profiling is broken in build 139 (garbage in  
function name)
Summary: The name in a deferred JVM/TI DynamicCodeGenerated event  
needs to be explicitly saved.

Reviewed-by:


I'm targeting this fix at HSX-21-B12/JDK7-B142; I just missed the
RT_Baseline cutoff for HSX-21-B11/JDK7-B141. Since we are getting
down to the wire on JDK7, I would like at least three reviewers.

I'm in the process of running the following test suites (the
JPDA parts of the Serviceability stack):

  nsk.jvmti, nsk.jvmti_unit,
  nsk.jdwp,
  nsk.jdi, SDK_JDI, SDK_JLI,
  nsk.hprof, nsk.jdb,
  nsk.sajdi,
  vm.heapdump, SDK_MISC_ATTACH, SDK_MISC_JVMSTAT, SDK_MISC_TOOLS

on the following configs:

  {Solaris X86, WinXP} x {Client VM} x {product, fastdebug} x  
{Xmixed, Xcomp}


So far preliminary results show no regressions and no new failures.

I've also provided JPRT test bits to the Analyzer team and they
have confirmed that the issue is resolved.

Thanks, in advance, for any reviews.

Dan

hg: jdk7/hotspot-rt/hotspot: 23 new changesets

[karen . kinnear]

Changeset: 7dc5384467e0
Author:coleenp
Date:  2011-02-12 10:28 -0500
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/7dc5384467e0

7022659: errorHandler doesn't compile without precompiled headers
Summary: add proper includes in errorHandler.hpp
Reviewed-by: phh, kamg

! src/share/vm/utilities/errorReporter.hpp

Changeset: 0e531ab5ba04
Author:trims
Date:  2011-03-01 11:53 -0800
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/0e531ab5ba04

Merge


Changeset: 8c0d0510d36f
Author:dcubed
Date:  2011-03-03 09:31 -0800
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/8c0d0510d36f

Merge


Changeset: 4e0069ff33df
Author:johnc
Date:  2011-02-28 09:10 -0800
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/4e0069ff33df

7022200: G1: optimized build broken
Summary: Make the G1 specific version of is_in_closed_subset() available in all 
builds.
Reviewed-by: tonyp, jcoomes

! src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp
! src/share/vm/gc_implementation/g1/g1CollectedHeap.hpp

Changeset: 11303bede852
Author:jcoomes
Date:  2011-03-03 21:02 -0800
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/11303bede852

Merge


Changeset: d89a22843c62
Author:iveresov
Date:  2011-02-22 15:25 -0800
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/d89a22843c62

7020521: arraycopy stubs place prebarriers incorrectly
Summary: Rearranged the pre-barrier placement in arraycopy stubs so that they 
are properly called in case of chained calls. Also refactored the code a little 
bit so that it looks uniform across the platforms and is more readable.
Reviewed-by: never, kvn

! src/cpu/sparc/vm/stubGenerator_sparc.cpp
! src/cpu/x86/vm/stubGenerator_x86_32.cpp
! src/cpu/x86/vm/stubGenerator_x86_64.cpp

Changeset: d5a078cf7f39
Author:iveresov
Date:  2011-02-22 18:13 -0800
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/d5a078cf7f39

Merge


Changeset: ba5d119730dd
Author:kvn
Date:  2011-02-23 12:28 -0800
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/ba5d119730dd

Merge


Changeset: d411927672ed
Author:never
Date:  2011-02-23 19:09 -0800
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/d411927672ed

7012072: CompileTheWorld causes incorrect class initialization
Reviewed-by: kvn, twisti

! src/share/vm/prims/unsafe.cpp

Changeset: 5a41a201d08c
Author:kvn
Date:  2011-02-24 10:28 -0800
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/5a41a201d08c

6812217: Base memory of MergeMem node violates assert during killing expanded 
AllocateArray node
Summary: The assert in MergeMemNode::memory_at() misses the case when address 
is TOP.
Reviewed-by: never

! src/share/vm/opto/memnode.cpp

Changeset: 6f3746e69a78
Author:never
Date:  2011-02-24 11:09 -0800
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/6f3746e69a78

7021603: crash in fill_sync_handler with ExtendedDTrace probes
Reviewed-by: iveresov

! src/share/vm/c1/c1_GraphBuilder.cpp

Changeset: 8190d4b75e09
Author:never
Date:  2011-02-24 14:49 -0800
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/8190d4b75e09

Merge


Changeset: 41d4973cf100
Author:kvn
Date:  2011-02-26 12:10 -0800
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/41d4973cf100

6942326: x86 code in string_indexof() could read beyond reserved heap space
Summary: copy small (8) strings on stack if str+16 crosses a page boundary and 
load from stack into XMM. Back up pointer when loading string's tail.
Reviewed-by: never

! src/cpu/x86/vm/assembler_x86.cpp
! src/cpu/x86/vm/assembler_x86.hpp
! src/cpu/x86/vm/x86_32.ad
! src/cpu/x86/vm/x86_64.ad
! src/share/vm/opto/library_call.cpp
! src/share/vm/opto/memnode.cpp
+ test/compiler/6942326/Test.java

Changeset: 1b4e6a5d98e0
Author:twisti
Date:  2011-02-28 06:07 -0800
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/1b4e6a5d98e0

7012914: JSR 292 MethodHandlesTest C1: frame::verify_return_pc(return_address) 
failed: must be a return pc
Reviewed-by: never, bdelsart

! src/cpu/sparc/vm/c1_LIRAssembler_sparc.cpp
! src/cpu/sparc/vm/c1_Runtime1_sparc.cpp
! src/cpu/sparc/vm/methodHandles_sparc.cpp
! src/cpu/x86/vm/c1_LIRAssembler_x86.cpp
! src/cpu/x86/vm/c1_Runtime1_x86.cpp
! src/cpu/x86/vm/methodHandles_x86.cpp
! src/cpu/x86/vm/stubGenerator_x86_32.cpp
! src/share/vm/c1/c1_Runtime1.cpp
! src/share/vm/c1/c1_Runtime1.hpp
! src/share/vm/code/nmethod.cpp
! src/share/vm/code/nmethod.hpp
! src/share/vm/runtime/sharedRuntime.cpp
! src/share/vm/utilities/macros.hpp

Changeset: 50c0f22d6d0e
Author:never
Date:  2011-02-28 17:12 -0800
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/50c0f22d6d0e

7023229: extraneous include of precompiled.hpp in hsdis.c
Reviewed-by: never, jrose
Contributed-by: volker.simo...@gmail.com

! 

hg: jdk7/hotspot-rt/hotspot: 27 new changesets

[karen . kinnear]

Changeset: 5d8f5a6dced7
Author:iveresov
Date:  2011-03-04 15:14 -0800
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/5d8f5a6dced7

7020403: Add AdvancedCompilationPolicy for tiered
Summary: This implements adaptive tiered compilation policy.
Reviewed-by: kvn, never

! src/share/vm/oops/methodKlass.cpp
! src/share/vm/oops/methodOop.hpp
+ src/share/vm/runtime/advancedThresholdPolicy.cpp
+ src/share/vm/runtime/advancedThresholdPolicy.hpp
! src/share/vm/runtime/arguments.cpp
! src/share/vm/runtime/compilationPolicy.cpp

Changeset: 4cd9add59b1e
Author:never
Date:  2011-03-04 20:01 -0800
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/4cd9add59b1e

7024866: #  assert(limit == NULL || limit = nm-code_end()) failed: in bounds
Reviewed-by: kvn, iveresov

! src/share/vm/code/nmethod.cpp

Changeset: 8ec5e1f45ea1
Author:never
Date:  2011-03-04 22:44 -0800
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/8ec5e1f45ea1

Merge


Changeset: 8e72cd29b15d
Author:kvn
Date:  2011-03-05 11:02 -0800
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/8e72cd29b15d

6589823: Error: meet not symmetric
Summary: arrays pointers meet must fall to bottom if exact array klasses in 
upper lattice are not equal or super klass is exact.
Reviewed-by: never

! src/share/vm/opto/type.cpp

Changeset: 425688247f3d
Author:never
Date:  2011-03-06 22:09 -0800
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/425688247f3d

6965570: assert(!needs_patching  x-is_loaded(),how do we know it's volatile 
if it's not loaded)
Reviewed-by: iveresov

! src/share/vm/c1/c1_Canonicalizer.cpp
! src/share/vm/c1/c1_GraphBuilder.cpp
! src/share/vm/c1/c1_Instruction.hpp
! src/share/vm/c1/c1_LIRGenerator.cpp
! src/share/vm/c1/c1_ValueMap.hpp

Changeset: 1c0cf339481b
Author:kvn
Date:  2011-03-09 09:15 -0800
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/1c0cf339481b

7025742: Can not use CodeCache::unallocated_capacity() with fragmented CodeCache
Summary: Use largest_free_block() instead of unallocated_capacity().
Reviewed-by: iveresov, never, ysr

! src/share/vm/code/codeCache.cpp
! src/share/vm/code/codeCache.hpp
! src/share/vm/code/nmethod.cpp
! src/share/vm/compiler/compileBroker.cpp
! src/share/vm/memory/heap.cpp
! src/share/vm/opto/output.cpp
! src/share/vm/runtime/sweeper.cpp

Changeset: 83f08886981c
Author:kvn
Date:  2011-03-11 07:50 -0800
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/83f08886981c

7026631: field _klass is incorrectly set for dual type of TypeAryPtr::OOPS
Summary: add missing check this-dual() != TypeAryPtr::OOPS into 
TypeAryPtr::klass().
Reviewed-by: never

! src/share/vm/opto/type.cpp

Changeset: 799d8ccf63cf
Author:jrose
Date:  2011-03-11 21:19 -0800
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/799d8ccf63cf

Merge

! src/share/vm/oops/methodOop.hpp
! src/share/vm/runtime/arguments.cpp

Changeset: 72dee110246f
Author:jrose
Date:  2011-03-11 22:33 -0800
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/72dee110246f

6839872: remove implementation inheritance from JSR 292 APIs
Summary: consolidate runtime support in java.dyn.MethodHandleNatives; include 
transitional compatibility logic
Reviewed-by: twisti

! src/share/vm/classfile/classFileParser.cpp
! src/share/vm/classfile/javaClasses.cpp
! src/share/vm/classfile/systemDictionary.cpp
! src/share/vm/classfile/systemDictionary.hpp
! src/share/vm/classfile/vmSymbols.cpp
! src/share/vm/classfile/vmSymbols.hpp
! src/share/vm/interpreter/linkResolver.cpp
! src/share/vm/oops/instanceKlass.hpp
! src/share/vm/oops/methodOop.cpp
! src/share/vm/prims/methodHandleWalk.cpp
! src/share/vm/prims/methodHandles.cpp
! src/share/vm/prims/nativeLookup.cpp
! src/share/vm/runtime/globals.hpp

Changeset: 8033953d67ff
Author:jrose
Date:  2011-03-11 22:34 -0800
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/8033953d67ff

7012648: move JSR 292 to package java.lang.invoke and adjust names
Summary: package and class renaming only; delete unused methods and classes
Reviewed-by: twisti

! src/cpu/sparc/vm/assembler_sparc.cpp
! src/cpu/sparc/vm/cppInterpreter_sparc.cpp
! src/cpu/sparc/vm/interpreter_sparc.cpp
! src/cpu/sparc/vm/methodHandles_sparc.cpp
! src/cpu/sparc/vm/templateTable_sparc.cpp
! src/cpu/x86/vm/assembler_x86.cpp
! src/cpu/x86/vm/interpreter_x86_32.cpp
! src/cpu/x86/vm/interpreter_x86_64.cpp
! src/cpu/x86/vm/methodHandles_x86.cpp
! src/cpu/x86/vm/templateTable_x86_32.cpp
! src/cpu/x86/vm/templateTable_x86_64.cpp
! src/share/vm/c1/c1_LIR.hpp
! src/share/vm/c1/c1_LIRGenerator.cpp
! src/share/vm/ci/ciCallSite.cpp
! src/share/vm/ci/ciCallSite.hpp
! src/share/vm/ci/ciField.cpp
! src/share/vm/ci/ciMethod.cpp
! src/share/vm/ci/ciMethodHandle.hpp
! src/share/vm/ci/ciObjectFactory.cpp
! src/share/vm/ci/ciStreams.cpp
! 

hg: jdk7/hotspot-rt/hotspot: 4 new changesets

[karen . kinnear]

Changeset: 70b50ac7e2af
Author:cl
Date:  2011-03-10 17:10 -0800
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/70b50ac7e2af

Added tag jdk7-b133 for changeset 1b3a350709e4

! .hgtags

Changeset: 447e6faab4a8
Author:trims
Date:  2011-03-11 11:18 -0800
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/447e6faab4a8

Merge


Changeset: 02e6fc2effd8
Author:trims
Date:  2011-03-11 22:41 -0800
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/02e6fc2effd8

Merge


Changeset: 4775a1e3e923
Author:acorn
Date:  2011-03-14 11:43 -0700
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/4775a1e3e923

Merge

Re: SECOND CALL: code review for jvmstat/jps fix (6954420)

[Karen Kinnear]

Dan,

Approved.

Looks very tricky to track down.

Only minor suggestion - line 1361 warning: if ret_code == OS_ERR, it  
might be nice to
print %s and strerror(errno) rather than %d and ret_code. Not a big  
deal.


thanks,
Karen

On Feb 15, 2011, at 7:49 PM, Daniel D. Daugherty wrote:


I need a Runtime team reviewer!

Dan

On 2/14/2011 4:02 PM, Daniel D. Daugherty wrote:

Greetings,

I have a fix for the following jvmstat/jps bug:

6954420: 2/4 jps shows process information unavailable sometimes
Summary: Make sure the backing store file is flushed in  
create_sharedmem_resources() and get_user_name_slow() no longer  
checks the size of the backing store file.

Reviewed-by:

Here is the URL for the webrev:

  http://cr.openjdk.java.net/~dcubed/6954420-webrev/0/

Thanks, in advance, for any reviews.

Dan

Re: Request for review, 6766644: Redefinition of compiled method fails with assertion Can not load classes with the Compiler thread

[Karen Kinnear]

Currently to maintain the proper ordering of load and unload events, 
when you post
a compiled_method_load event, if there are pending 
compiled_method_unload events,

then we lock the nmethod to ensure that the nmethod is not flushed
or unloaded while posting the event. What if we did that until the 
compiled_method_load

event finished posting?

thanks,
Karen


On 01/25/11 14:00, Tom Rodriguez wrote:

On Jan 25, 2011, at 8:49 AM, Keith McGuigan wrote:

  

Hello,

This code modifies the way that JVMTI compiled-method-load events are posted.  Previously, they were posted directly from the compiler thread, which could cause issues if the JVMTI event handling code made calls to RedefineClasses, since the compiler thread is unable to load classes if that is required.  This solution is to defer the posting of the events to the Service thread (formerly: LowMemoryDetector 
thread) which is a Java thread and is able to load classes.



The posting appears to be asynchronous which I don't think will work.  The 
method could be unloaded or invalidated and freed before the event has been 
posted.  It has to be done synchronously I think.

I have a vague memory of someone saying that there were restrictions on what a 
JVMTI client was allowed to do in response to a CompiledMethodLoad but I can't 
find any evidence of that.  So is that just a bogus memory or is there some 
restriction like this?

tom

  

The Service thread now handles both low-memory detection and JVMTI deferred 
event posting.   I left the door open for other types of events to be deferred and posted 
via this mechanism in case we find other situations where posting events from a non-Java 
thread causes problems.

webrev:  http://cr.openjdk.java.net/~kamg/6766644/webrev.01/

--
- Keith



  

hg: jdk7/hotspot-rt/hotspot: 8 new changesets

[karen . kinnear]

Changeset: 4e22405d98d6
Author:iveresov
Date:  2010-10-19 11:14 -0700
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/4e22405d98d6

6989669: Coops: -Xshare:dump causes crash
Summary: Temporarily fix to disable compressed oops with CDS
Reviewed-by: dholmes, twisti, kvn, never

! src/share/vm/runtime/arguments.cpp
! src/share/vm/runtime/globals.hpp

Changeset: 68d6141ea19d
Author:cl
Date:  2010-10-07 15:12 -0700
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/68d6141ea19d

Added tag jdk7-b113 for changeset beef35b96b81

! .hgtags

Changeset: 52f19c724d96
Author:trims
Date:  2010-10-14 15:52 -0700
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/52f19c724d96

Merge

! .hgtags

Changeset: 570870354f86
Author:trims
Date:  2010-10-14 16:05 -0700
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/570870354f86

6992267: Bump the HS20 build number to 02
Summary: Update the HS20 build number to 02
Reviewed-by: jcoomes

! make/hotspot_version

Changeset: 477faa484f91
Author:cl
Date:  2010-10-14 19:24 -0700
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/477faa484f91

Added tag jdk7-b114 for changeset 68d6141ea19d

! .hgtags

Changeset: bdbc48857210
Author:trims
Date:  2010-10-20 16:49 -0700
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/bdbc48857210

Merge

! .hgtags

Changeset: 9eaf8ba53f3d
Author:trims
Date:  2010-10-20 17:07 -0700
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/9eaf8ba53f3d

Merge


Changeset: 60ce9dade348
Author:acorn
Date:  2010-10-26 14:43 -0400
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/60ce9dade348

Merge

hg: jdk7/hotspot-rt/hotspot: 2 new changesets

[karen . kinnear]

Changeset: fa83ab460c54
Author:acorn
Date:  2010-10-22 15:59 -0400
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/fa83ab460c54

6988353: refactor contended sync subsystem
Summary: reduce complexity by factoring synchronizer.cpp
Reviewed-by: dholmes, never, coleenp

- src/os/linux/vm/objectMonitor_linux.cpp
- src/os/linux/vm/objectMonitor_linux.hpp
- src/os/linux/vm/objectMonitor_linux.inline.hpp
- src/os/solaris/vm/objectMonitor_solaris.cpp
- src/os/solaris/vm/objectMonitor_solaris.hpp
- src/os/solaris/vm/objectMonitor_solaris.inline.hpp
- src/os/windows/vm/objectMonitor_windows.cpp
- src/os/windows/vm/objectMonitor_windows.hpp
- src/os/windows/vm/objectMonitor_windows.inline.hpp
! src/share/vm/includeDB_compiler1
! src/share/vm/includeDB_core
! src/share/vm/includeDB_features
! src/share/vm/includeDB_jvmti
! src/share/vm/prims/jvmtiImpl.cpp
! src/share/vm/prims/jvmtiImpl.hpp
+ src/share/vm/prims/jvmtiRawMonitor.cpp
+ src/share/vm/prims/jvmtiRawMonitor.hpp
+ src/share/vm/runtime/basicLock.cpp
+ src/share/vm/runtime/basicLock.hpp
! src/share/vm/runtime/mutex.hpp
+ src/share/vm/runtime/objectMonitor.cpp
! src/share/vm/runtime/objectMonitor.hpp
! src/share/vm/runtime/objectMonitor.inline.hpp
+ src/share/vm/runtime/park.cpp
+ src/share/vm/runtime/park.hpp
! src/share/vm/runtime/synchronizer.cpp
! src/share/vm/runtime/synchronizer.hpp
! src/share/vm/runtime/thread.cpp
! src/share/vm/runtime/thread.hpp

Changeset: a312a67b32ef
Author:acorn
Date:  2010-10-25 13:31 -0400
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/a312a67b32ef

Merge

! src/share/vm/includeDB_core

hg: jdk7/hotspot-rt/hotspot: 16 new changesets

[karen . kinnear]

Changeset: 75588558f1bf
Author:never
Date:  2010-10-07 21:40 -0700
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/75588558f1bf

6980792: Crash exception happened outside interpreter, nmethods and vtable 
stubs (1)
Reviewed-by: kvn

! src/cpu/sparc/vm/stubGenerator_sparc.cpp
! src/share/vm/opto/library_call.cpp
! src/share/vm/opto/loopTransform.cpp
! src/share/vm/opto/runtime.cpp

Changeset: a222fcfba398
Author:twisti
Date:  2010-10-08 02:42 -0700
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/a222fcfba398

6990549: Zero and Shark fixes after 6978355 and 6953144
Reviewed-by: twisti
Contributed-by: Gary Benson gben...@redhat.com

! src/cpu/zero/vm/interpreterRT_zero.hpp
! src/share/vm/code/nmethod.cpp
! src/share/vm/oops/methodOop.cpp
! src/share/vm/shark/sharkCompiler.hpp

Changeset: d55217dc206f
Author:twisti
Date:  2010-10-11 04:18 -0700
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/d55217dc206f

6829194: JSR 292 needs to support compressed oops
Reviewed-by: kvn, jrose

! src/cpu/sparc/vm/assembler_sparc.cpp
! src/cpu/sparc/vm/assembler_sparc.hpp
! src/cpu/sparc/vm/methodHandles_sparc.cpp
! src/cpu/sparc/vm/stubRoutines_sparc.hpp
! src/cpu/sparc/vm/templateTable_sparc.cpp
! src/cpu/x86/vm/assembler_x86.cpp
! src/cpu/x86/vm/assembler_x86.hpp
! src/cpu/x86/vm/methodHandles_x86.cpp
! src/cpu/x86/vm/stubRoutines_x86_64.hpp
! src/cpu/x86/vm/templateTable_x86_32.cpp
! src/cpu/x86/vm/templateTable_x86_64.cpp
! src/share/vm/asm/codeBuffer.hpp
! src/share/vm/ci/ciInstanceKlass.cpp
! src/share/vm/ci/ciTypeFlow.cpp
! src/share/vm/classfile/classFileParser.cpp
! src/share/vm/oops/oop.inline.hpp

Changeset: a932f331ef90
Author:twisti
Date:  2010-10-12 02:21 -0700
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/a932f331ef90

6991065: missed a review comment in 6829194
Reviewed-by: kvn

! src/share/vm/classfile/classFileParser.cpp

Changeset: c393f046f4c5
Author:iveresov
Date:  2010-10-12 23:51 -0700
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/c393f046f4c5

6991512: G1 barriers fail with 64bit C1
Summary: Fix compare-and-swap intrinsic problem with G1 post-barriers and issue 
with branch ranges in G1 stubs on sparc
Reviewed-by: never, kvn

! src/cpu/sparc/vm/assembler_sparc.hpp
! src/cpu/sparc/vm/c1_CodeStubs_sparc.cpp
! src/cpu/sparc/vm/c1_LIRGenerator_sparc.cpp
! src/cpu/x86/vm/c1_LIRAssembler_x86.cpp
! src/cpu/x86/vm/c1_LIRGenerator_x86.cpp
! src/share/vm/c1/c1_LIRGenerator.cpp

Changeset: 5beba6174298
Author:twisti
Date:  2010-10-13 01:19 -0700
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/5beba6174298

6987555: JSR 292 unboxing to a boolean value fails on big-endian SPARC
Reviewed-by: never, jrose

! src/cpu/sparc/vm/methodHandles_sparc.cpp
! src/share/vm/prims/methodHandles.cpp
! src/share/vm/prims/methodHandles.hpp
+ test/compiler/6987555/Test6987555.java

Changeset: ecca2e3e2767
Author:twisti
Date:  2010-10-13 13:31 -0700
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/ecca2e3e2767

Merge


Changeset: 357451a9ae6a
Author:roland
Date:  2010-10-13 10:29 +0200
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/357451a9ae6a

6991211: assert failure on sparc: can not have caller-save register operands 
at calls
Summary: fixes sparc only assert failure following 6972540
Reviewed-by: never

! src/cpu/sparc/vm/c1_LinearScan_sparc.hpp

Changeset: 94d77a279225
Author:roland
Date:  2010-10-13 15:38 -0700
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/94d77a279225

Merge


Changeset: b98784e85f71
Author:kvn
Date:  2010-10-14 10:46 -0700
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/b98784e85f71

Merge


Changeset: c32059ef4dc0
Author:johnc
Date:  2010-10-12 09:36 -0700
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/c32059ef4dc0

6971296: G1: simplify G1RemSet class hierarchy
Summary: Remove G1RemSet base class and StupidG1RemSet class; rename 
HRInto_G1RemSet to just G1RemSet.
Reviewed-by: ysr, tonyp

! src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp
! src/share/vm/gc_implementation/g1/g1OopClosures.hpp
! src/share/vm/gc_implementation/g1/g1RemSet.cpp
! src/share/vm/gc_implementation/g1/g1RemSet.hpp
! src/share/vm/gc_implementation/g1/g1RemSet.inline.hpp
! src/share/vm/gc_implementation/g1/g1_globals.hpp
! src/share/vm/gc_implementation/includeDB_gc_g1

Changeset: b14ec34b1e07
Author:jcoomes
Date:  2010-10-12 11:29 -0700
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/b14ec34b1e07

6989448: G1: refactor and simplify G1ParScanThreadState
Reviewed-by: iveresov, tonyp

! src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp
! src/share/vm/gc_implementation/g1/g1CollectedHeap.hpp

Changeset: ee813f7b46e4
Author:jcoomes
Date:  2010-10-14 11:57 -0700
URL:   

hg: jdk7/hotspot-rt/hotspot: 2 new changesets

[karen . kinnear]

Changeset: b96a3e44582f
Author:acorn
Date:  2010-06-03 13:21 -0400
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/b96a3e44582f

6852873: Reduce safepoint cleanup time
Summary: New optional flags to reduce inflated monitor cleanup times
Reviewed-by: chrisphi, dice

! src/share/vm/runtime/globals.hpp
! src/share/vm/runtime/synchronizer.cpp
! src/share/vm/runtime/synchronizer.hpp
! src/share/vm/runtime/thread.cpp
! src/share/vm/runtime/thread.hpp

Changeset: be0d50d3de2a
Author:acorn
Date:  2010-06-03 13:34 -0400
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/be0d50d3de2a

Merge

Re: URGENT code review request for Win* UnhandledExceptionFilter fix (6918421)

[Karen Kinnear]

Dan,

Looks good - thanks for fixing this. If you have time, could you  
possibly

fix mothod-method on line 1976?

thanks,
Karen

On Feb 1, 2010, at 9:08 PM, Tom Rodriguez wrote:


Seems ok.

tom

On Feb 1, 2010, at 4:31 PM, Daniel D. Daugherty wrote:


Greetings,

I have an urgent code review request for a Win* fix. Basically,
I'm making a minor tweak to Ivan's original fix for the following
bug:

 6550813 5/3 Crash because of FPU control word being modified
 by native code (win32)

Here's the URL for my tweak:

 http://cr.openjdk.java.net/~dcubed/6918421-webrev/0/

The fix passes the customer test attached to 6918421 and
it passes the test associated with Ivan's fix for 6550813.

I'm targeting this fix for HSX-17-B09; since this is a P1 it
will be backported to JDK6-UNN...

Thanks, in advance, for any reviews.

Dan

hg: jdk7/hotspot-rt/hotspot: 2 new changesets

[karen . kinnear]

Changeset: a75edfd400ea
Author:acorn
Date:  2009-11-11 15:49 -0500
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/a75edfd400ea

6893504: LinkageError for bootstrap duplicate class definitions.
Reviewed-by: kamg, xlu

! src/share/vm/classfile/systemDictionary.cpp
! src/share/vm/classfile/systemDictionary.hpp

Changeset: 1920bd911283
Author:acorn
Date:  2009-11-23 16:24 -0500
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/1920bd911283

Merge

! src/share/vm/classfile/systemDictionary.cpp
! src/share/vm/classfile/systemDictionary.hpp

hg: jdk7/hotspot-rt/hotspot: 3 new changesets

[karen . kinnear]

Changeset: c3c4a1d3801a
Author:andrew
Date:  2009-09-23 11:36 -0700
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/c3c4a1d3801a

6884552: remove some unnecessary #ifdef's introduced in the fix for 4957990
Summary: Removed the unnecessary #ifdef's which were interfering with the build 
of the Zero-assembler port
Reviewed-by: ysr, jcoomes

! src/share/vm/gc_implementation/parallelScavenge/psParallelCompact.cpp
! src/share/vm/gc_implementation/shared/markSweep.cpp

Changeset: 1af62b6ca0f9
Author:apetrusenko
Date:  2009-09-25 04:39 -0700
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/1af62b6ca0f9

Merge


Changeset: 054afbef9081
Author:acorn
Date:  2009-09-28 12:27 -0400
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/054afbef9081

Merge

hg: jdk7/hotspot-rt/hotspot: 2 new changesets

[karen . kinnear]

Changeset: ad6585fd4087
Author:acorn
Date:  2009-09-04 12:53 -0400
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/ad6585fd4087

6830542: Performance: JVM_DefineClass already verified.
Reviewed-by: kamg, phh

! make/linux/makefiles/mapfile-vers-debug
! make/linux/makefiles/mapfile-vers-product
! make/solaris/makefiles/mapfile-vers
! src/share/vm/classfile/classFileParser.cpp
! src/share/vm/classfile/classFileParser.hpp
! src/share/vm/classfile/classLoader.cpp
! src/share/vm/classfile/systemDictionary.cpp
! src/share/vm/classfile/systemDictionary.hpp
! src/share/vm/classfile/verifier.cpp
! src/share/vm/classfile/verifier.hpp
! src/share/vm/code/dependencies.cpp
! src/share/vm/oops/instanceKlass.cpp
! src/share/vm/oops/instanceKlass.hpp
! src/share/vm/oops/instanceKlassKlass.cpp
! src/share/vm/prims/jni.cpp
! src/share/vm/prims/jvm.cpp
! src/share/vm/prims/jvm.h
! src/share/vm/prims/jvmtiRedefineClasses.cpp
! src/share/vm/runtime/vmStructs.cpp

Changeset: 26b774d693aa
Author:acorn
Date:  2009-09-16 09:10 -0400
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/26b774d693aa

Merge

! src/share/vm/classfile/classFileParser.cpp
! src/share/vm/classfile/classFileParser.hpp
- src/share/vm/gc_implementation/shared/coTracker.cpp
- src/share/vm/gc_implementation/shared/coTracker.hpp
- src/share/vm/gc_implementation/shared/gcOverheadReporter.cpp
- src/share/vm/gc_implementation/shared/gcOverheadReporter.hpp
! src/share/vm/oops/instanceKlass.cpp
! src/share/vm/oops/instanceKlass.hpp
! src/share/vm/oops/instanceKlassKlass.cpp

hg: jdk7/hotspot-rt/hotspot: 6879572: SA fails _is_marked_dependent not found

[karen . kinnear]

Changeset: 83c29a26f67c
Author:acorn
Date:  2009-09-16 15:42 -0400
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/83c29a26f67c

6879572: SA fails _is_marked_dependent not found
Reviewed-by: kamg, dcubed

! src/share/vm/classfile/classFileParser.cpp
! src/share/vm/code/dependencies.cpp
! src/share/vm/oops/instanceKlass.hpp
! src/share/vm/oops/instanceKlassKlass.cpp
! src/share/vm/runtime/vmStructs.cpp

hg: jdk7/hotspot-rt/hotspot: 14 new changesets

[karen . kinnear]

Changeset: b1f5ced5da21
Author:jcoomes
Date:  2009-09-03 19:21 -0700
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/b1f5ced5da21

6879076: disable jprt sync after builds are done
Reviewed-by: kamg, dholmes

! make/jprt.properties

Changeset: 68ef3fdcdb76
Author:ysr
Date:  2009-09-10 16:46 -0700
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/68ef3fdcdb76

6872136: CMS: confusing message may be printed when a collector is switched off 
implicitly
Summary: Fix CDS/CMS option overrides related to iCMS option 
CMSIncrementalMode; explicate overrides to error stream.
Reviewed-by: coleenp

! src/share/vm/runtime/arguments.cpp

Changeset: 489a4f8dcd0f
Author:twisti
Date:  2009-08-27 06:17 -0700
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/489a4f8dcd0f

6865583: Verbose CIPrintMethodCodes asserts when ldc an empty String
Summary: ldc seems to load an empty String and that leads to an assert on 
offset  length, which are both zero.
Reviewed-by: kvn, never

! src/share/vm/classfile/javaClasses.cpp

Changeset: 8fe1963e3964
Author:kvn
Date:  2009-08-28 11:19 -0700
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/8fe1963e3964

6875577: CTW fails with /hotspot/src/share/vm/opto/memnode.cpp
Summary: Fix do_null_check to check for unloaded klass for all oop pointers.
Reviewed-by: never, cfang

! src/share/vm/opto/graphKit.cpp
! src/share/vm/opto/library_call.cpp

Changeset: 1fbd5d696bf4
Author:twisti
Date:  2009-08-31 02:24 -0700
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/1fbd5d696bf4

6875967: CTW fails with./generated/adfiles/ad_sparc.cpp:6711
Reviewed-by: cfang, never

! src/cpu/sparc/vm/sparc.ad

Changeset: ace8397c8563
Author:cfang
Date:  2009-08-31 08:31 -0700
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/ace8397c8563

6876276: assert(!is_visited,visit only once)
Summary: schedule the superword loads based on dependence constraints
Reviewed-by: kvn, never

! src/share/vm/opto/superword.cpp
! test/compiler/6636138/Test1.java
! test/compiler/6636138/Test2.java

Changeset: ff1a29907b6c
Author:never
Date:  2009-08-31 17:07 -0700
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/ff1a29907b6c

6855215: Calculation error (NaN) after about 1500 calculations
Reviewed-by: kvn

! src/cpu/x86/vm/c1_LIRGenerator_x86.cpp
! src/cpu/x86/vm/c1_LinearScan_x86.cpp
! src/share/vm/c1/c1_LIR.cpp
! src/share/vm/c1/c1_LIR.hpp
+ test/compiler/6855215/Test6855215.java

Changeset: 0f1c19b7a52d
Author:kvn
Date:  2009-09-08 10:42 -0700
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/0f1c19b7a52d

6875619: CTW fails with /hotspot/src/share/vm/opto/type.hpp
Summary: In load_array_length() cast array's type to TypeOopPtr when calling 
make_ideal_length() method.
Reviewed-by: never

! src/share/vm/opto/graphKit.cpp

Changeset: 26fbe81d30cf
Author:kvn
Date:  2009-09-08 16:56 -0700
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/26fbe81d30cf

6880052: SIGSEGV in GraphKit::null_check_common()
Summary: Check that a klass is not NULL before the is_loaded() call.
Reviewed-by: never

! src/share/vm/opto/graphKit.cpp

Changeset: 9a4e87ba1a90
Author:kvn
Date:  2009-09-09 16:28 -0700
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/9a4e87ba1a90

6880533: test/compiler/6865031/Test.java miss -XX:+IgnoreUnrecognizedVMOptions
Summary: Add missing test option -XX:+IgnoreUnrecognizedVMOptions.
Reviewed-by: never

! test/compiler/6865031/Test.java

Changeset: 159d56b94894
Author:kvn
Date:  2009-09-10 10:36 -0700
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/159d56b94894

6880574: C2 assert in escape.cpp:445 on linux-amd64
Summary: Look through chained AddP nodes in get_addp_base().
Reviewed-by: jrose

! src/share/vm/opto/escape.cpp

Changeset: c7e94e8fff43
Author:kvn
Date:  2009-09-10 18:18 -0700
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/c7e94e8fff43

6880053: assert(alloc_obj-as_CheckCastPP()-type() != TypeInstPtr::NOTNULL)
Summary: Removed second CheckCastPP and use MembarCPUOrder after arraycopy to 
cloned object.
Reviewed-by: never

! src/share/vm/opto/library_call.cpp
! src/share/vm/opto/type.cpp
! src/share/vm/opto/type.hpp

Changeset: a6f533fc33e0
Author:kvn
Date:  2009-09-14 11:45 -0700
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/a6f533fc33e0

Merge


Changeset: e5b31fd85b72
Author:acorn
Date:  2009-09-15 16:28 -0400
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/e5b31fd85b72

Merge

hg: jdk7/hotspot-rt/hotspot: 10 new changesets

[karen . kinnear]

Changeset: 05f89f00a864
Author:jmasa
Date:  2009-08-24 10:36 -0700
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/05f89f00a864

6798898: CMS: bugs related to class unloading
Summary: Override should_remember_klasses() and remember_klass() as needed.
Reviewed-by: ysr, jcoomes

! src/share/vm/gc_implementation/concurrentMarkSweep/cmsOopClosures.hpp
! src/share/vm/gc_implementation/concurrentMarkSweep/cmsOopClosures.inline.hpp
! 
src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp
! 
src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.hpp
! src/share/vm/gc_implementation/includeDB_gc_concurrentMarkSweep
! src/share/vm/memory/iterator.cpp
! src/share/vm/memory/iterator.hpp
! src/share/vm/memory/referenceProcessor.cpp

Changeset: e1fdf4fd34dc
Author:tonyp
Date:  2009-08-19 12:53 -0400
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/e1fdf4fd34dc

687: G1: remove the concurrent overhead tracker
Summary: Removing the concurrent overhead tracker from G1, along with the GC 
overhead reporter and the G1AccountConcurrentOverhead (both of which rely on 
the the concurrent overhead tracker).
Reviewed-by: iveresov, johnc

! src/share/vm/gc_implementation/g1/concurrentG1RefineThread.cpp
! src/share/vm/gc_implementation/g1/concurrentG1RefineThread.hpp
! src/share/vm/gc_implementation/g1/concurrentMark.cpp
! src/share/vm/gc_implementation/g1/concurrentMark.hpp
! src/share/vm/gc_implementation/g1/concurrentMarkThread.cpp
! src/share/vm/gc_implementation/g1/concurrentZFThread.cpp
! src/share/vm/gc_implementation/g1/concurrentZFThread.hpp
! src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp
! src/share/vm/gc_implementation/g1/g1CollectorPolicy.cpp
! src/share/vm/gc_implementation/g1/g1CollectorPolicy.hpp
! src/share/vm/gc_implementation/g1/g1MMUTracker.cpp
! src/share/vm/gc_implementation/g1/g1MMUTracker.hpp
! src/share/vm/gc_implementation/g1/g1_globals.hpp
! src/share/vm/gc_implementation/includeDB_gc_g1
! src/share/vm/gc_implementation/includeDB_gc_shared
- src/share/vm/gc_implementation/shared/coTracker.cpp
- src/share/vm/gc_implementation/shared/coTracker.hpp
- src/share/vm/gc_implementation/shared/gcOverheadReporter.cpp
- src/share/vm/gc_implementation/shared/gcOverheadReporter.hpp

Changeset: ead53f6b615d
Author:tonyp
Date:  2009-08-24 13:52 -0700
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/ead53f6b615d

Merge

- src/share/vm/gc_implementation/shared/coTracker.cpp
- src/share/vm/gc_implementation/shared/coTracker.hpp
- src/share/vm/gc_implementation/shared/gcOverheadReporter.cpp
- src/share/vm/gc_implementation/shared/gcOverheadReporter.hpp

Changeset: b37c246bf7ce
Author:jcoomes
Date:  2009-08-11 15:37 -0700
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/b37c246bf7ce

6861660: OopMapBlock count/size confusion
Reviewed-by: tonyp, iveresov

! src/share/vm/classfile/classFileParser.cpp
! src/share/vm/classfile/classFileParser.hpp
! src/share/vm/memory/oopFactory.cpp
! src/share/vm/memory/oopFactory.hpp
! src/share/vm/oops/instanceKlass.cpp
! src/share/vm/oops/instanceKlass.hpp
! src/share/vm/oops/instanceKlassKlass.cpp
! src/share/vm/oops/instanceKlassKlass.hpp
! src/share/vm/oops/instanceRefKlass.cpp

Changeset: 9eebd3ac74cf
Author:jcoomes
Date:  2009-08-13 16:22 -0700
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/9eebd3ac74cf

6845368: large objects cause a crash or unexpected exception
Reviewed-by: jmasa, iveresov

! src/share/vm/classfile/classFileParser.cpp
! src/share/vm/classfile/classFileParser.hpp
! src/share/vm/memory/oopFactory.cpp
! src/share/vm/memory/oopFactory.hpp
! src/share/vm/oops/instanceKlass.cpp
! src/share/vm/oops/instanceKlass.hpp
! src/share/vm/oops/instanceKlassKlass.cpp
! src/share/vm/oops/instanceKlassKlass.hpp
! src/share/vm/oops/instanceRefKlass.cpp
+ test/gc/6845368/bigobj.java

Changeset: 8624da129f0b
Author:apetrusenko
Date:  2009-08-31 05:27 -0700
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/8624da129f0b

6841313: G1: dirty cards of survivor regions in parallel
Reviewed-by: tonyp, iveresov

! src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp
! src/share/vm/gc_implementation/g1/g1_globals.hpp
! src/share/vm/memory/cardTableModRefBS.cpp
! src/share/vm/memory/cardTableModRefBS.hpp

Changeset: 8b46c4d82093
Author:ysr
Date:  2009-09-02 00:04 -0700
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/8b46c4d82093

4957990: Perm heap bloat in JVM
Summary: Treat ProfileData in MDO's as a source of weak, not strong, roots. 
Fixes the bug for stop-world collection -- the case of concurrent collection 
will be fixed separately.
Reviewed-by: jcoomes, jmasa, kvn, never

! src/share/vm/code/nmethod.cpp
! src/share/vm/code/nmethod.hpp
! src/share/vm/gc_implementation/concurrentMarkSweep/cmsOopClosures.hpp
! 

hg: jdk7/hotspot-rt/hotspot: 6825642: nsk sajdi tests fail with NullPointerException

[karen . kinnear]

Changeset: 23276f80d930
Author:acorn
Date:  2009-04-02 14:26 -0400
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/23276f80d930

6825642: nsk sajdi tests fail with NullPointerException
Reviewed-by: xlu, coleenp, kamg, swamyv

! agent/src/share/classes/sun/jvm/hotspot/HotSpotTypeDataBase.java
! agent/src/share/classes/sun/jvm/hotspot/runtime/VM.java
! src/share/vm/runtime/vmStructs.cpp

hg: jdk7/hotspot-rt/langtools: 4 new changesets

[karen . kinnear]

Changeset: 889ec3ddc91b
Author:tbell
Date:  2009-03-17 11:28 -0700
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/langtools/rev/889ec3ddc91b

6814592: Legal notice repair needed in langtools/test/tools/javap/T4884240.java
Reviewed-by: jjg

! test/tools/javap/T4884240.java

Changeset: edd944553131
Author:bpatel
Date:  2009-03-19 19:00 -0700
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/langtools/rev/edd944553131

6786688: Javadoc HTML WCAG 2.0 accessibility issues in standard doclet - Table 
must have captions and headers
Reviewed-by: jjg

! src/share/classes/com/sun/tools/doclets/formats/html/AbstractMemberWriter.java
! 
src/share/classes/com/sun/tools/doclets/formats/html/AbstractPackageIndexWriter.java
! 
src/share/classes/com/sun/tools/doclets/formats/html/AnnotationTypeOptionalMemberWriterImpl.java
! 
src/share/classes/com/sun/tools/doclets/formats/html/AnnotationTypeRequiredMemberWriterImpl.java
! src/share/classes/com/sun/tools/doclets/formats/html/ClassUseWriter.java
! 
src/share/classes/com/sun/tools/doclets/formats/html/ConstantsSummaryWriterImpl.java
! 
src/share/classes/com/sun/tools/doclets/formats/html/ConstructorWriterImpl.java
! src/share/classes/com/sun/tools/doclets/formats/html/DeprecatedListWriter.java
! 
src/share/classes/com/sun/tools/doclets/formats/html/EnumConstantWriterImpl.java
! src/share/classes/com/sun/tools/doclets/formats/html/FieldWriterImpl.java
! src/share/classes/com/sun/tools/doclets/formats/html/HtmlDocletWriter.java
! src/share/classes/com/sun/tools/doclets/formats/html/MethodWriterImpl.java
! 
src/share/classes/com/sun/tools/doclets/formats/html/NestedClassWriterImpl.java
! 
src/share/classes/com/sun/tools/doclets/formats/html/PackageIndexFrameWriter.java
! src/share/classes/com/sun/tools/doclets/formats/html/PackageIndexWriter.java
! src/share/classes/com/sun/tools/doclets/formats/html/PackageUseWriter.java
! src/share/classes/com/sun/tools/doclets/formats/html/PackageWriterImpl.java
! src/share/classes/com/sun/tools/doclets/formats/html/StylesheetWriter.java
! 
src/share/classes/com/sun/tools/doclets/formats/html/SubWriterHolderWriter.java
! src/share/classes/com/sun/tools/doclets/formats/html/markup/HtmlWriter.java
! 
src/share/classes/com/sun/tools/doclets/formats/html/resources/standard.properties
! 
src/share/classes/com/sun/tools/doclets/internal/toolkit/PackageSummaryWriter.java
! 
src/share/classes/com/sun/tools/doclets/internal/toolkit/builders/PackageSummaryBuilder.java
! 
src/share/classes/com/sun/tools/doclets/internal/toolkit/resources/doclets.properties
! test/com/sun/javadoc/testHeadings/TestHeadings.java
! test/com/sun/javadoc/testHtmlStrongTag/TestHtmlStrongTag.java
+ test/com/sun/javadoc/testHtmlTableTags/TestHtmlTableTags.java
+ test/com/sun/javadoc/testHtmlTableTags/pkg1/C1.java
+ test/com/sun/javadoc/testHtmlTableTags/pkg1/I1.java
+ test/com/sun/javadoc/testHtmlTableTags/pkg1/package-info.java
+ test/com/sun/javadoc/testHtmlTableTags/pkg2/C2.java
+ test/com/sun/javadoc/testHtmlTableTags/pkg2/C3.java
+ test/com/sun/javadoc/testHtmlTableTags/pkg2/C4.java
+ test/com/sun/javadoc/testHtmlTableTags/pkg2/package-info.java
! test/com/sun/javadoc/testNewLanguageFeatures/TestNewLanguageFeatures.java
! test/com/sun/javadoc/testSummaryHeading/TestSummaryHeading.java

Changeset: b000f7c728ae
Author:bpatel
Date:  2009-03-20 15:50 -0700
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/langtools/rev/b000f7c728ae

6820360: Fix for definition list tags nesting adds an extra list tag for 
package summary page.
Reviewed-by: jjg

! src/share/classes/com/sun/tools/doclets/formats/html/HtmlDocletWriter.java
! test/com/sun/javadoc/testHtmlDefinitionListTag/TestHtmlDefinitionListTag.java
+ test/com/sun/javadoc/testHtmlDefinitionListTag/pkg1/package-info.java

Changeset: 3bf905cb80e7
Author:tbell
Date:  2009-03-21 13:53 -0700
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/langtools/rev/3bf905cb80e7

Merge

hg: jdk7/hotspot-rt/hotspot: 4766230: Hotspot vtable inconsistencies cause core dumps. 6579515. 6582242.

[karen . kinnear]

Changeset: 4aaa9f5e02a8
Author:acorn
Date:  2009-03-18 17:20 -0400
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/4aaa9f5e02a8

4766230: Hotspot vtable inconsistencies cause core dumps. 6579515. 6582242.
Reviewed-by: kamg, coleenp

! src/share/vm/classfile/classFileParser.cpp
! src/share/vm/oops/instanceKlass.cpp
! src/share/vm/oops/instanceKlass.hpp
! src/share/vm/oops/klassVtable.cpp
! src/share/vm/oops/klassVtable.hpp

hg: jdk7/hotspot-rt/hotspot: 24 new changesets

[karen . kinnear]

Changeset: c6c601a0f2d6
Author:ysr
Date:  2009-03-02 16:37 -0800
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/c6c601a0f2d6

6797870: Add -XX:+{HeapDump,PrintClassHistogram}{Before,After}FullGC
Summary: Call newly created CollectedHeap::dump_{pre,post}_full_gc before and 
after every stop-world full collection cycle on GenCollectedHeap and 
ParallelScavengeHeap. (Support for G1CollectedHeap forthcoming under CR 
6810861.) Small modifications to existing heap dumping and class histogram 
implementation, especially to allow multiple on-the-fly histos/dumps by the VM 
thread during a single safepoint.
Reviewed-by: jmasa, alanb, mchung

! src/share/vm/gc_implementation/parallelScavenge/psMarkSweep.cpp
! src/share/vm/gc_implementation/parallelScavenge/psParallelCompact.cpp
! src/share/vm/gc_implementation/shared/vmGCOperations.cpp
! src/share/vm/gc_implementation/shared/vmGCOperations.hpp
! src/share/vm/gc_interface/collectedHeap.cpp
! src/share/vm/gc_interface/collectedHeap.hpp
! src/share/vm/includeDB_gc
! src/share/vm/memory/genCollectedHeap.cpp
! src/share/vm/memory/heapInspection.cpp
! src/share/vm/memory/heapInspection.hpp
! src/share/vm/runtime/globals.hpp
! src/share/vm/runtime/os.cpp
! src/share/vm/services/attachListener.cpp
! src/share/vm/services/heapDumper.cpp
! src/share/vm/services/heapDumper.hpp

Changeset: 4f360ec815ba
Author:iveresov
Date:  2009-03-06 13:50 -0800
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/4f360ec815ba

6720309: G1: don't synchronously update RSet during evacuation pauses
6720334: G1: don't update RSets of collection set regions during an evacuation 
pause
Summary: Introduced a deferred update mechanism for delaying the rset updates 
during the collection pause
Reviewed-by: apetrusenko, tonyp

! src/share/vm/gc_implementation/g1/concurrentG1RefineThread.cpp
! src/share/vm/gc_implementation/g1/dirtyCardQueue.cpp
! src/share/vm/gc_implementation/g1/dirtyCardQueue.hpp
! src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp
! src/share/vm/gc_implementation/g1/g1CollectedHeap.hpp
! src/share/vm/gc_implementation/g1/g1RemSet.cpp
! src/share/vm/gc_implementation/g1/g1RemSet.hpp
! src/share/vm/gc_implementation/g1/g1RemSet.inline.hpp
! src/share/vm/gc_implementation/g1/g1_globals.hpp
! src/share/vm/gc_implementation/g1/ptrQueue.cpp
! src/share/vm/gc_implementation/g1/ptrQueue.hpp
! src/share/vm/memory/cardTableModRefBS.cpp
! src/share/vm/memory/cardTableModRefBS.hpp

Changeset: 0db4adb6e914
Author:tonyp
Date:  2009-03-07 11:07 -0500
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/0db4adb6e914

6810698: G1: two small bugs in the sparse remembered sets
Summary: The _expanded flag of the sparse RSets is not reset and this can leave 
a RSet in an inconsistent state if it is expanded more than once. Also, we 
should be iterating over the _cur, instead of the _next, sparse table
Reviewed-by: apetrusenko, iveresov

! src/share/vm/gc_implementation/g1/sparsePRT.cpp
! src/share/vm/gc_implementation/g1/sparsePRT.hpp

Changeset: ae1579717a57
Author:tonyp
Date:  2009-03-07 11:07 -0500
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/ae1579717a57

6812428: G1: Error: assert(ret || obj_in_cs(obj),sanity)
Summary: The length of the fast cset test vector is decided at the beginning of 
a GC, but more regions can be added during the GC. The simple fix is to set the 
length of the fast cset test vector to the max.
Reviewed-by: iveresov

! src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp

Changeset: 7ea5ca260b28
Author:tonyp
Date:  2009-03-07 11:07 -0500
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/7ea5ca260b28

6814467: G1: small fixes related to concurrent marking verboseness
Summary: A few small fixes to remove some inconsistencies in the concurrent 
mark-related verbose GC output.
Reviewed-by: jmasa

! src/share/vm/gc_implementation/g1/concurrentMark.cpp
! src/share/vm/gc_implementation/g1/concurrentMarkThread.cpp
! src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp

Changeset: bcedf688d882
Author:tonyp
Date:  2009-03-09 11:32 -0400
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/bcedf688d882

Merge

! src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp
! src/share/vm/runtime/os.cpp

Changeset: 19f25e603e7b
Author:kvn
Date:  2009-03-03 18:25 -0800
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/19f25e603e7b

6812721: Block's frequency should not be NaN
Summary: Set MIN_BLOCK_FREQUENCY block's frequency when calculated block's 
frequency is NaN
Reviewed-by: never

! src/share/vm/opto/gcm.cpp

Changeset: 56aae7be60d4
Author:jrose
Date:  2009-03-04 09:58 -0800
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/56aae7be60d4

6812678: macro assembler needs delayed binding of a few constants (for 6655638)
Summary: minor assembler enhancements preparing for method handles
Reviewed-by: