On Sat, 27 Apr 2024 00:48:49 GMT, Dean Long <dl...@openjdk.org> wrote:

>> Move immutable nmethod's data from CodeCache to C heap. It includes 
>> `dependencies, nul_chk_table, handler_table, scopes_pcs, scopes_data, 
>> speculations, jvmci_data`. It amounts for about 30% (optimized VM) of space 
>> in CodeCache.
>> 
>> Use HotSpot's `os::malloc()` to allocate memory in C heap for immutable 
>> nmethod's data. Bail out compilation if allocation failed.
>> 
>> Shuffle fields order and change some fields size from 4 to 2 bytes to avoid 
>> nmethod's header size increase.
>> 
>> Tested tier1-5, stress,xcomp
>> 
>> Our performance testing does not show difference.
>> 
>> Example of updated `-XX:+PrintNMethodStatistics` output is in JBS comment.
>
> src/hotspot/share/code/nmethod.cpp line 1332:
> 
>> 1330: #if INCLUDE_JVMCI
>> 1331:     _speculations_offset     = _scopes_data_offset;
>> 1332:     _jvmci_data_offset       = _speculations_offset;
> 
> Why not use 0 for all these?

Right. Before all these offsets were assigned to _dependencies_offset which was 
not 0.
But now we can use 0 for all of them since "_dependencies_offset" is 0.

> src/hotspot/share/code/nmethod.cpp line 1484:
> 
>> 1482:       // Calculate positive offset as distance between the start of 
>> stubs section
>> 1483:       // (which is also the end of instructions section) and the start 
>> of the handler.
>> 1484:       CHECKED_CAST(_unwind_handler_offset, int16_t, (_stub_offset - 
>> code_offset() - offsets->value(CodeOffsets::UnwindHandler)));
> 
> Suggestion:
> 
>       int unwind_handler_offset = code_offset() + 
> offsets->value(CodeOffsets::UnwindHandler);
>       CHECKED_CAST(_unwind_handler_offset, int16_t, _stub_offset - 
> unwind_handler_offset);

Will do.

> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/code/NMethod.java line 
> 528:
> 
>> 526:   private int getScopesDataOffset()   { return (int) 
>> scopesDataOffsetField  .getValue(addr); }
>> 527:   private int getScopesPCsOffset()    { return (int) 
>> scopesPCsOffsetField   .getValue(addr); }
>> 528:   private int getDependenciesOffset() { return (int) 0; }
> 
> Suggestion:
> 
> 
> No longer used.

Will remove.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18984#discussion_r1581667051
PR Review Comment: https://git.openjdk.org/jdk/pull/18984#discussion_r1581668080
PR Review Comment: https://git.openjdk.org/jdk/pull/18984#discussion_r1581679317

Reply via email to