RFR (XS): 8003879: Duplicate definitions in vmStructs

2012-11-21 Thread Mikael Vidstedt
Please review the below change. The change for 7045397 introduced a couple of duplicate entries in the vmStructs::localHotSpotVMTypes array. This shows up when using the jmap tool in a rather ugly way: Attaching to process ID 25561, please wait... Warning: the type "MetablockTreeDictionary"

Re: RFR (XS): 8003879: Duplicate definitions in vmStructs

2012-11-21 Thread David Holmes
Hi Mikael, On 22/11/2012 11:42 AM, Mikael Vidstedt wrote: Please review the below change. The change for 7045397 introduced a couple of duplicate entries in the vmStructs::localHotSpotVMTypes array. This shows up when using the jmap tool in a rather ugly way: Other than an indentation probl

Re: RFR (XS): 8003879: Duplicate definitions in vmStructs

2012-11-21 Thread serguei.spit...@oracle.com
It looks good modulo what David pointed out. Thanks, Serguie On 11/21/12 8:17 PM, David Holmes wrote: Hi Mikael, On 22/11/2012 11:42 AM, Mikael Vidstedt wrote: Please review the below change. The change for 7045397 introduced a couple of duplicate entries in the vmStructs::localHotSpotVMTy

Re: RFR (XS): 8003879: Duplicate definitions in vmStructs

2012-11-22 Thread Mikael Vidstedt
On 2012-11-21 20:17, David Holmes wrote: Hi Mikael, On 22/11/2012 11:42 AM, Mikael Vidstedt wrote: Please review the below change. The change for 7045397 introduced a couple of duplicate entries in the vmStructs::localHotSpotVMTypes array. This shows up when using the jmap tool in a rather ug

Re: RFR (XS): 8003879: Duplicate definitions in vmStructs

2012-11-22 Thread Mikael Vidstedt
New webrev available here: http://cr.openjdk.java.net/~mikael/8003879/webrev.01 Cheers, Mikael On 2012-11-22 14:19, Mikael Vidstedt wrote: On 2012-11-21 20:17, David Holmes wrote: Hi Mikael, On 22/11/2012 11:42 AM, Mikael Vidstedt wrote: Please review the below change. The change for 704

Re: RFR (XS): 8003879: Duplicate definitions in vmStructs

2012-11-22 Thread Mikael Vidstedt
I gave it some more thought and decided to try what it would look like having the test be part of the ExecuteInternalVMTests family instead. I like this one better personally, but comments are appreciated! http://cr.openjdk.java.net/~mikael/8003879/webrev.02 Cheers, Mikael On 2012-11-22 14:

Re: RFR (XS): 8003879: Duplicate definitions in vmStructs

2012-11-22 Thread David Holmes
Hi Mikael, I prefer this as an internal test too. Though I don't understand why execute_internal_vm_tests is in jni.cpp rather than jvm.cpp - this is an enhancement to the VM interface not the JNI interface isn't it? (Separate issue of course). BTW the weird indentation makes more sense view

Re: RFR (XS): 8003879: Duplicate definitions in vmStructs

2012-11-22 Thread Mikael Vidstedt
On 2012-11-22 16:35, David Holmes wrote: Hi Mikael, I prefer this as an internal test too. Though I don't understand why execute_internal_vm_tests is in jni.cpp rather than jvm.cpp - this is an enhancement to the VM interface not the JNI interface isn't it? (Separate issue of course). I th

Re: RFR (XS): 8003879: Duplicate definitions in vmStructs

2012-11-22 Thread David Holmes
On 22/11/2012 2:17 PM, David Holmes wrote: I have a few comments on this part: - Array indices should be int's not size_t (ie signed not unsigned) I humbly withdraw that comment. It is required to be an "integer type" which includes both signed and unsigned and various sizes. I certainly pr

Re: RFR (XS): 8003879: Duplicate definitions in vmStructs

2012-11-22 Thread Mikael Vidstedt
On 2012-11-22 19:04, David Holmes wrote: On 22/11/2012 2:17 PM, David Holmes wrote: I have a few comments on this part: - Array indices should be int's not size_t (ie signed not unsigned) I humbly withdraw that comment. It is required to be an "integer type" which includes both signed and un

Re: RFR (XS): 8003879: Duplicate definitions in vmStructs

2012-11-26 Thread serguei.spit...@oracle.com
Looks good. However, it'd be nice to fix the indentation commented by David (use frames to notice it): *vmStructs_cms.hpp*: 68declare_type(AFLBinaryTreeDictionary, FreeBlockDictionary) *jni.cpp:* 2110declare_type(MetablockTreeDictionary, FreeBlockDictionary) Tha

Re: RFR (XS): 8003879: Duplicate definitions in vmStructs

2012-11-26 Thread Mikael Vidstedt
Serguei, I'm confused. When I look at the latest webrev (http://cr.openjdk.java.net/~mikael/8003879/webrev.02/) I believe the indentation is correct - correct being defined as aligned on the first letter of the type name. Please clarify what you expect if you do not agree. Thanks, Mikael O

Re: RFR (XS): 8003879: Duplicate definitions in vmStructs

2012-11-26 Thread serguei.spit...@oracle.com
Mikael, Sorry for the confusion. I see what you mean. I missed that you made it consistent with the current style in these files. Agree, that is the best you can do. Thanks, Serguei On 11/26/12 2:37 PM, Mikael Vidstedt wrote: Serguei, I'm confused. When I look at the latest webrev (http://c

Re: RFR (XS): 8003879: Duplicate definitions in vmStructs

2012-11-26 Thread Mikael Vidstedt
No worries, thanks for clarifying and for the review(s)! Cheers, Mikael On 11/26/2012 3:00 PM, serguei.spit...@oracle.com wrote: Mikael, Sorry for the confusion. I see what you mean. I missed that you made it consistent with the current style in these files. Agree, that is the best you can