RFR (S): 8004747: Remove last_entry from VM_STRUCT macros

[Mikael Vidstedt]

Thanks David!

Still need a 2nd reviewer!

Cheers,
Mikael

On 12/28/2012 3:34 PM, David Holmes wrote:
> Mikael,
>
> That all sounds fine to me.
>
> Thanks,
> David
>
> On 29/12/2012 3:48 AM, Mikael Vidstedt wrote:
>>
>> David,
>>
>> I did two things (both on linux/x86_64 only):
>>
>> 1. I compared the pre-processor output manually. Unfortunately there are
>> line numbers sprinkled around in the pre-processed file which makes
>> comparing the output a bit more complex than it should be. Modulo the
>> line numbers and some white space changes the outputs matche.
>> 2. I wrote a small test which mimics what the SA does - it looks up the
>> four exported structures dynamically and reads the data. Apart from the
>> "address" field in the VMStructEntry entries the outputs almost [1]
>> match. The address field is expected to vary since it reflects the
>> actual placement in memory of various static variables.
>>
>> Are there other experiments you can think of?
>>
>> Cheers,
>> Mikael
>>
>> [1] Using this tool I actually found one bug in the table - one entry
>> (ClassLoaderDataGraph::_unloading) is declared as nonstatic when it is
>> indeed static. I'll file a separate bug for that.
>>
>> On 12/11/2012 8:16 PM, David Holmes wrote:
>>> Hi Mikael,
>>>
>>> This looks okay to me - I like the simplification. However as I don't
>>> perform mental macro substitution particularly well (okay, not at all
>>> ;-)) I was wondering if there was a simple validation test where we
>>> could compare the pre-processed output before and after?
>>>
>>> Thanks,
>>> David
>>>
>>> On 11/12/2012 7:01 AM, Mikael Vidstedt wrote:
>>>>
>>>> Please review the following change:
>>>>
>>>> Webrev: http://cr.openjdk.java.net/~mikael/8004747/webrev.00/
>>>>
>>>> Background:
>>>>
>>>> This patch does 3 things (depending on how you count it):
>>>>
>>>> 1. Move the call to generating the last entry for the VM structures
>>>> database
>>>>
>>>> The vmStructs functionality is a macro based way of creating a 
>>>> database
>>>> of information about VM internal structures, types and constants.
>>>>
>>>> The database for structures is defined in runtime/vmStructs.cpp in an
>>>> array called localHotSpotVMStructs. The content of the array is
>>>> generated with calls to a few different macros (VM_STRUCTS,
>>>> VM_STRUCTS_PARALLELGC, VM_STRUCTS_CMS, VM_STRUCTS_G1, 
>>>> VM_STRUCTS_CPU and
>>>> VM_STRUCTS_OS_CPU respectively). Common for all these macros is the 
>>>> fact
>>>> that the last argument passed in is another macro
>>>> (GENERATE_VM_STRUCT_LAST_ENTRY) which is a generator for the end
>>>> marker/last entry in the database; a special entry which can be easily
>>>> located by an external consumer of the information.
>>>>
>>>> Even though the end marker generator macro
>>>> (GENERATE_VM_STRUCT_LAST_ENTRY) is passed to all the VM_STRUCT* 
>>>> macros,
>>>> the actual end marker must be generated once and only once. The way 
>>>> this
>>>> is currently handled is that only the last VM_STRUCT macro, which
>>>> happens to be VM_STRUCTS_OS_CPU, actually makes use of its last 
>>>> argument
>>>> and adds it to the end of the array.
>>>>
>>>> Not only is this fairly complex to understand, it's also more fragile
>>>> than it needs to be.
>>>>
>>>> This change removes the last argument for all the VM_STRUCT macros and
>>>> instead explicitly inserts the end marker at the end of the
>>>> localHotSpotVMStructs array.
>>>>
>>>> The same VM_STRUCTS macros are being used in VMStructs::init to do 
>>>> type
>>>> checking. Instead of passing the end marker generator into the macros
>>>> the last parameter is instead CHECK_SENTINEL, which is defined to 
>>>> expand
>>>> to nothing, which means it can be safely removed.
>>>>
>>>> 2. Move the end marker generating for the VM types, VM int 
>>>> constants and
>>>> VM long constants databases
>>>>
>>>> Repeat the exact above story, but replace anything called 
>>>> Structs/STRUCT
>>>> with Types/TYPES, IntConstants/INT_CONSTANTS and
>>>> LongConstants/LONG_CONSTANTS respectively.
>>>>
>>>> 3. Minor prettification
>>>>
>>>> In addition to the above the change also removes some superfluous
>>>> backslashes from a few of the VM_STRUCT* macro calls.
>>>>
>>>> Thanks,
>>>> Mikael
>>>>
>>