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

[Mikael Vidstedt]

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