On Mon, 15 Apr 2024 03:24:07 GMT, Vladimir Kozlov <k...@openjdk.org> wrote:
>> This is part of changes which try to reduce size of `nmethod` and `codeblob`
>> data vs code in CodeCache.
>> These changes reduced size of `nmethod` header from 288 to 232 bytes. From
>> 304 to 248 in optimized VM:
>>
>> Statistics for 1282 bytecoded nmethods for C2:
>> total in heap = 5560352 (100%)
>> header = 389728 (7.009053%)
>>
>> vs
>>
>> Statistics for 1322 bytecoded nmethods for C2:
>> total in heap = 8307120 (100%)
>> header = 327856 (3.946687%)
>>
>>
>> Several unneeded fields in `nmethod` and `CodeBlob` were removed. Some
>> fields were changed from `int` to `int16_t` with added corresponding asserts
>> to make sure their values are fit into 16 bits.
>>
>> I did additional cleanup after recent `CompiledMethod` removal.
>>
>> Tested tier1-7,stress,xcomp and performance testing.
>
> Vladimir Kozlov has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Union fields which usages do not overlap
src/hotspot/share/code/nmethod.cpp line 1235:
> 1233: int skipped_insts_size =
> code_buffer->total_skipped_instructions_size();
> 1234: #ifdef ASSERT
> 1235: assert(((skipped_insts_size >> 16) == 0), "size is bigger than
> 64Kb: %d", skipped_insts_size);
Suggestion:
I think it's simpler just to use checked_cast below.
src/hotspot/share/code/nmethod.cpp line 1240:
> 1238: int consts_offset =
> code_buffer->total_offset_of(code_buffer->consts());
> 1239: assert(consts_offset == 0, "const_offset: %d", consts_offset);
> 1240: #endif
Suggestion:
src/hotspot/share/code/nmethod.cpp line 1241:
> 1239: assert(consts_offset == 0, "const_offset: %d", consts_offset);
> 1240: #endif
> 1241: _skipped_instructions_size = (uint16_t)skipped_insts_size;
Suggestion:
_skipped_instructions_size =
checked_cast<uint16_t>(code_buffer->total_skipped_instructions_size());
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18768#discussion_r1566764300
PR Review Comment: https://git.openjdk.org/jdk/pull/18768#discussion_r1566765068
PR Review Comment: https://git.openjdk.org/jdk/pull/18768#discussion_r1566759786
