RFR: 8243996: Remove hardcoded field offsets from HotSpot

[David Holmes]

Hi Erik,

On 14/05/2020 1:26 am, Erik Österlund wrote:
> Hi,
> 
> Let's remove hardcoded field offsets. Main trick to get this done is to 
> split interpreter initialization
> into two parts: 1. reserve the interpreter memory, 2. generate the 
> interpreter. This allows reserving
> it before loading methods (which generates i2c adapters that assert 
> things are "in the interpreter",
> requiring knowledge about its address range), and then load the methods, 
> and then generate the interpreter.
> This moving of interpreter code generation until after methods have been 
> loaded allows offsets to be
> looked up, needed by code generation. That way, we no longer need to 
> hardcode field offsets, which
> makes it less annoying to change the field layout code in the future.
> 
> Thanks to Coleen for pre-reviewing this.
> 
> Bug:
> https://bugs.openjdk.java.net/browse/JDK-8243996
> 
> Webrev:
> http://cr.openjdk.java.net/~eosterlund/8243996/webrev.01/

This seems like a good simplification, and allows for future 
flexibility. Initialization order changes are always tricky as the 
dependencies are not always obvious - there may be logging or error path 
related code that eventually assumes the interpreter has been generated 
- but we can deal with such issues if they arise.

Generally all seems fine to me - I won't comment on the colour of the 
bikeshed. :)

One small comment:

src/hotspot/share/runtime/init.cpp

! void interpreter_init_code();      // after methods loaded

given the later comment:

+   interpreter_init_code(); // after javaClasses_init and before any 
method gets linked (not loaded)

should the first comment be expanded to:

void interpreter_init_code();      // after methods loaded, but before 
they are linked

?

Also IIUC the parenthetical "(not loaded)" alludes to the currently 
incorrect comment:

void invocationCounter_init(); // before any methods loaded

but once you've fixed that there's no context to know what the "(not 
loaded)" relates to so it can just be dropped.

Thanks,
David
-----

> Thanks,
> /Erik