RFR: 8292265: Add old gen used field at G1HeapSummary JFR event.
Hi all, Can I have reviews for this change which adds 'old gen used' field at existing G1HeapSummary event? Testing: tier1 ~ tier3 Thanks, Sangheon ------------- Commit messages: - 8292265: Add old gen used field at G1HeapSummary JFR event Changes: https://git.openjdk.org/jdk/pull/11303/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=11303&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8292265 Stats: 10 lines in 4 files changed: 6 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/11303.diff Fetch: git fetch https://git.openjdk.org/jdk pull/11303/head:pull/11303 PR: https://git.openjdk.org/jdk/pull/11303
On Tue, 22 Nov 2022 23:09:52 GMT, Sangheon Kim <sangheki@openjdk.org> wrote:
Hi all,
Can I have reviews for this change which adds 'old gen used' field at existing G1HeapSummary event?
Testing: tier1 ~ tier3
Thanks, Sangheon
Changes requested by tschatzl (Reviewer). src/hotspot/share/gc/g1/g1CollectedHeap.cpp line 2568:
2566: 2567: size_t old_used_bytes = _monitoring_support->old_gen_used(); 2568:
On the one hand using the information from the MXBeans/MonitoringSupport and on the other hand recalculating this data manually, is likely to set us up for inconsistencies. I recommend using `MonitoringSupport` data for as many as possible values here.
From what I can see for young gc, this method is called as part of the `G1JFRTracerMark` which encloses the `G1MonitoringScope`, i.e. the data from `MonitoringSupport` is all up to date. Same seems to be true for full gc.
So getting all data from `MonitoringSupport` here is imo the best way to get consistent data vs. using parts of that data and recalculating others manually. ------------- PR: https://git.openjdk.org/jdk/pull/11303
Hi all,
Can I have reviews for this change which adds 'old gen used' field at existing G1HeapSummary event?
Testing: tier1 ~ tier3
Thanks, Sangheon
Sangheon Kim has updated the pull request incrementally with one additional commit since the last revision: Thomas review ------------- Changes: - all: https://git.openjdk.org/jdk/pull/11303/files - new: https://git.openjdk.org/jdk/pull/11303/files/5c8c1d35..85a4ac3a Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=11303&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=11303&range=00-01 Stats: 7 lines in 1 file changed: 1 ins; 2 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/11303.diff Fetch: git fetch https://git.openjdk.org/jdk pull/11303/head:pull/11303 PR: https://git.openjdk.org/jdk/pull/11303
On Mon, 28 Nov 2022 06:30:42 GMT, Sangheon Kim <sangheki@openjdk.org> wrote:
Hi all,
Can I have reviews for this change which adds 'old gen used' field at existing G1HeapSummary event?
Testing: tier1 ~ tier3
Thanks, Sangheon
Sangheon Kim has updated the pull request incrementally with one additional commit since the last revision:
Thomas review
Lgtm. ------------- Marked as reviewed by tschatzl (Reviewer). PR: https://git.openjdk.org/jdk/pull/11303
On Wed, 30 Nov 2022 09:21:15 GMT, Thomas Schatzl <tschatzl@openjdk.org> wrote:
Sangheon Kim has updated the pull request incrementally with one additional commit since the last revision:
Thomas review
Lgtm.
Thanks @tschatzl and @albertnetymk for your reviews. ------------- PR: https://git.openjdk.org/jdk/pull/11303
On Mon, 28 Nov 2022 06:30:42 GMT, Sangheon Kim <sangheki@openjdk.org> wrote:
Hi all,
Can I have reviews for this change which adds 'old gen used' field at existing G1HeapSummary event?
Testing: tier1 ~ tier3
Thanks, Sangheon
Sangheon Kim has updated the pull request incrementally with one additional commit since the last revision:
Thomas review
src/hotspot/share/gc/g1/g1CollectedHeap.cpp line 2562:
2560: size_t eden_used_bytes = _monitoring_support->eden_space_used(); 2561: size_t survivor_used_bytes = _monitoring_support->survivor_space_used(); 2562: size_t old_used_bytes = _monitoring_support->old_gen_used();
I think it's best to include `gen` in the name (across the whole PR), just to avoid the confusion of Old-region-type vs old-generation. ------------- PR: https://git.openjdk.org/jdk/pull/11303
Hi all,
Can I have reviews for this change which adds 'old gen used' field at existing G1HeapSummary event?
Testing: tier1 ~ tier3
Thanks, Sangheon
Sangheon Kim has updated the pull request incrementally with one additional commit since the last revision: Albert review ------------- Changes: - all: https://git.openjdk.org/jdk/pull/11303/files - new: https://git.openjdk.org/jdk/pull/11303/files/85a4ac3a..d538cbef Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=11303&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=11303&range=01-02 Stats: 8 lines in 4 files changed: 0 ins; 0 del; 8 mod Patch: https://git.openjdk.org/jdk/pull/11303.diff Fetch: git fetch https://git.openjdk.org/jdk pull/11303/head:pull/11303 PR: https://git.openjdk.org/jdk/pull/11303
On Thu, 8 Dec 2022 19:04:40 GMT, Sangheon Kim <sangheki@openjdk.org> wrote:
Hi all,
Can I have reviews for this change which adds 'old gen used' field at existing G1HeapSummary event?
Testing: tier1 ~ tier3
Thanks, Sangheon
Sangheon Kim has updated the pull request incrementally with one additional commit since the last revision:
Albert review
Marked as reviewed by ayang (Reviewer). ------------- PR: https://git.openjdk.org/jdk/pull/11303
On Tue, 22 Nov 2022 23:09:52 GMT, Sangheon Kim <sangheki@openjdk.org> wrote:
Hi all,
Can I have reviews for this change which adds 'old gen used' field at existing G1HeapSummary event?
Testing: tier1 ~ tier3
Thanks, Sangheon
This pull request has now been integrated. Changeset: 8ea2a677 Author: Sangheon Kim <sangheki@openjdk.org> URL: https://git.openjdk.org/jdk/commit/8ea2a6777b68986df9d191f1bf983549d72cb3f8 Stats: 12 lines in 4 files changed: 5 ins; 0 del; 7 mod 8292265: Add old gen used field at G1HeapSummary JFR event Reviewed-by: tschatzl, ayang ------------- PR: https://git.openjdk.org/jdk/pull/11303
participants (3)
-
Albert Mingkun Yang
-
Sangheon Kim
-
Thomas Schatzl