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
