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

[David Holmes]

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
>>>
>