RFR: 8334147: Shenandoah: Avoid taking lock for free set logging [v5]
Xiaolong Peng
xpeng at openjdk.org
Thu Jun 27 05:35:42 UTC 2024
> Hi all,
> This pull request propose a fix for the issue https://bugs.openjdk.org/browse/JDK-8334147
>
>> There are multiple places in Shenandoah where we take heap lock for potential free set diagnostics. There is no point in taking that lock if we do not report anything. We should at very least take the lock only when logging is actually needed.
>
> Basically it adds a public method ```ShenandoahFreeSet::log_status_with_heap_lock``` which acquire lock when there is need to print logs, also make the ```log_status``` private since not expect it to be called out of ShenandoahFreeSet class.
>
> <s>The change for ShenandoahFreeSet::rebuild is probably debatable, ShenandoahFreeSet::log_status will acquire lock again after ShenandoahFreeSet::rebuild is executed, hence the metrics/information in log may not be always consistent, but it might be fine. </s>
>
> Additional test:
> - [x] `make test TEST=hotspot_gc_shenandoah`
>
> Test summary
> ==============================
> TEST TOTAL PASS FAIL ERROR
> jtreg:test/hotspot/jtreg:hotspot_gc_shenandoah 259 259 0 0
> ==============================
> TEST SUCCESS
>
>
> Best,
> Xiaolong.
Xiaolong Peng has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 11 additional commits since the last revision:
- Merge branch 'openjdk:master' into JDK-8334147
- code format
- Renaming
- Missed one log
- Add method log_status_with_heap_lock and make log_status private
- Add shenandoah_assert_not_heaplocked
- 8334147: Shenandoah: Avoid taking lock for free set logging
- Move free_set()->log_status() out of ShenandoahFullGC::phase5_epilog
- code format
- Restructure code of ShenandoahFreeSet::finish_rebuild to move log_status out of it to avoid re-entering heap lock
- ... and 1 more: https://git.openjdk.org/jdk/compare/78c43e3e...8ea36214
-------------
Changes:
- all: https://git.openjdk.org/jdk/pull/19915/files
- new: https://git.openjdk.org/jdk/pull/19915/files/e6895d60..8ea36214
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=19915&range=04
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=19915&range=03-04
Stats: 1378 lines in 20 files changed: 1016 ins; 215 del; 147 mod
Patch: https://git.openjdk.org/jdk/pull/19915.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/19915/head:pull/19915
PR: https://git.openjdk.org/jdk/pull/19915
More information about the shenandoah-dev
mailing list