RFR: 8321137: Reconsider ICStub alignment [v2]

Andrew Haley aph at openjdk.org
Tue Jan 16 09:47:22 UTC 2024


On Mon, 8 Jan 2024 09:47:36 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> This continues from #16911. It initially started as performance optimization to compact `ICStubs`, but I think the safety arguments for fitting the `ICStub` per instruction cache line prevails. See bug and previous PR for more gory details. The footprint improvements on some architectures come as side-effect of untying the `ICStub` size from `CodeEntryAlignment` to (sometimes lower) cache line size.
>> 
>> Note that the size of `ICStub` is important, because `ICBuffer` is small (10K by default), and its depletion causes the `ICBufferFull` safepoint. I would make a (separate) argument to bump the default `ICBuffer` size a bit to make it less important.
>> 
>> Current patch affects `ICStub` size in different ways on different platforms, since current size is effectively 2x`CodeEntryAlignment` and new size is cache line size:
>>  - ARM32: 32 -> 64 bytes :(
>>  - AArch64: 128 -> 64 bytes :)
>>  - x86_64: 64 -> 64 bytes :|
>>  - x86_32: 32 -> 64 bytes :(
>>  - PPC64: 512 -> 128 bytes :))
>>  - RISC-V: 128 -> 64 bytes :)
>>  - S390X: 128 -> 256 bytes :(
>>  - Zero: <not applicable, Zero does not use IC stubs>
>> 
>> Additional testing:
>>  - [x] Linux x86_64 server fastdebug, `tier{1,2,3,4}`
>>  - [x] Linux AArch64 server fastdebug, `tier{1,2,3,4}`
>
> Aleksey Shipilev has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Inline new_ic_stub

src/hotspot/share/code/icBuffer.hpp line 69:

> 67:   // cache coherency on some architectures to order the updates to ICStub and setting
> 68:   // the destination to the ICStub. Note that cache lines size might be larger than
> 69:   // CodeEntryAlignment that is a normal alignment for CodeBlobs.

This is a paragraph that raises more questions than it answers. The relationship between ordering and instruction cache line coherency is pretty tenuous even on x86 these days, isn't it?  I'd not say "For extra correctness/safety," but "To be cautious,".

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17277#discussion_r1453172823


More information about the hotspot-dev mailing list