RFR: 8370947: Mitigate Neoverse-N1 erratum 1542419 negative impact on GCs and JIT performance [v28]
Evgeny Astigeevich
eastigeevich at openjdk.org
Wed Feb 25 17:36:19 UTC 2026
On Thu, 19 Feb 2026 16:13:56 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:
>> Evgeny Astigeevich has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Do fullGC when multiple threads execute test methods
>
> src/hotspot/share/code/relocInfo.cpp line 621:
>
>> 619:
>> 620:
>> 621: bool metadata_Relocation::fix_metadata_relocation() {
>
> I understand we return the status here, so that we avoid invalidation when there is no real patching work was done. Granted, it likely matches the behavior we have. But I wonder how much this buys us? If we are doing the deferred invalidation in a very broad scope, it stands to reason we would _almost definitely_ have to invalidate, and all this tracking would _nearly always_ give us the same answer, "Do invalidate"?
>
> IOW, this might be an unnecessary complication of the interface.
>
> _Dropping_ this change would also be more robust, in cases something somewhere _forgets_ to announce the code cache was changed? If we don't trust the downstream code about this and just summarily invalidate, it feels safer.
@shipilev
> But I wonder how much this buys us?
SPECjvm crypto.aes is the case for this change.
When I ran it on Graviton 2, 64 cores I got the following scores:
- Baseline: 1047(min), 1058(max), 1052(geomean)
- PR: 1058(min), 1078(max), 1069(geomean), +1.6% improvement
**OPTION 1**: If we don't track code modification in `fix_oop_relocations` and assume any call of `fix_oop_relocations` requires icache invalidation, results are the following:
- 1032(min), 1057(max), 1045(geomean), -0.7% regression
**OPTION 2**: If we revert `fix_metadata_relocation` but add `modified_code |= !reloc->metadata_is_immediate()`, results are the following:
- 1033(min), 1060(max), 1047(geomean), -0.5% regression
You can see there is not much difference between OPTION 1 and 2.
I checked how many icache invalidations we had:
- Baseline (average): 12 263 308
- PR (average): 3 460 323
- OPTION 1 (average): 3 719 972
- OPTION 2 (average): 3 648 767
This also confirms that OPTION 1 and 2 are not much different.
There might be OPTION 3. `fix_metadata_relocation` is only implemented for ARM32. It is no-op for other platforms. Maybe we can move `fix_metadata_relocation` to the arm32 backend, remove it from other backends and add the call of `fix_metadata_relocation` under `#ifdef ARM32`. Are there any plans to remove the arm32 backend in the future?
> Dropping this change would also be more robust, in cases something somewhere forgets to announce the code cache was changed?
SPECjvm shows that forgetting could cost performance if the code is on the hot path.
I think we should go with the change in `fix_metadata_relocation` this PR introduces or OPTION 3.
What do you think?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28328#discussion_r2854402066
More information about the shenandoah-dev
mailing list