Re: JEP 158 support for JIT

[Christian Thalinger]

> On Jan 2, 2017, at 6:08 PM, Yasumasa Suenaga  wrote:
> 
> Thanks Vladimir,
> 
>> Definitely not in JDK 9. And I can't say when it could be done or done at 
>> all.
> 
> I hope this feature will be implemented ASAP.

You can always contribute an implementation.  Shouldn’t be too difficult.

> 
> 
> Yasumasa
> 
> 
> On 2017/01/03 12:00, Vladimir Kozlov wrote:
>> On 1/2/17 6:33 PM, Yasumasa Suenaga wrote:
>>> Hi all,
>>> 
>>> Java 9 has JEP 158: Unified JVM Logging.
>>> This JEP describes that existing 'tty->print...' logging should use unified 
>>> logging as output. However, C2 compiler does
>>> not seem to use it.
>>> 
>>> Do you have any plan to use JEP 158 in JIT codes?
>> 
>> Definitely not in JDK 9. And I can't say when it could be done or done at 
>> all.
>> 
>> Regards,
>> Vladimir
>> 
>>> 
>>> I uploaded Unified JVM logging viewer to GitHub [1].
>>> I want to draw chart(s) or list all JIT'ed methods on it if possible.
>>> (Especially I want to get log from PrintCompilation and PrintIntrinsics 
>>> through Unified JVM logging)
>>> 
>>> 
>>> Thanks,
>>> 
>>> Yasumasa
>>> 
>>> 
>>> [1] https://github.com/YaSuenag/ulviewer
>>> 

Re: RFR[9u-dev]: 8150900: Implement diagnostic_pd

[Christian Thalinger]

src/share/vm/runtime/globals.hpp

-  develop_pd(bool, ImplicitNullChecks,  \
+  diagnostic_pd(bool, ImplicitNullChecks,  
\
   "Generate code for implicit null checks") \
Align the \

> On May 10, 2016, at 1:47 AM, Cheleswer Sahu  wrote:
> 
> Hi, 
> I need one reviewer (R) to review these changes before pushing in JDK9.  Can 
> somebody please review the changes.
> 
> Regards,
> Cheleswer
> 
>> -Original Message-
>> From: Kevin Walls
>> Sent: Friday, May 06, 2016 3:53 PM
>> To: Cheleswer Sahu; Gerard Ziemski
>> Cc: serviceability-dev@openjdk.java.net; hotspot-runtime-
>> d...@openjdk.java.net
>> Subject: Re: RFR[9u-dev]: 8150900: Implement diagnostic_pd
>> 
>> 
>> Thanks Cheleswer, looks good to me too, have been over the macros as
>> much as I can!
>> 
>> Thanks
>> Kevin
>> 
>> 
>> 
>> On 03/05/2016 07:34, Cheleswer Sahu wrote:
>>> Hi Gerard,
>>> 
>>> 
 -Original Message-
 From: Gerard Ziemski
 Sent: Monday, May 02, 2016 9:07 PM
 To: Cheleswer Sahu
 Cc: hotspot-runtime-...@openjdk.java.net; serviceability-
 d...@openjdk.java.net
 Subject: Re: RFR[9u-dev]: 8150900: Implement diagnostic_pd
 
 hi Cheleswer,
 
 
 #1 Shouldn’t the following files be modified as well? :
 
 open:
 
 src/cpu/sparc/vm/globals_sparc.hpp
 src/cpu/x86/vm/globals_x86.hpp
 src/cpu/zero/vm/globals_zero.hpp
 
 closed:
 
 cpu/arm/vm/globals_arm.hpp
>>> I have implemented  "diagnostic_pd" using "product_pd" as a reference
>> implementation. "product_pd" is not implemented for " ARCH_FLAGS ",
>> therefore I have also not implemented "diagnostic_pd" for "ARCH_FLAGS"
>> type.
>>> 
 share/vm/runtime/globals_ext.hpp
 share/vm/runtime/os_ext.hpp
>>> These 2 files are under closed repository, so I have initiated a separate
>> internal review request for those changes.
>>> 
 
 #2 Bunch of header files need to be updated with 2016 for Copyright:
 
 /*
 - * Copyright (c) 2011, 2015, Oracle and/or its affiliates. All rights 
 reserved.
 + * Copyright (c) 2011, 2016, Oracle and/or its affiliates. All rights 
 reserved.
  * ORACLE PROPRIETARY/CONFIDENTIAL.  Use is subject to license terms.
  */
 
 
>>> I agree, I will update the copyright headers.
>>> 
 #3 What tests have you run? Did you do:
 
 a) JPRT hotspot
 b) RBT hs-nightly-runtime
 
>>> I have run JPRT hostspot tests for this. It shows no error.
>>> 
 Please email me if you need help with those.
 
 
 #4 Just heads up that I will be shortly asking for review of my fix
 (https://bugs.openjdk.java.net/browse/JDK-8073500), which touches
 many of the same file, so one of us will have a tricky merge
 
>>> Thanks for informing about this.
>>> 
>>> 
>>> Regards,
>>> Cheleswer
>>> 
 cheers
 
> On May 2, 2016, at 4:51 AM, Cheleswer Sahu
  wrote:
> Hi,
> 
> 
> 
> Please review the code changes for
 https://bugs.openjdk.java.net/browse/JDK-8150900.
> 
> 
> Webrev Link: http://cr.openjdk.java.net/~csahu/8150900/webrev.00/
> 
> 
> 
> Enhancement Brief:  A new variant of flag "diagnostic_pd" is
>> implemented.
 All flags which are diagnostic in nature and platform dependent can
 be placed
> under this variant. These flags can be enable using  "-
 XX:+UnlockDiagnosticVMOptions".
> At present I have placed 4 flags under "diagnostic_pd"
> 
> 1.1. InitArrayShortSize
> 
> 2.2. ImplicitNullChecks
> 
> 3.3. InlineFrequencyCount
> 
> 4.4. PostLoopMultiversioning
> 
> 
> 
> 
> 
> Regards,
> 
> Cheleswer
>> 

Re: RFR(S): 8071999: SA's buildreplayjars fail with exception

[Christian Thalinger]

Since you’re saying it only fixes it partially should we file a followup bug 
and maybe leave a comment behind?

 On Feb 4, 2015, at 7:04 AM, Roland Westrelin roland.westre...@oracle.com 
 wrote:
 
 
 buildreplayjars sometimes fail with an exception and I think that's
 caused by the lack of support for default methods when the SA dumps
 classes. This change fixes it at least partially. It may not be the
 cleanest way to support default methods but it already proved useful
 when investigating a compiler crash so unless someone has a better fix
 I would like to push it.
 
 http://cr.openjdk.java.net/~roland/8071999/webrev.00/
 
 Roland.

Re: RFR: 8059625 - JEP-JDK-8043304: Test task: DTrace- tests for segmented codecache feature

[Christian Thalinger]

  40 public static int[] getAvailableCompilationLevels() {
  41 if (System.getProperty(java.vm.info).startsWith(interpreted )) 
{
  42 return new int[0];
  43 }

Wouldn’t it be better to check the UseCompiler flag instead?

 On Dec 19, 2014, at 11:03 AM, Dmitrij Pochepko dmitrij.poche...@oracle.com 
 wrote:
 
 Hi all,
 
 Please review changes for https://bugs.openjdk.java.net/browse/JDK-8059625 -  
 JEP-JDK-8043304: Test task: DTrace- tests for segmented codecache feature
 
 Description: this fix introduce dtrace test, which verify that different 
 combinations of available compile levels(and, in case compile levels allows 
 it, different code heaps as result)  doesn't affect callstack shown by 
 dtrace. There is a control class SegmentedCodeCacheDtraceTest.java and class 
 for running via dtrace SegmentedCodeCacheDtraceTestWorker.java. A dtrace d 
 script is also present (SegmentedCodeCacheDtraceTestScript.d). A control 
 class is using DtraceRunner.java to run dtrace and then analyzing results 
 using class SegmentedCodeCacheDtraceResultsAnalyzer with 
 DtraceResultsAnalyzer interface.
 There is also a small class CompilerUtils.java created for usefull common 
 code.
 
 webrev: http://cr.openjdk.java.net/~iignatyev/dpochepk/8059625/webrev.00/
 
 Additional note: Please note that this path assumes that fix for JDK-8066440 
 - Various changes in testlibrary for JDK-8059613 is also applied.
 
 Thanks,
 Dmitrij

Re: [9] RFR (XS): JSR 292 support for PopFrame has a fragile coupling with DirectMethodHandle

[Christian Thalinger]

Looks good.  Would be good to put a FIXME on the comment as well so we don’t 
forget to remove it.

On May 26, 2014, at 12:25 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:

 https://bugs.openjdk.java.net/browse/JDK-8034935
 http://cr.openjdk.java.net/~vlivanov/8034935/webrev.00
 
 Citing John's explanation of motivation for this change (from the bug):
 There is a coupling from bytecodes that call MethodHandle.linkToStatic (and 
 similar linker methods) and the PopFrame feature. The linker methods accept 
 an extra appendix argument of type MemberName which is popped from the list 
 before vectoring to the desired method (it supplies the name of that method).
 
 In order to re-execute the call, the MemberName must be recovered somehow. 
 Currently, the JVM assumes that the interpreter frame's local #0 will contain 
 a DirectMethodHandle which holds the desired MemberName. The JVM should also 
 accept the MemberName itself, and eventually stop looking for the 
 DirectMethodHandle.
 
 This will simplify the handshake between JVM and JSR 292 implementation 
 bytecodes. The DMH is difficult to recover at the point of call to 
 linkToStatic, although the MemberName is guaranteed live at that point.
 
 Also, making this change (perhaps in two steps) will allow the JVM to stop 
 coupling to SystemDictionary::DirectMethodHandle_klass. Such couplings should 
 be minimized.
 
 Testing: JPRT, jtreg (java/lang/invoke), vm.mlvm.testlist
 
 Thanks!
 
 Best regards,
 Vladimir Ivanov

Re: RFR: 8041934 com/sun/jdi/RepStep.java fails on all platforms with assert(_cur_stack_depth == count_frames()) failed: cur_stack_depth out of sync

[Christian Thalinger]

Looks good.

On May 13, 2014, at 11:58 PM, Staffan Larsen staffan.lar...@oracle.com wrote:

 Thanks Christian,
 
 I will make the change below before I push.
 
 /Staffan
 
 
 diff --git a/src/cpu/x86/vm/sharedRuntime_x86_32.cpp 
 b/src/cpu/x86/vm/sharedRuntime_x86_32.cpp
 --- a/src/cpu/x86/vm/sharedRuntime_x86_32.cpp
 +++ b/src/cpu/x86/vm/sharedRuntime_x86_32.cpp
 @@ -2248,7 +2248,7 @@
  // if we are now in interp_only_mode and in that case we do the jvmti
  // callback.
  Label skip_jvmti_method_exit;
 -__ cmpb(Address(thread, JavaThread::interp_only_mode_offset()), 0);
 +__ cmpl(Address(thread, JavaThread::interp_only_mode_offset()), 0);
  __ jcc(Assembler::zero, skip_jvmti_method_exit, true);
 
  save_native_result(masm, ret_type, stack_slots);
 diff --git a/src/cpu/x86/vm/sharedRuntime_x86_64.cpp 
 b/src/cpu/x86/vm/sharedRuntime_x86_64.cpp
 --- a/src/cpu/x86/vm/sharedRuntime_x86_64.cpp
 +++ b/src/cpu/x86/vm/sharedRuntime_x86_64.cpp
 @@ -2495,7 +2495,7 @@
  // if we are now in interp_only_mode and in that case we do the jvmti
  // callback.
  Label skip_jvmti_method_exit;
 -__ cmpb(Address(r15_thread, JavaThread::interp_only_mode_offset()), 0);
 +__ cmpl(Address(r15_thread, JavaThread::interp_only_mode_offset()), 0);
  __ jcc(Assembler::zero, skip_jvmti_method_exit, true);
 
  save_native_result(masm, ret_type, stack_slots);
 
 
 On 14 maj 2014, at 00:21, Christian Thalinger 
 christian.thalin...@oracle.com wrote:
 
 Since:
 
   int   _interp_only_mode;
 
 is an int field I would prefer to actually read the value as an int instead 
 of just a byte on x86:
 +__ cmpb(Address(r15_thread, JavaThread::interp_only_mode_offset()), 0);
 Otherwise this looks good.
 
 On May 13, 2014, at 11:30 AM, Staffan Larsen staffan.lar...@oracle.com 
 wrote:
 
 
 On 13 maj 2014, at 18:53, Daniel D. Daugherty daniel.daughe...@oracle.com 
 wrote:
 
  new webrev is here: http://cr.openjdk.java.net/~sla/8041934/webrev.02/
 
 src/share/vm/runtime/sharedRuntime.hpp
 No comments.
 
 src/share/vm/runtime/sharedRuntime.cpp
 No comments.
 
 src/cpu/sparc/vm/sharedRuntime_sparc.cpp
 No comments.
 
 src/cpu/x86/vm/sharedRuntime_x86_32.cpp
 No comments.
 
 src/cpu/x86/vm/sharedRuntime_x86_64.cpp
 No comments.
 
 On the switch from call_VM_leaf() - call_VM(), I looked through all
 the mentions of the string call_VM in the three src/cpu files. Your
 change adds the first call_VM() use in all three files and the other
 places that mention the string call_VM seem to have gone through
 a great deal of trouble not to use call_VM() directly. I have no
 specific thing I think is wrong, but I find this data worrisome…
 
 Yes, it would be great if someone from the compiler team could review this, 
 too.
 
 Thanks,
 /Staffan
 
 
 Thumbs up!
 
 Dan
 
 
 On 5/13/14 3:20 AM, Staffan Larsen wrote:
 
 On 9 maj 2014, at 20:18, serguei.spit...@oracle.com wrote:
 
 Staffan,
 
 This is important discovery, thanks!
 The fix looks good to me.
 One question below.
 
 Thanks,
 Serguei
 
 
 On 5/9/14 3:47 AM, Staffan Larsen wrote:
 On 8 maj 2014, at 19:05, Daniel D. Daugherty 
 daniel.daughe...@oracle.com wrote:
 
 webrev: http://cr.openjdk.java.net/~sla/8041934/webrev.00/
 src/share/vm/runtime/sharedRuntime.hpp
No comments.
 
 src/share/vm/runtime/sharedRuntime.cpp
line 994: JRT_LEAF(int, SharedRuntime::jvmti_method_exit(
I'm not sure that JRT_LEAF is right. I would think that
JvmtiExport::post_method_exit() would have to grab one or
more locks with all the state checks it has to make...
 
For reference, InterpreterRuntime::post_method_exit()
is a wrapper around JvmtiExport::post_method_exit()
and it is IRT_ENTRY instead of IRT_LEAF.
 Good catch!
 
 Q: Is correct to use call_VM_leaf (vs call_VM ) in the  
 sharedRuntime_arch.cpp ?
 
 +__ call_VM_leaf(
 + CAST_FROM_FN_PTR(address, SharedRuntime::jvmti_method_exit),
 + thread, rax);
 
 That is another good catch! It should probably be call_VM as you note. 
 The reason for all these leaf references is because we used the dtrace 
 probe as a template - obviously without fully understanding what we did 
 :-/
 
 I have changed to code to use call_VM instead. This also required a 
 change from jccb to jcc for the jump (which is now longer than an 8-bit 
 offset). 
 
 new webrev is here: http://cr.openjdk.java.net/~sla/8041934/webrev.02/
 
 Thanks,
 /Staffan
 
 
 
 src/cpu/sparc/vm/sharedRuntime_sparc.cpp
No comments.
 
 src/cpu/x86/vm/sharedRuntime_x86_32.cpp
No comments.
 
 src/cpu/x86/vm/sharedRuntime_x86_64.cpp
No comments.
 
 I'm guessing that PPC has the same issue, but I'm presuming
 that someone else (Vladimir?) will handle that…
 Yes, I was hoping that I could file a follow-up bug for the platforms I 
 didn’t know how to fix.
 
 Updated review: http://cr.openjdk.java.net/~sla/8041934/webrev.01/
 
 Thanks,
 /Staffan
 
 Dan
 
 
 On 5

Re: RFR: 8041934 com/sun/jdi/RepStep.java fails on all platforms with assert(_cur_stack_depth == count_frames()) failed: cur_stack_depth out of sync

[Christian Thalinger]

Since:

  int   _interp_only_mode;

is an int field I would prefer to actually read the value as an int instead of 
just a byte on x86:
+__ cmpb(Address(r15_thread, JavaThread::interp_only_mode_offset()), 0);
Otherwise this looks good.

On May 13, 2014, at 11:30 AM, Staffan Larsen staffan.lar...@oracle.com wrote:

 
 On 13 maj 2014, at 18:53, Daniel D. Daugherty daniel.daughe...@oracle.com 
 wrote:
 
  new webrev is here: http://cr.openjdk.java.net/~sla/8041934/webrev.02/
 
 src/share/vm/runtime/sharedRuntime.hpp
 No comments.
 
 src/share/vm/runtime/sharedRuntime.cpp
 No comments.
 
 src/cpu/sparc/vm/sharedRuntime_sparc.cpp
 No comments.
 
 src/cpu/x86/vm/sharedRuntime_x86_32.cpp
 No comments.
 
 src/cpu/x86/vm/sharedRuntime_x86_64.cpp
 No comments.
 
 On the switch from call_VM_leaf() - call_VM(), I looked through all
 the mentions of the string call_VM in the three src/cpu files. Your
 change adds the first call_VM() use in all three files and the other
 places that mention the string call_VM seem to have gone through
 a great deal of trouble not to use call_VM() directly. I have no
 specific thing I think is wrong, but I find this data worrisome…
 
 Yes, it would be great if someone from the compiler team could review this, 
 too.
 
 Thanks,
 /Staffan
 
 
 Thumbs up!
 
 Dan
 
 
 On 5/13/14 3:20 AM, Staffan Larsen wrote:
 
 On 9 maj 2014, at 20:18, serguei.spit...@oracle.com wrote:
 
 Staffan,
 
 This is important discovery, thanks!
 The fix looks good to me.
 One question below.
 
 Thanks,
 Serguei
 
 
 On 5/9/14 3:47 AM, Staffan Larsen wrote:
 On 8 maj 2014, at 19:05, Daniel D. Daugherty 
 daniel.daughe...@oracle.com wrote:
 
 webrev: http://cr.openjdk.java.net/~sla/8041934/webrev.00/
 src/share/vm/runtime/sharedRuntime.hpp
No comments.
 
 src/share/vm/runtime/sharedRuntime.cpp
line 994: JRT_LEAF(int, SharedRuntime::jvmti_method_exit(
I'm not sure that JRT_LEAF is right. I would think that
JvmtiExport::post_method_exit() would have to grab one or
more locks with all the state checks it has to make...
 
For reference, InterpreterRuntime::post_method_exit()
is a wrapper around JvmtiExport::post_method_exit()
and it is IRT_ENTRY instead of IRT_LEAF.
 Good catch!
 
 Q: Is correct to use call_VM_leaf (vs call_VM ) in the  
 sharedRuntime_arch.cpp ?
 
 +__ call_VM_leaf(
 + CAST_FROM_FN_PTR(address, SharedRuntime::jvmti_method_exit),
 + thread, rax);
 
 That is another good catch! It should probably be call_VM as you note. The 
 reason for all these leaf references is because we used the dtrace probe as 
 a template - obviously without fully understanding what we did :-/
 
 I have changed to code to use call_VM instead. This also required a change 
 from jccb to jcc for the jump (which is now longer than an 8-bit offset). 
 
 new webrev is here: http://cr.openjdk.java.net/~sla/8041934/webrev.02/
 
 Thanks,
 /Staffan
 
 
 
 src/cpu/sparc/vm/sharedRuntime_sparc.cpp
No comments.
 
 src/cpu/x86/vm/sharedRuntime_x86_32.cpp
No comments.
 
 src/cpu/x86/vm/sharedRuntime_x86_64.cpp
No comments.
 
 I'm guessing that PPC has the same issue, but I'm presuming
 that someone else (Vladimir?) will handle that…
 Yes, I was hoping that I could file a follow-up bug for the platforms I 
 didn’t know how to fix.
 
 Updated review: http://cr.openjdk.java.net/~sla/8041934/webrev.01/
 
 Thanks,
 /Staffan
 
 Dan
 
 
 On 5/8/14 12:06 AM, Staffan Larsen wrote:
 All,
 
 This is a fix for an assert in JVMTI that verifies that JVMTI’s 
 internal notion of the number of frames on the stack is correct.
 
 When running in -Xcomp mode and enable single-stepping (or 
 method_entry/exit) we will revert all frames on the stack to be run by 
 the interpreter. Only the interpreter can send single-step and 
 method_entry/exit. However, if one of the frames is a native wrapper, 
 that frame will not be reverted (presumably because we don't know how 
 to do that). This will cause us to miss a method_exit event when that 
 native frame is popped. This in turn will mess up the logic in JVMTI 
 that keeps track of the number of frames currently on the stack (see 
 JvmtiThreadState::_cur_stack_depth) and will trigger the assert.
 
 The proposed solution is to include a method_exit event in the native 
 wrapper frame if interpreted mode has been enabled. This needs updates 
 to SharedRuntime::generate_native_wrapper() for all platforms.
 
 Kudos to Rickard for helping me write the code.
 
 webrev: http://cr.openjdk.java.net/~sla/8041934/webrev.00/
 bug: https://bugs.openjdk.java.net/browse/JDK-8041934
 
 The fix has been verified by running the failing test in JPRT with 
 -Xcomp.
 
 Thanks,
 /Staffan
 
 
 
 

Re: RFR 6471769: Error: assert(_cur_stack_depth == count_frames(), cur_stack_depth out of sync)

[Christian Thalinger]

Looks good.

On Feb 25, 2014, at 12:43 PM, serguei.spit...@oracle.com wrote:

 Please, review the fix for:
  https://bugs.openjdk.java.net/browse/JDK-6471769
 
 
 Open webrev:
 http://cr.openjdk.java.net/~sspitsyn/webrevs/2014/hotspot/6471769-JVMTI-DEPTH.1
 
 Summary:
 
  This is another Test Stabilization issue.
  The fix is very similar to other JVMTI stabilization fixes.
  It is to use safepoints for updating the PopFrame data instead of relying on 
 the
  suspend equivalent condition mechanism 
 (JvmtiEnv::is_thread_fully_suspended())
  which is not adequate from the reliability point of view.
 
 Testing:
  In progress: nsk.jvmti, nsk.jdi, nsk.jdwp, JTreg com/sun/jdi
 
 
 Thanks,
 Serguei
 

Re: RFR (XS) 6471769: Error: assert(_cur_stack_depth == count_frames(), cur_stack_depth out of sync)

[Christian Thalinger]

I cannot comment on the performance impact but the change looks good.

On Jan 31, 2014, at 6:58 PM, serguei.spit...@oracle.com wrote:

 Please, review the fix for:
  https://bugs.openjdk.java.net/browse/JDK-6471769
 
 
 Open webrev:
 http://cr.openjdk.java.net/~sspitsyn/webrevs/2014/hotspot/6471769-JVMTI-FRAME/
 
 Summary:
 
  There is a general issue in the suspend equivalent condition mechanism:
  Two subsequent calls to the JvmtiEnv::is_thread_fully_suspended() may return 
 different results:
- 1-st: true
- 2-nd: false
 
  This more generic suspend equivalent issue is covered by another bug:
https://bugs.openjdk.java.net/browse/JDK-6280037
 
  The bug to fix in this review is a specific manifestation of the 6280037
  in the JVMTI GetFrameCount() that has a big impact on the SQE nightly.
  It is on the Test Stabilization radar (as well as the 6280037).
  There are many tests intermittently failing because of this.
 
  The webrev for review is a one-liner work around the 6280037 for the 
 GetFrameCount().
 
  The JVMTI GetFrameCount() spec tells:
If this function is called for a thread actively executing bytecodes (for 
 example,
 not the current thread and not suspended), the information returned is 
 transient.
 
  So, it is Ok to call the GetFrameCount() for non-suspended target threads.
  To achieve safety, the frame count for non-suspended threads is calculated 
 at a safepoint.
  It should be Ok and more safe to do the same for suspended threads as well.
  There is no big performance impact because it is already on a slow path.
  It is still important to avoid safepointing when the target thread is 
 current.
 
  The bug 6280037 should go out of the Test Stabilization radar (remove the 
 svc-nightly label)
  as the most of the impacted tests are covered by the 6471769.
 
 
 Testing:
  In progress: nsk.jvmti, nsk.jdi, nsk.jdwp, impacted JTreg tests
 
 
 Thanks,
 Serguei
 

Re: RFR 8019389: SA-JDI JSR292: sun.jvm.hotspot.jdi.StackFrame.thisObject() throws sun.jvm.hotspot.utilities.AssertionFailure: sanity check

[Christian Thalinger]

Looks good.

On Jan 21, 2014, at 12:41 AM, Olivier Lagneau olivier.lagn...@oracle.com 
wrote:

 Please find the new webrev with copyright date fixed (changed to 2014).
 
 Webrev: http://cr.openjdk.java.net/~olagneau/8019389/webrev.01/
 
 Olivier.
 
 Olivier Lagneau said  on date 1/20/2014 5:50 PM:
 
 Oops, right !
 
 Will fix that.
 
 Olivier.
 
 shanliang said  on date 1/20/2014 4:13 PM:
 
 Olivier,
 
 Now it is 2014 :)
 
 
 Olivier Lagneau wrote:
 
 Please review the following simple fix.
 
 Issue: https://bugs.openjdk.java.net/browse/JDK-8019389
 Webrev: http://cr.openjdk.java.net/~olagneau/8019389/webrev.00/
 
 The issue is due to the fact that _invokeHandle bytecode is generated by 
 hotspot,
 but is not declared in agent code. Just declaring the new bytecode solves 
 the assertion failure.
 
 However the tests reported in 8019389 (bootstrapOtherStratumInStackTrace, 
 targetOtherStratumInStackTrace)
 suffer the problem from  JDK-7016268 : Can't get strata information 
 through SA-JDI
 Thus, the stratum mismatch related to JDK-7016268 will still be present 
 after fix.
 This second problem has to be fixed through JDK-7016268.
 
 Thanks,
 Olivier.
 
 
 

Re: RFR(L): 8028468: Add inlining information into ciReplay

[Christian Thalinger]

Why are we using a different file for the inline data?

On Dec 27, 2013, at 6:15 PM, Vladimir Kozlov vladimir.koz...@oracle.com wrote:

 http://cr.openjdk.java.net/~kvn/8028468/webrev/
 
 https://bugs.openjdk.java.net/browse/JDK-8028468
 
 Add inlining information into Compilation Replay data to correctly inline 
 methods during recompilation.
 Add ability to use Replay data to replay compilation DURING normal execution 
 of a program (in debug VM only because ciReplay is defined only in debug VM). 
 Currently a program is not executed when we do recompilation.
 Allow dump and replay inlining for specified method during a program 
 execution.
 
 Thanks,
 Vladimir
 

hg: jdk8/tl/jdk: 8026502: java/lang/invoke/MethodHandleConstants.java fails on all platforms

[christian . thalinger]

Changeset: 66884b270b44
Author:twisti
Date:  2013-10-24 10:52 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/66884b270b44

8026502: java/lang/invoke/MethodHandleConstants.java fails on all platforms
Reviewed-by: iveresov, jrose

! src/share/classes/java/lang/invoke/MethodHandles.java

Re: RR(S): JDK-8015848: SA: ByteCodeRewriter needs implementation for invokedynamic

[Christian Thalinger]

On Sep 5, 2013, at 12:31 AM, Dmitry Samersoff dmitry.samers...@oracle.com 
wrote:

 On 2013-09-05 02:30, Christian Thalinger wrote:
 
 On Sep 4, 2013, at 1:35 PM, Dmitry Samersoff dmitry.samers...@oracle.com 
 wrote:
 
 Please, review the changes.
 
 http://cr.openjdk.java.net/~dsamersoff/JDK-8015848/webrev.01/
 
 With this change you can dump the boot class path classes again?
 
 Yes. Also I manually checked generated classes with javap.
 
 But I'll much appreciate you if you can apply the patch and confirm that
 output is OK from compiler team point of view.

Sorry.  I missed this email.  Let me give it a try today.

-- Chris

 
 -Dmitry
 
 -- 
 Dmitry Samersoff
 Oracle Java development team, Saint Petersburg, Russia
 * I would love to change the world, but they won't give me the source code.

Re: RR(S): JDK-8015848: SA: ByteCodeRewriter needs implementation for invokedynamic

[Christian Thalinger]

Yes, the dumping works but when I try to use the boot.jar file it complains 
about missing BootstrapMethods attributes:

$ /java/re/jdk/8/promoted/all/b95/binaries/linux-amd64/fastdebug/bin/java 
-XX:+ReplayCompiles -XX:ReplayDataFile=replay_pid26233.log 
-Xbootclasspath/p:boot.jar -cp app.jar 
Error occurred during initialization of VM
java/lang/ClassFormatError: Missing BootstrapMethods attribute in class file 
java/lang/CharSequence

Unfortunately I was expecting that.

-- Chris

On Sep 11, 2013, at 10:39 AM, Dmitry Samersoff dmitry.samers...@oracle.com 
wrote:

 Thanks!
 
 
 On 2013-09-11 20:37, Christian Thalinger wrote:
 
 On Sep 5, 2013, at 12:31 AM, Dmitry Samersoff dmitry.samers...@oracle.com 
 wrote:
 
 On 2013-09-05 02:30, Christian Thalinger wrote:
 
 On Sep 4, 2013, at 1:35 PM, Dmitry Samersoff dmitry.samers...@oracle.com 
 wrote:
 
 Please, review the changes.
 
 http://cr.openjdk.java.net/~dsamersoff/JDK-8015848/webrev.01/
 
 With this change you can dump the boot class path classes again?
 
 Yes. Also I manually checked generated classes with javap.
 
 But I'll much appreciate you if you can apply the patch and confirm that
 output is OK from compiler team point of view.
 
 Sorry.  I missed this email.  Let me give it a try today.
 
 -- Chris
 
 
 -Dmitry
 
 -- 
 Dmitry Samersoff
 Oracle Java development team, Saint Petersburg, Russia
 * I would love to change the world, but they won't give me the source code.
 
 
 
 -- 
 Dmitry Samersoff
 Oracle Java development team, Saint Petersburg, Russia
 * I would love to change the world, but they won't give me the sources.

Re: RR(S): JDK-8015848: SA: ByteCodeRewriter needs implementation for invokedynamic

[Christian Thalinger]

On Sep 4, 2013, at 1:35 PM, Dmitry Samersoff dmitry.samers...@oracle.com 
wrote:

 Please, review the changes.
 
 http://cr.openjdk.java.net/~dsamersoff/JDK-8015848/webrev.01/

With this change you can dump the boot class path classes again?

-- Chris

 
 -Dmitry
 
 -- 
 Dmitry Samersoff
 Oracle Java development team, Saint Petersburg, Russia
 * I would love to change the world, but they won't give me the sources.

Re: RFR: 8020962: dump loaded java classes when vm crash

[Christian Thalinger]

On Aug 12, 2013, at 8:43 PM, Yumin Qi yumin...@oracle.com wrote:

 Coleen,
 
  Chris Thalinger  already answered some of your concerns. For jar file which 
 dumped in this change, compiler replay has added functions to SA to dump out 
 replay jars against core file. Since customer is the one wants us to fix 
 problems in VM, the disclosure of such jar to us should be OK I think, any 
 way if we have core file, we already have the loaded classes.
  This should mainly be focused on dump jars as possible at the last stage of 
 error reporter. I chose using SA since with c++ to implement same 
 functionality I have to use zip file operation to do so, which is not a fit 
 at such condition, but in SA the same operation ready for use. The only case 
 we could not get jar files is no core file available, in such case, this 
 change will supply an alternate.
 
  Looks this need more discussion for further decision.

One more comment (just FTR):  the class dumping should only happen when we 
crash in one of the compiler threads; not always.

-- Chris

 
 Thanks
 Yumin
 
 
 On 8/12/2013 2:43 PM, Coleen Phillimore wrote:
 
 Yumin,
 
 I don't think this change should be added to the JVM for the following 
 reasons.
 
 1. Error handling should only contain safe actions.   We have concerns that 
 the SA is not that stable and would prevent getting a real core file in many 
 error situations.   You couldn't have tested all error situations because 
 these are usually the things we don't know about.   You also mention that 
 there are currently bugs preventing this feature from working in your first 
 email.
 
 2. I am not convinced that having a separate jar file with loaded classes 
 (is it a list that SA generates?) would be collected by an error reporter.   
 If it's collected, I don't see how helpful it would be.  Also a customer may 
 not want their classes disclosed in error reporting.
 
 3.  This doesn't seem important enough to include as a new feature in jdk8 
 release, which is feature-complete.   I don't see a customer request for it.
 
 Coleen
 
 On 08/12/2013 01:00 PM, Yumin Qi wrote:
 Chris, David
 
  Yes. This can be after crash with core file, but some time no core file 
 generated (also if the error could not repeat, we will never got 
 information again), so dumping  target process is also a choice. To avoid 
 more confusion, this step was launched at the very last moment, just before 
 raise abort to exit. At this moment, if client process could not attach to 
 the target process it will exit right away.
 
 Answers to David:
 
 Note that the SA is only present in a full JDK, not a JRE (full or compact 
 profile).
  Yes, in code, if no sa-jdi.jar found, skip fork.
 
 - What mechanism will the SA try to use to query the VM?
 
  After successfully attached to target process, SA will read via proc APIs 
 and vmStructs (named TYPEDB) to iterate  memory of system dictionary read 
 (only)  from target process to dump class information.
 
 - What if the state of the crashed VM stops the SA from being able to 
 attach properly (ie both processes hang)?
 
  That should be an attaching API problem. We haven't watched such case 
 happened so far.
 
 - What if it the SA also crashes, will it launch a third VM then a fourth 
 etc?
 
  Definitely don't want to see this happened in a chain. The solution may 
 use a property such as 
 sun.jvm.hotspot.DumpLoadedClasses.dumpingInProcess=true to pass into SA 
 process, at launching call, check if the property set, if set, do not fork. 
 When SA process died, it will generate core file first, note the target 
 process still waiting for its exit, so when target exit, the core file (if 
 both use default core as name) will be override by target. The SA process 
 will only leave a hs_err_pid*.log file. (? read such property in handler is 
 possible?)
 
 Also what is the nature of this dump? How big is it? Where will it go?
 The jars includes *app.jar and *boot.jar, the later usually can be ignored 
 (rt.jar etc system jars) unless it's rewritten by JVMTI agent. The app.jar 
 will contain all classes by customer, so we can do whatever we can to the 
 jar.
 
 
 Thanks
 Yumin
 
 
 On 8/12/2013 5:51 AM, Christian Tornqvist wrote:
 Hi Yumin,
 
 The idea is to do as little as possible in the VM error handler, since 
 we've crashed for some reason we don't know what state the process is in 
 and we have to be extremely careful in when we're gathering the 
 information. This seems like a step that is risky for all of the reasons 
 David mentioned below.
 
 It's also information that can easily be extracted post-mortem from the 
 core/mdmp using the method you described for OSX, so gathering this at the 
 time of a crash seems like an unnecessary risk.
 
 Thanks,
 Christian
 
 -Original Message-
 From: hotspot-compiler-dev-boun...@openjdk.java.net 
 [mailto:hotspot-compiler-dev-boun...@openjdk.java.net] On Behalf Of David 
 Holmes
 Sent: Monday, August 12, 2013 1:56 AM
 To: 

Re: RFR: 8020962: dump loaded java classes when vm crash

[Christian Thalinger]

On Aug 12, 2013, at 10:17 PM, Christian Tornqvist 
christian.tornqv...@oracle.com wrote:

 Hi Chris,
 
 The SA logic would be exercised during our internal testing and could be
 fixed before the release.  Right now the situation is we see a customer
 crash, we want to run the SA and notice that it's broken.  That's too late.
 
 This is an issue with our current testing of the SA. This should be
 addressed by making sure our test works and covers the functionality of the
 SA

I hope someone is doing this…

-- Chris

 , not by making the JVM dependent on the SA.
 
 Thanks,
 Christian
 
 -Original Message-
 From: hotspot-runtime-dev-boun...@openjdk.java.net
 [mailto:hotspot-runtime-dev-boun...@openjdk.java.net] On Behalf Of Christian
 Thalinger
 Sent: Monday, August 12, 2013 9:12 PM
 To: Coleen Phillimore
 Cc: serviceability-dev@openjdk.java.net serviceability-dev@openjdk.java.net;
 'hotspot compiler'; hotspot-runtime-...@openjdk.java.net; 'David Holmes'
 Subject: Re: RFR: 8020962: dump loaded java classes when vm crash
 
 
 On Aug 12, 2013, at 2:43 PM, Coleen Phillimore
 coleen.phillim...@oracle.com wrote:
 
 
 Yumin,
 
 I don't think this change should be added to the JVM for the following
 reasons.
 
 This change is something that I (kind of) requested from Yumin since it
 would help to reproduce crashes in the compilers.  Getting replay data after
 the fact can be very tricky (given all the system library interactions or a
 broken SA; see below).
 
 
 1. Error handling should only contain safe actions.   We have concerns
 that the SA is not that stable
 
 .which is a problem, I agree.  To make it more stable we need to use it so
 we can detect problems early.  
 
 Having the SA dump data when we crash in the compiler can be one of these
 usages.  The SA logic would be exercised during our internal testing and
 could be fixed before the release.  Right now the situation is we see a
 customer crash, we want to run the SA and notice that it's broken.  That's
 too late.
 
 and would prevent getting a real core file in many error situations.   You
 couldn't have tested all error situations because these are usually the
 things we don't know about.   You also mention that there are currently bugs
 preventing this feature from working in your first email.
 
 2. I am not convinced that having a separate jar file with loaded classes
 (is it a list that SA generates?) would be collected by an error reporter.
 If it's collected, I don't see how helpful it would be.
 
 As mentioned above, it's required to replay compilations.
 
 Also a customer may not want their classes disclosed in error reporting.
 
 They don't have to if they don't want to.
 
 -- Chris
 
 
 3.  This doesn't seem important enough to include as a new feature in jdk8
 release, which is feature-complete.   I don't see a customer request for it.
 
 Coleen
 
 On 08/12/2013 01:00 PM, Yumin Qi wrote:
 Chris, David
 
 Yes. This can be after crash with core file, but some time no core file
 generated (also if the error could not repeat, we will never got information
 again), so dumping  target process is also a choice. To avoid more
 confusion, this step was launched at the very last moment, just before raise
 abort to exit. At this moment, if client process could not attach to the
 target process it will exit right away.
 
 Answers to David:
 
 Note that the SA is only present in a full JDK, not a JRE (full or
 compact profile).
 Yes, in code, if no sa-jdi.jar found, skip fork.
 
 - What mechanism will the SA try to use to query the VM?
 
 After successfully attached to target process, SA will read via proc
 APIs and vmStructs (named TYPEDB) to iterate  memory of system dictionary
 read (only)  from target process to dump class information.
 
 - What if the state of the crashed VM stops the SA from being able to
 attach properly (ie both processes hang)?
 
 That should be an attaching API problem. We haven't watched such case
 happened so far.
 
 - What if it the SA also crashes, will it launch a third VM then a fourth
 etc?
 
 Definitely don't want to see this happened in a chain. The solution 
 may use a property such as 
 sun.jvm.hotspot.DumpLoadedClasses.dumpingInProcess=true to pass into 
 SA process, at launching call, check if the property set, if set, do 
 not fork. When SA process died, it will generate core file first, 
 note the target process still waiting for its exit, so when target 
 exit, the core file (if both use default core as name) will be 
 override by target. The SA process will only leave a hs_err_pid*.log 
 file. (? read such property in handler is possible?)
 
 Also what is the nature of this dump? How big is it? Where will it go?
 The jars includes *app.jar and *boot.jar, the later usually can be
 ignored (rt.jar etc system jars) unless it's rewritten by JVMTI agent. The
 app.jar will contain all classes by customer, so we can do whatever we can
 to the jar.
 
 
 Thanks
 Yumin
 
 
 On 8/12/2013 5:51 AM, Christian

Re: Review Request (M) 7187554: JSR 292: JVMTI PopFrame needs to handle appendix arguments

[Christian Thalinger]

On Jul 25, 2013, at 5:14 PM, serguei.spit...@oracle.com wrote:

 
 Please, review the fix for:
  bug: http://bugs.sun.com/view_bug.do?bug_id=7187554
  jbs: https://jbs.oracle.com/bugs/browse/JDK-7187554
 
 Open webrev:
 http://cr.openjdk.java.net/~sspitsyn/webrevs/2013/hotspot/7187554-JVMTI-JSR292.1

src/share/vm/classfile/javaClasses.hpp:

+ public:
+  // Accessors
+  static oopmember(oop mh);
+  static void   set_member(oop mh, oop member);

Where is the implementation for these methods?  Because if we had them you 
could use it here:

src/share/vm/interpreter/interpreterRuntime.cpp:

+oop member_name = 
(dmh_oop)-obj_field(java_lang_invoke_DirectMethodHandle::member_offset_in_bytes());

Otherwise this change looks really good and clean.  It's not so bad after all.

-- Chris

 
 Summary:
  Restore the appendix argument of a polymorphic intrinsic call
  needed for a invokestatic re-execution after JVMTI PopFrame().
 
 Description
  When JVMTI's PopFrame removes a frame that was called via a call site that
  takes an appendix and that call site is reexecuted the appendix is not on
  the stack anymore because it got removed by the adapter.
  This fix is to detect such a case and push the appendix on the stack again 
 before reexecution.
 
 
 Testing:
  UTE tests - in progress: vm.mlvm.testlist, nsk.jvmti.testlist, 
 nsk.jdi.testlist
 
 Thanks,
 Serguei

hg: jdk8/tl/jdk: 8019184: MethodHandles.catchException() fails when methods have 8 args + varargs

[christian . thalinger]

Changeset: bd6949f9dbb2
Author:twisti
Date:  2013-07-03 11:35 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/bd6949f9dbb2

8019184: MethodHandles.catchException() fails when methods have 8 args + varargs
Reviewed-by: jrose

! src/share/classes/java/lang/invoke/MethodHandleImpl.java
+ test/java/lang/invoke/TestCatchExceptionWithVarargs.java

hg: jdk8/tl/jdk: 7177472: JSR292: MethodType interning penalizes scalability

[christian . thalinger]

Changeset: 2b63fda275a3
Author:twisti
Date:  2013-06-17 16:24 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/2b63fda275a3

7177472: JSR292: MethodType interning penalizes scalability
Reviewed-by: twisti
Contributed-by: Aleksey Shipilev aleksey.shipi...@oracle.com

! src/share/classes/java/lang/invoke/MethodType.java

Re: Review Request (S) 8014052: JSR292: assert(end_offset == next_offset) failed: matched ending

[Christian Thalinger]

Looks good.  -- Chris

On May 30, 2013, at 11:19 PM, serguei.spit...@oracle.com wrote:

 Please, review the fix for:
  bug: https://jbs.oracle.com/bugs/browse/JDK-8014052
  jbs: https://jbs.oracle.com/bugs/browse/JDK-8014052
 
 
 Open webrev:
 http://cr.openjdk.java.net/~sspitsyn/webrevs/2013/hotspot/8014052-JVMTI-JSR292.1
 
 Summary:
  A call to the finalize_operands_merge() must be unconditional.
  Otherwise, the merge of the bootstrap method operands is not completed 
 correctly.
 
 
 Testing: vm/mlvm, nsk/jvmti, nsk/jdi, nsk/jdwp
 
 Thanks,
 Serguei

Re: Review Request (S) 8015436: compiler/ciReplay/TestSA.sh fails with assert() index is out of bounds

[Christian Thalinger]

Looks good.  -- Chris

On May 29, 2013, at 9:08 PM, serguei.spit...@oracle.com wrote:

 Please, review the fix and unit test for:
  bug: http://bugs.sun.com/view_bug.do?bug_id=8015436
  jbs:  https://jbs.oracle.com/bugs/browse/JDK-8015436
 
 Open webrev:
 http://cr.openjdk.java.net/~sspitsyn/webrevs/2013/hotspot/8015436-JVMTI-JSR292.1
 
 Summary:
  The InstanceKlass _initial_method_idnum value must be adjusted as necessary 
 if the overpass methods are added.
  I guess, it is Ok to integrate a unit test covering the fix under the same 
 bug id.
 
 Testing:
  Newly added hotspot unit test: compiler/8015436/Test8015436.java
  The originally failed test:compiler/ciReplay/TestSA.sh
  The vm/mlvm tests
 
 Thanks,
 Serguei

Re: Review Request (S) 8014288: perf regression in nashorn JDK-8008448.js test after 8008511 changes

[Christian Thalinger]

Looks good.  -- Chris

On May 23, 2013, at 4:19 PM, serguei.spit...@oracle.com wrote:

 Please, review the fix for:
  bug: http://bugs.sun.com/view_bug.do?bug_id=8014288
  jbs: https://jbs.oracle.com/bugs/browse/JDK-8014288
 
 Open webrev:
 http://cr.openjdk.java.net/~sspitsyn/webrevs/2013/hotspot/8014288-JVMTI-JSR292.1/
 
 Summary:
  The fix of the 8008511 introduced a performance regression for the Nashorn 
 tests.
  The fix is to use method_idnum() for direct indexing into the MemberNameTable
  that replaces a linear search caused the regression.
  No new regression test is needed as the Nashorn tests show the performance 
 is back to normal.
 
 Testing:
  The vm/mlvm and Nashorn tests
 
 Thanks,
 Serguei

Re: hg: hsx/hotspot-rt/hotspot: 8012927: 'assert(nbits == 32 || (-(1 nbits-1) = x x ( 1 nbits-1))) failed: value out of range' in interpreter initialization.

[Christian Thalinger]

On Apr 25, 2013, at 6:45 AM, Volker Simonis volker.simo...@gmail.com wrote:

 Hi Jiangli,
 
 that's really sad!
 
 First because it is not the way it is supposed to work and second
 because we detected and fixed this same problem a few days ago as
 well..
 
 I don't understand what's the problem to post a review request on the
 list and get the reviews by mail. That shouldn't take much longer than
 a verbal review, even if you and the reviewers all sit in the same
 room.

I concur with Volker.  Open bugs which are pushed to open repositories should 
have an open review on one of the OpenJDK mailing lists.  No matter if urgent 
or not.

I hope that an open review system (which I also hope is coming soon) makes it 
less likely that this happens again.

-- Chris

 
 Regards,
 Volker
 
 
 On Wed, Apr 24, 2013 at 6:26 PM, Jiangli Zhou jiangli.z...@oracle.com wrote:
 Hi David,
 
 It was an urgent fix and didn't go through the normal route.
 
 Thanks,
 Jiangli
 
 
 On 04/23/2013 09:32 PM, David Holmes wrote:
 
 Jiangli,
 
 I do not see a public Request for Review for this change.
 
 David
 
 On 24/04/2013 9:11 AM, jiangli.z...@oracle.com wrote:
 
 Changeset: 1ea6a35dcbe5
 Author:jiangli
 Date:  2013-04-23 12:32 -0400
 URL: http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/1ea6a35dcbe5
 
 8012927: 'assert(nbits == 32 || (-(1  nbits-1) = x  x  ( 1 
 nbits-1))) failed: value out of range' in interpreter initialization.
 Summary: Change br_null_short() to br_null().
 Reviewed-by: coleenp, hseigel
 
 ! src/cpu/sparc/vm/interp_masm_sparc.cpp
 
 

Re: hs25 review request (round 2): 8007037 JSR 292: the VM_RedefineClasses::append_entry() should do cross-checks with indy operands

[Christian Thalinger]

On Apr 11, 2013, at 4:08 PM, serguei.spit...@oracle.com wrote:

 Please, review the hs25 fix (round 2) below.
 
 Open webrev:
 http://cr.openjdk.java.net/~sspitsyn/webrevs/2013/hotspot/8007037-JVMTI-JSR292.2/

One thing that makes it hard to follow what going on are the variable names, 
e.g.:

new_bs_i
old_bs_i
merge_ops
found_i
argc (unusual usage of that name :-)
…

I suppose _i is for index?  bs for bootstrap?  I don't expect you to change 
this; just pointing out.

src/share/vm/prims/jvmtiRedefineClasses.cpp:

+// value by seaching the index map. Returns zero (-1) if there is no mapped

Returns zero?

The change seems to be fine.  For correctness I really rely on your testing 
here.

-- Chris

 
 CR:
 http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8007037
 https://jbs.oracle.com/bugs/browse/JDK-8007037
 
 
 This webrev includes a review fix from Coleen about explicit deallocation
 of the old operands array in ConstantPool::resize_operands() and a couple
 of fixes for the corner case when the old CP has no operands array:
 
 1121 void ConstantPool::extend_operands(constantPoolHandle ext_cp, TRAPS) {
 . . .
 1130   if (operand_array_length(operands()) == 0) {
 1131 ClassLoaderData* loader_data = pool_holder()-class_loader_data();
 1132 Arrayu2* new_ops = MetadataFactory::new_arrayu2(loader_data, 
 delta_size, CHECK);
 1133 // The first element index defines the offset of second part
 +operand_offset_at_put(new_ops, 0, 2*delta_len); // offset in new 
 array
 1135 set_operands(new_ops);
 1136   } else {
 1137 resize_operands(delta_len, delta_size, CHECK);
 1138   }
 1139
 1140 } // end extend_operands()
 
 
 void VM_RedefineClasses::append_operand(constantPoolHandle scratch_cp, int 
 old_bs_i,
 505constantPoolHandle *merge_cp_p, int *merge_cp_length_p, TRAPS) {
 . . .
 
 - 518   int new_base = (*merge_cp_p)-operand_next_offset_at(new_bs_i);
 
 --
 
 + 518   // We have _operands_cur_length == 0 when the merge_cp operands is 
 empty yet.
 + 519   // However, the operand_offset_at(0) was set in the extend_operands() 
 call.
 + 520   int new_base = (new_bs_i == 0) ? (*merge_cp_p)-operand_offset_at(0)
 + 521  : 
 (*merge_cp_p)-operand_next_offset_at(new_bs_i - 1);
 
 
 
 Description:
 
 References from INDY bootstrap method specifier operands to CP entries
 and back must be correctly merged at class redefinition.
 
 Some background.
 
 An invokedynamic bytecode spec:
 http://docs.oracle.com/javase/specs/jvms/se7/html/jvms-6.html#jvms-6.5.invokedynamic
 
 A invokedynamic instruction has an argument which is an index to the 
 *Constant Pool* item.
 That index must be a symbolic reference to a *call-site specifier*:
 http://docs.oracle.com/javase/specs/jvms/se7/html/jvms-4.html#jvms-4.4.10
 
 A CP item of the type *CONSTANT_InvokeDynamic_inf* has an index into
 the *bootstrap method attribute* of the class file:
 http://docs.oracle.com/javase/specs/jvms/se7/html/jvms-4.html#jvms-4.7.21
 
 The *|BootstrapMethods|* attribute elements normally have references to other 
 *Constant Pool* items.
 
 In VM the *bootstrap method attribute* is represented by the *operands* array 
 of the *ConstantPool*.
 
 The problem is is that all the force and back cross links between 
 *ConstantPool* elements
 and *operands* array elements must be correctly merged at class redefinition.
 
 Test coverage:
  vm.mlvm, nsk.jvmti, nsk.jdi tests on multiple platforms (32 vs 64-bit too).
  The testing looks good so far.
  One difficulty is that new vm.mlvm tests are currently failed because of 
 multiple reasons.
 
 
 Thanks,
 Serguei

Re: Review Request (S) 8006542: JSR 292: the VM_RedefineClasses::append_entry() must support invokedynamic entry kinds

[Christian Thalinger]

On Jan 22, 2013, at 4:07 PM, serguei.spit...@oracle.com wrote:

 
 Please, review the fix for:
  https://jbs.oracle.com/bugs/browse/JDK-8006542
 
 
 Open webrev:
  
 http://cr.openjdk.java.net/~sspitsyn/webrevs/2013/hotspot/8006542-JVMTI-JSR292.1/

src/share/vm/prims/jvmtiRedefineClasses.hpp:

+  // Support for constant pool merging (these routines are in alpha order):
   void append_entry(constantPoolHandle scratch_cp, int scratch_i,
 constantPoolHandle *merge_cp_p, int *merge_cp_length_p, TRAPS);
+  int find_or_append_indirect_entry(constantPoolHandle scratch_cp, int 
scratch_i,
+constantPoolHandle *merge_cp_p, int *merge_cp_length_p, TRAPS);
   int find_new_index(int old_index);

Not really alpha order ;-)

Otherwise this looks good (as far as I can tell).

-- Chris

 
 Summary:
  Need a support for invokedynamic entry kinds when new and old constant pools 
 are merged.
 
 Testing:
  vm/mlvm/indy/func/jvmti/redefineClassInBootstrap
  Asked the VM SQE team to develop new INDY tests for better coverage.
 
 
 Thanks,
 Serguei
 
 
 

Re: 1-line review request: 7194607 VerifyLocalVariableTableOnRetransformTest.sh fails after JSR-292 merge

[Christian Thalinger]

Thanks.  -- Chris

On Oct 31, 2012, at 5:46 PM, serguei.spit...@oracle.com wrote:

 I've created the hotspot/runtime CR:
  https://jbs.oracle.com/bugs/browse/JDK-8002087
 
 Assigned to myself.
 
 Thanks,
 Serguei
 
 
 On 10/31/12 2:13 PM, serguei.spit...@oracle.com wrote:
 Thanks, Christian.
 
 I'm not comfortable to fix it as a part of 7194607.
 One question is what tests are need to be run to verify possible fixes.
 
 I'll open a separate CR under hotspot/runtime to track the Interpreter 
 issues related to max_stack.
 
 
 Thanks,
 Serguei
 
 On 10/31/12 10:54 AM, Christian Thalinger wrote:
 On Oct 30, 2012, at 4:25 PM, serguei.spit...@oracle.com wrote:
 
 Ok, it seems there are some suspicious fragments in the interpreter code.
 Christian, could you, please, check and comment the fragments below?
 
 This is how the Method::max_stack() is defined:
 
 src/share/vm/oops/method.hpp:
 
   int  verifier_max_stack() const{ return _max_stack; }
   int   max_stack() const{ return _max_stack + 
 extra_stack_entries(); }
   void  set_max_stack(int size)  { _max_stack = size; }
   . . .
   static int extra_stack_entries() { return EnableInvokeDynamic ? 2 : 0; }
 
 
 The following code fragments are unaware that the method-max_stack() 
 returns _max_stack + extra_stack_entries()  :
 
 src/cpu/sparc/vm/cppInterpreter_sparc.cpp:
 src/cpu/sparc/vm/cppInterpreter_x86.cpp:
 
 static int size_activation_helper(int callee_extra_locals, int max_stack, 
 int monitor_size) {
   . . .
   const int extra_stack = 0; //6815692//Method::extra_stack_entries(); 
  
   return (round_to(max_stack +
extra_stack +
 Remove extra_stack.
 
. . .
 }
 . . .
 void BytecodeInterpreter::layout_interpreterState(interpreterState to_fill,
   . . .
   int extra_stack = 0; //6815692//Method::extra_stack_entries();   
 
   to_fill-_stack_limit = stack_base - (method-max_stack() + 1 + 
 extra_stack);
 Remove extra_stack (but keep the +1; see comment nearby).
 
   . . .
 }
 
 
 src/cpu/sparc/vm/templateInterpreter_sparc.cpp:
 
 static int size_activation_helper(int callee_extra_locals, int max_stack, 
 int monitor_size) {
   . . .
   const int max_stack_words = max_stack * Interpreter::stackElementWords;
   return (round_to((max_stack_words
//6815692//+ Method::extra_stack_words()

 The comment needs to be removed.
 
   . . .
 }
 
 At the size_activation_helper call sites the second parameter is usually 
 passed as method-max_stack().
 
 
 src/cpu/x86/vm/templateInterpreter_x86_32.cpp:
 src/cpu/x86/vm/templateInterpreter_x86_64.cpp:
 
 int AbstractInterpreter::size_top_interpreter_activation(Method* method) {
   . . .
   const int extra_stack = Method::extra_stack_entries();
   const int method_stack = (method-max_locals() + method-max_stack() + 
 extra_stack) *
 Remove extra_stack.
 
 Interpreter::stackElementWords;
   . . .
 }
 
 
 src/share/vm/interpreter/oopMapCache.cpp:
 
 void OopMapForCacheEntry::compute_map(TRAPS) {
   . . .
   // First check if it is a method where the stackmap is always empty
   if (method()-code_size() == 0 || method()-max_locals() + 
 method()-max_stack() == 0) {
 _entry-set_mask_size(0);
   } else {
   . . .
 }
 
 Above, if the invokedynamic is enabled then the method()-max_stack() can 
 not be 0.
 We need to check it if this fact does not break the fragment.
 That means we are always generating oop maps even if we wouldn't need them. 
  Let me think more about this...
 
 -- Chris
 
 
 I'm still looking at other places...
 
 
 Thanks,
 Serguei
 
 
 On 10/30/12 10:41 AM, serguei.spit...@oracle.com wrote:
 I have a plan to look at it, at least for other serviceablity code.
 It'd be good if someone from the runtime or compiler team checked it too.
 
 Thanks,
 Serguei
 
 
 On 10/30/12 10:37 AM, Daniel D. Daugherty wrote:
 Thumbs up.
 
 Is someone going to do an audit for similar missing changes
 from max_stack() (not max_size()) to verifier_max_stack()?
 
 Dan
 
 
 On 10/30/12 1:30 PM, serguei.spit...@oracle.com wrote:
 Hello,
 
 
 Please, review the fix for CR:
 http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7194607
 
 CR in JIRA:
   https://jbs.oracle.com/bugs/browse/JDK-7194607
 
 Open webrev:
 http://cr.openjdk.java.net/~sspitsyn/webrevs/2012/7194607-JVMTI-max_size
  
 
 
 Summary:
 
 This issue is caused by the changes in the oops/method.hpp for 
 invokedynamic (JSR 292).
 Now the max_stack() adds +2 to the original code attribute stack size 
 if invokedynamic is enabled.
 The verifier_max_stack() must be used in the 
 jvmtiClassFileReconstituter.cpp
 instead of the max_size() to get the code attribute stack size.
 
 
 Thanks,
 Serguei
 
 
 

Re: 1-line review request: 7194607 VerifyLocalVariableTableOnRetransformTest.sh fails after JSR-292 merge

[Christian Thalinger]

On Oct 30, 2012, at 4:25 PM, serguei.spit...@oracle.com wrote:

 Ok, it seems there are some suspicious fragments in the interpreter code.
 Christian, could you, please, check and comment the fragments below?
 
 This is how the Method::max_stack() is defined:
 
 src/share/vm/oops/method.hpp:
 
   int  verifier_max_stack() const{ return _max_stack; }
   int   max_stack() const{ return _max_stack + 
 extra_stack_entries(); }
   void  set_max_stack(int size)  {_max_stack = size; }
   . . .
   static int extra_stack_entries() { return EnableInvokeDynamic ? 2 : 0; }
 
 
 The following code fragments are unaware that the method-max_stack() returns 
 _max_stack + extra_stack_entries()  :
 
 src/cpu/sparc/vm/cppInterpreter_sparc.cpp:
 src/cpu/sparc/vm/cppInterpreter_x86.cpp:
 
 static int size_activation_helper(int callee_extra_locals, int max_stack, int 
 monitor_size) {
   . . .
   const int extra_stack = 0; //6815692//Method::extra_stack_entries();  
 
   return (round_to(max_stack +
extra_stack +

Remove extra_stack.

. . .
 }
 . . .
 void BytecodeInterpreter::layout_interpreterState(interpreterState to_fill,
   . . .
   int extra_stack = 0; //6815692//Method::extra_stack_entries();  
  
   to_fill-_stack_limit = stack_base - (method-max_stack() + 1 + 
 extra_stack);

Remove extra_stack (but keep the +1; see comment nearby).

   . . .
 }
 
 
 src/cpu/sparc/vm/templateInterpreter_sparc.cpp:
 
 static int size_activation_helper(int callee_extra_locals, int max_stack, int 
 monitor_size) {
   . . .
   const int max_stack_words = max_stack * Interpreter::stackElementWords;
   return (round_to((max_stack_words
//6815692//+ Method::extra_stack_words()   
 

The comment needs to be removed.

   . . .
 }
 
 At the size_activation_helper call sites the second parameter is usually 
 passed as method-max_stack().
 
 
 src/cpu/x86/vm/templateInterpreter_x86_32.cpp:
 src/cpu/x86/vm/templateInterpreter_x86_64.cpp:
 
 int AbstractInterpreter::size_top_interpreter_activation(Method* method) {
   . . .
   const int extra_stack = Method::extra_stack_entries();
   const int method_stack = (method-max_locals() + method-max_stack() + 
 extra_stack) *

Remove extra_stack.

Interpreter::stackElementWords;
   . . .
 }
 
 
 src/share/vm/interpreter/oopMapCache.cpp:
 
 void OopMapForCacheEntry::compute_map(TRAPS) {
   . . .
   // First check if it is a method where the stackmap is always empty
   if (method()-code_size() == 0 || method()-max_locals() + 
 method()-max_stack() == 0) {
 _entry-set_mask_size(0);
   } else {
   . . .
 }
 
 Above, if the invokedynamic is enabled then the method()-max_stack() can not 
 be 0.
 We need to check it if this fact does not break the fragment.

That means we are always generating oop maps even if we wouldn't need them.  
Let me think more about this...

-- Chris

 
 
 I'm still looking at other places...
 
 
 Thanks,
 Serguei
 
 
 On 10/30/12 10:41 AM, serguei.spit...@oracle.com wrote:
 I have a plan to look at it, at least for other serviceablity code.
 It'd be good if someone from the runtime or compiler team checked it too.
 
 Thanks,
 Serguei
 
 
 On 10/30/12 10:37 AM, Daniel D. Daugherty wrote:
 Thumbs up.
 
 Is someone going to do an audit for similar missing changes
 from max_stack() (not max_size()) to verifier_max_stack()?
 
 Dan
 
 
 On 10/30/12 1:30 PM, serguei.spit...@oracle.com wrote:
 Hello,
 
 
 Please, review the fix for CR:
   http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7194607
 
 CR in JIRA:
   https://jbs.oracle.com/bugs/browse/JDK-7194607
 
 Open webrev:
   http://cr.openjdk.java.net/~sspitsyn/webrevs/2012/7194607-JVMTI-max_size
 
 
 Summary:
 
 This issue is caused by the changes in the oops/method.hpp for 
 invokedynamic (JSR 292). 
 Now the max_stack() adds +2 to the original code attribute stack size if 
 invokedynamic is enabled. 
 The verifier_max_stack() must be used in the 
 jvmtiClassFileReconstituter.cpp
 instead of the max_size() to get the code attribute stack size. 
 
 
 Thanks,
 Serguei
 
 
 

Re: RFR: 6879063: SA should use hsdis for disassembly

[Christian Thalinger]

This change broke the usage of previously built versions of hsdis.  I filed a 
bug for it:

8000489: older builds of hsdis don't work anymore after 6879063

Yumin, I assigned the bug to you.  Could you please take care of it?

-- Chris

On Aug 27, 2012, at 2:07 PM, Yumin Qi yumin...@oracle.com wrote:

 Hi, all
   
   Can I have you code review of 
   6879063: SA should use hsdis for disassembly
 
   http://cr.openjdk.java.net/~minqi/6879063
 
   The SA has Java based disassemblers for x86 and sparc but amd64.  Instead 
 of porting to amd64 we should switch over to using hsdis for it like the JVM 
 does.  This requires a new entry point into hsdis, 
 decode_instructions_virtual, which separates the address of the code being 
 disassembled from the buffer containing the code.  The existing uses of 
 decode_instructions have been updated to use the new interface and SA 
 Disassembler has Java native methods that call into hsdis and call back up to 
 Java to perform the disassembly. Also changed makefile for hsdis build for 
 both(i386/amd64).
 
   All the old disassembler logic was deleted since it's incompatible with the 
 new disassembly interface. Also deleted are dbx based SA interface and few 
 other dead files.
 
   Tested by dumping full assembly from core files.
 
   Reviewed-by:
   Contributed-by: Tom R (never)
 
   Thanks
   Yumin Qi
 

Re: RFR: 6879063: SA should use hsdis for disassembly

[Christian Thalinger]

Looks good.  -- Chris

On Aug 28, 2012, at 4:48 PM, Yumin Qi yumin...@oracle.com wrote:

 Hi, all 
 
   Updated with feedback suggestions. Please have a look again at the same 
 link.
 
 Thanks
 Yumin
  
 
   
 
 On 2012/8/27 14:07, Yumin Qi wrote:
 Hi, all
   
   Can I have you code review of 
   6879063: SA should use hsdis for disassembly
 
   http://cr.openjdk.java.net/~minqi/6879063
 
   The SA has Java based disassemblers for x86 and sparc but amd64.  Instead 
 of porting to amd64 we should switch over to using hsdis for it like the JVM 
 does.  This requires a new entry point into hsdis, 
 decode_instructions_virtual, which separates the address of the code being 
 disassembled from the buffer containing the code.  The existing uses of 
 decode_instructions have been updated to use the new interface and SA 
 Disassembler has Java native methods that call into hsdis and call back up 
 to Java to perform the disassembly. Also changed makefile for hsdis build 
 for both(i386/amd64).
 
   All the old disassembler logic was deleted since it's incompatible with 
 the new disassembly interface. Also deleted are dbx based SA interface and 
 few other dead files.
 
   Tested by dumping full assembly from core files.
 
   Reviewed-by:
   Contributed-by: Tom R (never)
 
   Thanks
   Yumin Qi
 

Re: RFR: 6879063: SA should use hsdis for disassembly

[Christian Thalinger]

On Aug 27, 2012, at 2:07 PM, Yumin Qi yumin...@oracle.com wrote:

 Hi, all
   
   Can I have you code review of 
   6879063: SA should use hsdis for disassembly
 
   http://cr.openjdk.java.net/~minqi/6879063
 
   The SA has Java based disassemblers for x86 and sparc but amd64.  Instead 
 of porting to amd64 we should switch over to using hsdis for it like the JVM 
 does.  This requires a new entry point into hsdis, 
 decode_instructions_virtual, which separates the address of the code being 
 disassembled from the buffer containing the code.  The existing uses of 
 decode_instructions have been updated to use the new interface and SA 
 Disassembler has Java native methods that call into hsdis and call back up to 
 Java to perform the disassembly. Also changed makefile for hsdis build for 
 both(i386/amd64).
 
   All the old disassembler logic was deleted since it's incompatible with the 
 new disassembly interface. Also deleted are dbx based SA interface and few 
 other dead files.
 
   Tested by dumping full assembly from core files.
 
   Reviewed-by:
   Contributed-by: Tom R (never)

src/share/tools/hsdis/hsdis.c:

Maybe decode_instructions should call decode_instructions_virtual.

agent/src/share/classes/sun/jvm/hotspot/asm/Disassembler.java:

+String cpu = VM.getVM().getCPU();
+if (cpu.equals(sparc)) {
+   if (VM.getVM().isLP64()) {
+  libname = hsdis-sparcv9;
+  options = v9only;
+   } else {
+  libname = hsdis-sparc;
+   }
+} else if (cpu.equals(x86)) {
+   libname = hsdis-i386;
+} else if (cpu.equals(amd64)) {
+   libname = hsdis-amd64;
+} else if (cpu.equals(ia64)) {
+   libname = hsdis-ia64;
+}

Should we use some default libname here like:

  libname = hsdis- + cpu;

so we cover other architectures too (and only special-case sparc, x86, ...)?

Otherwise it looks good.

-- Chris

 
   Thanks
   Yumin Qi
 

Re: Code review for SA changes to allow for new processor architectures (7154641)

[Christian Thalinger]

On Aug 21, 2012, at 2:47 PM, BILL PITTORE bill.pitt...@oracle.com wrote:

 These changes allow for the (easier) addition of new processor types to SA 
 other than the standard x86, amd64 and sparc. By using reflection, it is not 
 necessary to instantiate the new class directly in the existing code. Rather 
 the class name is derived from the cpu/os name and is loaded and the 
 constructor called. Note that the existing cpus (x86, amd64, and sparc) code 
 was not modified. Only newly added cpus would go through the reflection code 
 path.

And why is that?  -- Chris

 
 http://cr.openjdk.java.net/~bpittore/7154641/webrev.00/
 
 thanks,
 bill
 

Re: RFR: 6830717: replay of compilations would help with debugging

[Christian Thalinger]

On Aug 6, 2012, at 2:40 PM, yumin...@oracle.com wrote:

 Hi,
 
  Please give your comments of the changes about
  6830717: replay of compilations would help with debugging.
 http://monaco.sfbay.sun.com/detail.jsf?cr=6830717
 
  Sometime jvm crashes in the process of compilation or in compiled method. To 
 reproduce the compilation process in debug JVM is helpful for quickly 
 identifying root cause.
  To do recompilation, we collect data by using of SA (Serviceability Agent) 
 to extract application and system class data from core file. Those 
 information includes nmethod, methodOop, methodDataOop, instanceKlass and 
 corresponding ci counterparts.
  With reconfiguring similar (not exactly same) compiling environment as in 
 the core file, try to compile again the failed java methods.
 
  contributed by Tom R (never).   webrev:
 
 http://cr.openjdk.java.net/~minqi/6830717/ 
 http://cr.openjdk.java.net/%7Eminqi/6830717/

src/share/vm/ci/ciReplay.cpp:

 603 if (strcmp(field_signature, [B) == 0) {
 604   oop value = oopFactory::new_byteArray(length, CHECK);
 605   java_mirror-obj_field_put(fd.offset(), value);
 606 } else if (strcmp(field_signature, [Z) == 0) {
 607   oop value = oopFactory::new_boolArray(length, CHECK);
 608   java_mirror-obj_field_put(fd.offset(), value);
 609 } else if (strcmp(field_signature, [C) == 0) {
 610   oop value = oopFactory::new_charArray(length, CHECK);
 611   java_mirror-obj_field_put(fd.offset(), value);
 612 } else if (strcmp(field_signature, [S) == 0) {
 613   oop value = oopFactory::new_shortArray(length, CHECK);
 614   java_mirror-obj_field_put(fd.offset(), value);
 615 } else if (strcmp(field_signature, [F) == 0) {
 616   oop value = oopFactory::new_singleArray(length, CHECK);
 617   java_mirror-obj_field_put(fd.offset(), value);
 618 } else if (strcmp(field_signature, [D) == 0) {
 619   oop value = oopFactory::new_doubleArray(length, CHECK);
 620   java_mirror-obj_field_put(fd.offset(), value);
 621 } else if (strcmp(field_signature, [I) == 0) {
 622   oop value = oopFactory::new_intArray(length, CHECK);
 623   java_mirror-obj_field_put(fd.offset(), value);
 624 } else if (strcmp(field_signature, [J) == 0) {
 625   oop value = oopFactory::new_longArray(length, CHECK);
 626   java_mirror-obj_field_put(fd.offset(), value);
 627 } else if (field_signature[0] == '['  field_signature[1] == 'L') 
{

Can't we switch on the second character?  And move this call:

 630   java_mirror-obj_field_put(fd.offset(), value);

out of the switch?

 637   if (strcmp(field_signature, I) == 0) {
 638 int value = atoi(string_value);
 639 java_mirror-int_field_put(fd.offset(), value);
 640   } else if (strcmp(field_signature, B) == 0) {
 641 int value = atoi(string_value);
 642 java_mirror-byte_field_put(fd.offset(), value);
 643   } else if (strcmp(field_signature, C) == 0) {
 644 int value = atoi(string_value);
 645 java_mirror-char_field_put(fd.offset(), value);
 646   } else if (strcmp(field_signature, S) == 0) {
 647 int value = atoi(string_value);
 648 java_mirror-short_field_put(fd.offset(), value);
 649   } else if (strcmp(field_signature, Z) == 0) {
 650 int value = atol(string_value);
 651 java_mirror-bool_field_put(fd.offset(), value);
 652   } else if (strcmp(field_signature, J) == 0) {
 653 jlong value;
 654 if (sscanf(string_value, INT64_FORMAT, value) != 1) {
 655   fprintf(stderr, Error parsing long: %s\n, string_value);
 656   return;
 657 }
 658 java_mirror-long_field_put(fd.offset(), value);
 659   } else if (strcmp(field_signature, F) == 0) {
 660 float value = atof(string_value);
 661 java_mirror-float_field_put(fd.offset(), value);
 662   } else if (strcmp(field_signature, D) == 0) {
 663 double value = atof(string_value);
 664 java_mirror-double_field_put(fd.offset(), value);
 665   } else if (strcmp(field_signature, Ljava/lang/String;) == 0) {
 666 Handle value = java_lang_String::create_from_str(string_value, 
CHECK);
 667 java_mirror-obj_field_put(fd.offset(), value());
 668   } else if (field_signature[0] == 'L') {

Same here (not the put though)?

Otherwise it looks good.

-- Chris

 
 Thanks
 Yumin

Re: Pls review 6173675/7003271

[Christian Thalinger]

On Jan 4, 2011, at 10:00 PM, Paul Hohensee wrote:
 These two rfes implement per-thread approximate memory allocation tracking.
 6173675 also adds multi-thread-id versions of getThreadCpuTime and 
 getThreadUserTime.
 
 6173675 MM: approximate memory allocation rate/amount per thread
 7003271 Hotspot should track cumulative Java heap bytes allocated on a 
 per-thread basis

src/os/solaris/vm/thread_solaris.inline.hpp:

  56   Thread *candidate = ThreadLocalStorage::_get_thread_cache[ix];

You could have fixed the * in that line too.

src/share/vm/opto/macro.cpp:

1242 new (C, 4) StorePNode( needgc_false, contended_phi_rawmem, 
eden_top_adr,
1243TypeRawPtr::BOTTOM, new_eden_top );

1249 new (C, 5) StorePConditionalNode( needgc_false, 
contended_phi_rawmem, eden_top_adr,
1250   new_eden_top, 
fast_oop/*old_eden_top*/ );

1284   Node* new_alloc_bytes = new (C, 3) AddLNode( alloc_bytes, alloc_size 
);

Could you also remove the spaces before and after the parentheses when you're 
at it?

MacroAssembler::incr_allocated_bytes:

It seems we could use a RegisterOrConstant size_in_bytes argument instead of 
Register var_size_in_bytes, int con_size_in_bytes.

Otherwise it looks good.

-- Christian

Re: review (S) for 6943485: JVMTI always on capabilities change code generation too much

[Christian Thalinger]

On Tue, 2010-04-20 at 15:39 -0700, Tom Rodriguez wrote:
 http://cr.openjdk.java.net/~never/6943485
 
 6943485: JVMTI always on capabilities change code generation too much
 Reviewed-by:
 
 The set of always on JVMTI capabilities include thing which will cause
 changes in the code the compiler generates which interferes with using
 JVMTI to support profiling since you aren't profiling the same
 generated code that you would be executing.  In particular the
 redefine and transform classes capabilities set the
 JvmtiExport::can_access_local_variables() flag which causes the
 compilers to keep all Java locals live even if they aren't needed for
 computation.  Additionally we will currently turn of EA is this is
 set, which was done as the fix for 6895383.  I've changed the setting
 of JvmtiExport::can_access_local_variables to only be driven by the
 two capabilities can_access_locals_variables and
 can_generate_breakpoints which should restrict these to be set when
 running under a debugger, since the original intent of keeping Java
 locals alive was to help simulate debugging while running under the
 interpreter where Java locals keep their values until they are
 overwritten.  JvmtiExport::can_access_local_variables is only used by
 the compilers so this doesn't affect any other part of the system.  I
 also moved some code which kept locals live in exception paths under
 the can_access_live_locals flag which made the flag
 can_examine_or_deopt_anywhere go dead.  There were also some variables
 Compilation::_needs_debug_information and Compile::_deopt_happens
 which are always true and always must be true that I just deleted.
 
 Tested with runthese and all the NSK serviceability tests.  Also
 compared the code we generate when running with and without an agent
 to make sure they were the same.

The changes look good and I think the changes are OK.

-- Christian