RFR: 8343789: Move mutable nmethod data out of CodeCache

[Boris Ulasevich]

On Thu, 7 Nov 2024 23:18:03 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:

> Main question is: why you did it only for nmethod?

Yes. I did it symmetrically to a separate immutable data storage for nmethod. Now I see I do not like implementation where for some blobs relocation info is local, but for other it stays aside. I am going to rework that.

> any performance effects with this change?

On the aarch machine I see a slight improvement on the big benchmark caused by the code sparsity improvement. Though I need to do more benchmarking to make sure I am not making things worse for others.


Benchmark              Mode  Cnt    Score   Error  Units  |  Benchmark              Mode  Cnt    Score   Error  Units
JmhDotty.runOperation    ss  999  861.717 ± 1.543  ms/op  |  JmhDotty.runOperation    ss  999  840.959 ± 1.473  ms/op
                                                          |
       34555411781      cache-misses:u                    |         34343012187      cache-misses:u
     2913869717708      cpu-cycles:u                      |       2863838151745      cpu-cycles:u
     4185324759051      instructions:u                    |       4209616523046      instructions:u            
     1460914744576      L1-icache-loads:u                 |       1452066316397      L1-icache-loads:u
       97806845375      L1-icache-load-misses:u           |         93815390496      L1-icache-load-misses:u   
     1191854820746      iTLB-loads:u                      |       1169231847276      iTLB-loads:u
       10591067761      iTLB-load-misses:u                |         10134696419      iTLB-load-misses:u        
      838964735227      branch-loads:u                    |        838353168582      branch-loads:u
       25829615231      branch-load-misses:u              |         24361474411      branch-load-misses:u
      836291984964      br_pred:u                         |        838153583659      br_pred:u
       25733552818      br_mis_pred:u                     |         24353396612      br_mis_pred:u
         562168308      group0-code_sparsity:u            |           449848707      group0-code_sparsity:u

> Mutable sizes % do not add up:

Thanks. The correct sizes:

Statistics for 21032 bytecoded nmethods for C1:
 mutable data    = 10488856 (8.688409%)
   relocation    = 6573064 (62.667118%)
   oops          = 515680 (4.916456%)
   metadata      = 3400112 (32.416424%)
Statistics for 8171 bytecoded nmethods for C2:
 mutable data    = 6572064 (10.118859%)
   relocation    = 3406216 (51.828709%)
   oops          = 305912 (4.654733%)
   metadata      = 2859936 (43.516556%)

> My idea was to move relocation info data (which has big size) into `immutable` data section of nmethod. And leave mutable `_oops` and `_metadata` together with code since they are relatively small and we need to patch them together with code.

Hmm. If relocation info goes to an immutable blob, oops+metadata hardly deserves a separate blob.

> src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 5273:
> 
>> 5271:   } else {
>> 5272:     address dummy = address(uintptr_t(pc()) & -wordSize); // A nearby aligned address
>> 5273:     mov(dst, Address(dummy, rspec));
> 
> Why this is needed?

- it is not a load from a Constant Pool, so calling ldr_constant here is seems incorrect
- the ldr_constant function utilizes either ldr (with a range limit of ±1MB) or, when -XX:-NearCpool is enabled, adrp (range limit of ±2GB) followed by ldr — both of which may fall short when mutable data is allocated on the C heap.

> src/hotspot/share/code/codeBlob.hpp line 129:
> 
>> 127:   address _mutable_data;
>> 128:   int     _mutable_data_size;
>> 129: 
> 
> Should we add special CodeBlob subclass for nmethod to avoid increase of size for all blobs and stubs?

I am not sure. All CodeBlobs with relocation info needs a mutable data. Let me know if you think it must be a separate subclass.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/21276#issuecomment-2466182799
PR Comment: https://git.openjdk.org/jdk/pull/21276#issuecomment-2466183939
PR Comment: https://git.openjdk.org/jdk/pull/21276#issuecomment-2466185286
PR Review Comment: https://git.openjdk.org/jdk/pull/21276#discussion_r1835344697
PR Review Comment: https://git.openjdk.org/jdk/pull/21276#discussion_r1852207740