RFR: 8317562: [JFR] Compilation queue statistics
Adding a new periodic jfr event to monitor and output statistics for the compiler queues. You will see one event per compiler queue (c1 and c2) Passes tier1 on linux (x86) and mac (aarch64) ------------- Commit messages: - jfr profiles - added requirement that java test is not run with interperter only - Addressed feedback: fix coding style and replaces int with uint - Replaced magic numbers - Update src/hotspot/share/jfr/metadata/metadata.xml - Update src/hotspot/share/jfr/periodic/jfrCompilerQueueUtilization.cpp - Update src/hotspot/share/jfr/periodic/jfrCompilerQueueUtilization.cpp - Update src/hotspot/share/jfr/periodic/jfrCompilerQueueUtilization.cpp - Add periodic jfr CompilerQueueUtilization event Changes: https://git.openjdk.org/jdk/pull/16211/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=16211&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8317562 Stats: 248 lines in 10 files changed: 248 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/16211.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16211/head:pull/16211 PR: https://git.openjdk.org/jdk/pull/16211
On Tue, 17 Oct 2023 00:35:54 GMT, Mat Carter <macarte@openjdk.org> wrote:
Adding a new periodic jfr event to monitor and output statistics for the compiler queues. You will see one event per compiler queue (c1 and c2)
Passes tier1 on linux (x86) and mac (aarch64)
very nitpicky - take or leave the suggestions :-) src/hotspot/share/compiler/compileBroker.hpp line 121:
119: int size() const { return _size; } 120: 121: int get_peak_size() const { return _peak_size; }
whitespace looks out by one? src/hotspot/share/jfr/metadata/metadata.xml line 861:
859: <Field type="long" contentType="count" name="totalAdded" label="Total Added" description="Total requests"/> 860: <Field type="long" contentType="count" name="totalRemoved" label="Total Removed" description="Total removed requests"/> 861: <Field type="int" name="compilerThreadCount" label="Number of Compiler Threads for this queue" />
Suggestion: <Field type="int" name="compilerThreadCount" label="Number of Compiler Threads for this queue"/> src/hotspot/share/jfr/periodic/jfrCompilerQueueUtilization.cpp line 37:
35: }; 36: 37: // If current counters are less than previous we assume the interface has been reset
Suggestion: // If current counters are less than previous, we assume the interface has been reset src/hotspot/share/jfr/periodic/jfrCompilerQueueUtilization.cpp line 49:
47: void JfrCompilerQueueUtilization::send_events() { 48: static CompilerQueueEntry compilerQueueEntries[2] = { 49: {CompileBroker::c1_compile_queue(), 1, 0,0},
Suggestion: {CompileBroker::c1_compile_queue(), 1, 0, 0}, src/hotspot/share/jfr/periodic/jfrCompilerQueueUtilization.cpp line 50:
48: static CompilerQueueEntry compilerQueueEntries[2] = { 49: {CompileBroker::c1_compile_queue(), 1, 0,0}, 50: {CompileBroker::c2_compile_queue(), 2, 0,0}};
Suggestion: {CompileBroker::c2_compile_queue(), 2, 0, 0}}; src/hotspot/share/jfr/periodic/jfrCompilerQueueUtilization.cpp line 55:
53: static JfrTicks last_sample_instant; 54: const JfrTickspan interval = cur_time - last_sample_instant; 55: for(int i = 0; i < 2; i ++)
2 is a magic number, maybe a comment above the for declartion explaining why? ------------- Changes requested by karianna (no project role). PR Review: https://git.openjdk.org/jdk/pull/16211#pullrequestreview-1681156472 PR Review Comment: https://git.openjdk.org/jdk/pull/16211#discussion_r1361399313 PR Review Comment: https://git.openjdk.org/jdk/pull/16211#discussion_r1361399583 PR Review Comment: https://git.openjdk.org/jdk/pull/16211#discussion_r1361399733 PR Review Comment: https://git.openjdk.org/jdk/pull/16211#discussion_r1361400079 PR Review Comment: https://git.openjdk.org/jdk/pull/16211#discussion_r1361400114 PR Review Comment: https://git.openjdk.org/jdk/pull/16211#discussion_r1361400356
On Tue, 17 Oct 2023 01:06:29 GMT, Martijn Verburg <karianna@openjdk.org> wrote:
Adding a new periodic jfr event to monitor and output statistics for the compiler queues. You will see one event per compiler queue (c1 and c2)
Passes tier1 on linux (x86) and mac (aarch64)
src/hotspot/share/jfr/periodic/jfrCompilerQueueUtilization.cpp line 55:
53: static JfrTicks last_sample_instant; 54: const JfrTickspan interval = cur_time - last_sample_instant; 55: for(int i = 0; i < 2; i ++)
2 is a magic number, maybe a comment above the for declartion explaining why?
Thanks for the feedback, made this clearer ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/16211#discussion_r1362382032
On Tue, 17 Oct 2023 00:35:54 GMT, Mat Carter <macarte@openjdk.org> wrote:
Adding a new periodic jfr event to monitor and output statistics for the compiler queues. You will see one event per compiler queue (c1 and c2)
Passes tier1 on linux (x86) and mac (aarch64)
Some quick comments. src/hotspot/share/compiler/compileBroker.hpp line 91:
89: 90: int _size; 91: int _total_added;
Can total be negative? src/hotspot/share/jfr/periodic/jfrCompilerQueueUtilization.cpp line 55:
53: return ((current - old) * NANOSECS_PER_SEC) / interval.nanoseconds(); 54: } 55:
NIT: Too many new lines? src/hotspot/share/jfr/periodic/jfrCompilerQueueUtilization.cpp line 67:
65: const JfrTickspan interval = cur_time - last_sample_instant; 66: for(int i = 0; i < num_compiler_queues; i ++) 67: {
I think this doesn't follow the coding style. See document in "/doc/" folder in the repository. test/jdk/jdk/jfr/event/compiler/TestCompilerQueueUtilization.java line 36:
34: * @test 35: * @key jfr 36: * @requires vm.hasJFR
Does it need C1 and/or C2? ------------- PR Review: https://git.openjdk.org/jdk/pull/16211#pullrequestreview-1682911744 PR Review Comment: https://git.openjdk.org/jdk/pull/16211#discussion_r1362487113 PR Review Comment: https://git.openjdk.org/jdk/pull/16211#discussion_r1362485381 PR Review Comment: https://git.openjdk.org/jdk/pull/16211#discussion_r1362486361 PR Review Comment: https://git.openjdk.org/jdk/pull/16211#discussion_r1362480494
On Tue, 17 Oct 2023 17:04:02 GMT, Cesar Soares Lucas <cslucas@openjdk.org> wrote:
Adding a new periodic jfr event to monitor and output statistics for the compiler queues. You will see one event per compiler queue (c1 and c2)
Passes tier1 on linux (x86) and mac (aarch64)
src/hotspot/share/compiler/compileBroker.hpp line 91:
89: 90: int _size; 91: int _total_added;
Can total be negative?
no - changed to uint
test/jdk/jdk/jfr/event/compiler/TestCompilerQueueUtilization.java line 36:
34: * @test 35: * @key jfr 36: * @requires vm.hasJFR
Does it need C1 and/or C2?
yes requires c1 or c2 ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/16211#discussion_r1362628813 PR Review Comment: https://git.openjdk.org/jdk/pull/16211#discussion_r1362629511
On Tue, 17 Oct 2023 19:04:00 GMT, Mat Carter <macarte@openjdk.org> wrote:
test/jdk/jdk/jfr/event/compiler/TestCompilerQueueUtilization.java line 36:
34: * @test 35: * @key jfr 36: * @requires vm.hasJFR
Does it need C1 and/or C2?
yes requires c1 or c2
added requirement that java is NOT run with Xint (interpreter mode) ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/16211#discussion_r1362633864
On Tue, 17 Oct 2023 00:35:54 GMT, Mat Carter <macarte@openjdk.org> wrote:
Adding a new periodic jfr event to monitor and output statistics for the compiler queues. You will see one event per compiler queue (c1 and c2)
Passes tier1 on linux (x86) and mac (aarch64)
Would be interesting to test it with Graal which use `c2_compile_queue` (Graal JIT replaces C2). src/hotspot/share/compiler/compileBroker.cpp line 530:
528: return _c2_compile_queue; 529: } 530:
Note, `*_compiler_queue` could be `nullptr` if VM is build without C2 or C1 or when run with `-XX:-TieredCompilation` (only C2 is used) or with `-XX:TierdStopAtLevel={1,2,3}` (only C1 is used). Make sure you check it in JFR event. src/hotspot/share/compiler/compileBroker.hpp line 123:
121: int get_peak_size() const { return _peak_size; } 122: int get_total_added() const { return _total_added; } 123: int get_total_removed() const { return _total_removed; }
Fields are `uint` type. These accessors should also return `uint`. ------------- PR Review: https://git.openjdk.org/jdk/pull/16211#pullrequestreview-1685714293 PR Review Comment: https://git.openjdk.org/jdk/pull/16211#discussion_r1364277126 PR Review Comment: https://git.openjdk.org/jdk/pull/16211#discussion_r1364268812
On Wed, 18 Oct 2023 17:35:59 GMT, Vladimir Kozlov <kvn@openjdk.org> wrote:
Mat Carter has updated the pull request incrementally with one additional commit since the last revision:
fixed return type and changed NULL to nullptr
src/hotspot/share/compiler/compileBroker.cpp line 530:
528: return _c2_compile_queue; 529: } 530:
Note, `*_compiler_queue` could be `nullptr` if VM is build without C2 or C1 or when run with `-XX:-TieredCompilation` (only C2 is used) or with `-XX:TierdStopAtLevel={1,2,3}` (only C1 is used).
Make sure you check it in JFR event.
Thank you! The JFR event does check for NULL (now nullptr) ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/16211#discussion_r1364317003
If there are no concerns with the JFR event, it’s initial settings or the test, given that the small compiler changes have already been reviewed would someone be kind enough to review and sponsor the commit Thanks in advance Mat From: hotspot-jfr-dev <hotspot-jfr-dev-retn@openjdk.org> on behalf of Mat Carter <macarte@openjdk.org> Date: Wednesday, October 18, 2023 at 11:12 AM To: hotspot-dev@openjdk.org <hotspot-dev@openjdk.org>, hotspot-jfr-dev@openjdk.org <hotspot-jfr-dev@openjdk.org> Subject: Re: RFR: 8317562: [JFR] Compilation queue statistics [v2] On Wed, 18 Oct 2023 17:35:59 GMT, Vladimir Kozlov <kvn@openjdk.org> wrote:
Mat Carter has updated the pull request incrementally with one additional commit since the last revision:
fixed return type and changed NULL to nullptr
src/hotspot/share/compiler/compileBroker.cpp line 530:
528: return _c2_compile_queue; 529: } 530:
Note, `*_compiler_queue` could be `nullptr` if VM is build without C2 or C1 or when run with `-XX:-TieredCompilation` (only C2 is used) or with `-XX:TierdStopAtLevel={1,2,3}` (only C1 is used).
Make sure you check it in JFR event.
Thank you! The JFR event does check for NULL (now nullptr) ------------- PR Review Comment: https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.openjdk.org%2Fjdk%2Fpull%2F16211%23discussion_r1364317003&data=05%7C01%7Cmatthew.carter%40microsoft.com%7C4c80af72893e4fb6326408dbd005cf39%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C638332495614878626%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=5wvfj1sZg3QJyoivoKGyz7xD3gO1lG3wRhuqFpt7sJY%3D&reserved=0<https://git.openjdk.org/jdk/pull/16211#discussion_r1364317003>
Adding a new periodic jfr event to monitor and output statistics for the compiler queues. You will see one event per compiler queue (c1 and c2)
Passes tier1 on linux (x86) and mac (aarch64)
Mat Carter has updated the pull request incrementally with one additional commit since the last revision: fixed return type and changed NULL to nullptr ------------- Changes: - all: https://git.openjdk.org/jdk/pull/16211/files - new: https://git.openjdk.org/jdk/pull/16211/files/6413be5b..6c0b1670 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=16211&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16211&range=00-01 Stats: 3 lines in 2 files changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/16211.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16211/head:pull/16211 PR: https://git.openjdk.org/jdk/pull/16211
On Wed, 18 Oct 2023 18:11:45 GMT, Mat Carter <macarte@openjdk.org> wrote:
Adding a new periodic jfr event to monitor and output statistics for the compiler queues. You will see one event per compiler queue (c1 and c2)
Passes tier1 on linux (x86) and mac (aarch64)
Mat Carter has updated the pull request incrementally with one additional commit since the last revision:
fixed return type and changed NULL to nullptr
Good for compiler part of changes. ------------- Marked as reviewed by kvn (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/16211#pullrequestreview-1685826018
On Wed, 18 Oct 2023 18:11:45 GMT, Mat Carter <macarte@openjdk.org> wrote:
Adding a new periodic jfr event to monitor and output statistics for the compiler queues. You will see one event per compiler queue (c1 and c2)
Passes tier1 on linux (x86) and mac (aarch64)
Mat Carter has updated the pull request incrementally with one additional commit since the last revision:
fixed return type and changed NULL to nullptr
test/jdk/jdk/jfr/event/compiler/TestCompilerQueueUtilization.java line 48:
46: recording.enable(EVENT_NAME); 47: recording.start(); 48: recording.stop();
Would it be useful to delay the recording stop in order to give more of an opportunity for compiler events to occur? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/16211#discussion_r1375076209
On Fri, 27 Oct 2023 21:53:38 GMT, Brian Stafford <bstafford@openjdk.org> wrote:
Mat Carter has updated the pull request incrementally with one additional commit since the last revision:
fixed return type and changed NULL to nullptr
test/jdk/jdk/jfr/event/compiler/TestCompilerQueueUtilization.java line 48:
46: recording.enable(EVENT_NAME); 47: recording.start(); 48: recording.stop();
Would it be useful to delay the recording stop in order to give more of an opportunity for compiler events to occur?
I've followed the pattern for other events that output periodically and don't want to add unnecessary time as that will cause the test suite to take longer, the test will fail if there are no events due to the line 'Events.hasEvents(events)' ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/16211#discussion_r1375145759
On Wed, 18 Oct 2023 18:11:45 GMT, Mat Carter <macarte@openjdk.org> wrote:
Adding a new periodic jfr event to monitor and output statistics for the compiler queues. You will see one event per compiler queue (c1 and c2)
Passes tier1 on linux (x86) and mac (aarch64)
Mat Carter has updated the pull request incrementally with one additional commit since the last revision:
fixed return type and changed NULL to nullptr
src/hotspot/share/jfr/metadata/metadata.xml line 857:
855: <Field type="long" contentType="count" name="size" label="Size" description="Current queue size"/> 856: <Field type="long" contentType="count" name="peak" label="Peak Size" description="Peak queue size"/> 857: <Field type="long" contentType="count" name="added" label="Added" description="Added requests"/>
There is no contentType called count. Please remove. No need to use a description, if it's just a repetition of the name. test/jdk/jdk/jfr/event/compiler/TestCompilerQueueUtilization.java line 2:
1: /* 2: * Copyright (c) 2013, 2023, Oracle and/or its affiliates. All rights reserved.
Why copyright 2013, the file is new. test/jdk/jdk/jfr/event/compiler/TestCompilerQueueUtilization.java line 45:
43: 44: public static void main(String[] args) throws Exception { 45: Recording recording = new Recording();
Use try-with-resources ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/16211#discussion_r1366464679 PR Review Comment: https://git.openjdk.org/jdk/pull/16211#discussion_r1366477481 PR Review Comment: https://git.openjdk.org/jdk/pull/16211#discussion_r1366464793
On Fri, 20 Oct 2023 04:55:24 GMT, Erik Gahlin <egahlin@openjdk.org> wrote:
Mat Carter has updated the pull request incrementally with one additional commit since the last revision:
fixed return type and changed NULL to nullptr
src/hotspot/share/jfr/metadata/metadata.xml line 857:
855: <Field type="long" contentType="count" name="size" label="Size" description="Current queue size"/> 856: <Field type="long" contentType="count" name="peak" label="Peak Size" description="Peak queue size"/> 857: <Field type="long" contentType="count" name="added" label="Added" description="Added requests"/>
There is no contentType called count. Please remove.
No need to use a description, if it's just a repetition of the name.
-removed contenttType="count" -removed superfluous descriptions
test/jdk/jdk/jfr/event/compiler/TestCompilerQueueUtilization.java line 2:
1: /* 2: * Copyright (c) 2013, 2023, Oracle and/or its affiliates. All rights reserved.
Why copyright 2013, the file is new.
copy paste mistake (fixed) ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/16211#discussion_r1375145353 PR Review Comment: https://git.openjdk.org/jdk/pull/16211#discussion_r1375145237
On Fri, 20 Oct 2023 04:55:44 GMT, Erik Gahlin <egahlin@openjdk.org> wrote:
Mat Carter has updated the pull request incrementally with one additional commit since the last revision:
fixed return type and changed NULL to nullptr
test/jdk/jdk/jfr/event/compiler/TestCompilerQueueUtilization.java line 45:
43: 44: public static void main(String[] args) throws Exception { 45: Recording recording = new Recording();
Use try-with-resources
Added ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/16211#discussion_r1375145987
Adding a new periodic jfr event to monitor and output statistics for the compiler queues. You will see one event per compiler queue (c1 and c2)
Passes tier1 on linux (x86) and mac (aarch64)
Mat Carter has updated the pull request incrementally with one additional commit since the last revision: Addressed review comments ------------- Changes: - all: https://git.openjdk.org/jdk/pull/16211/files - new: https://git.openjdk.org/jdk/pull/16211/files/6c0b1670..310ce342 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=16211&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16211&range=01-02 Stats: 26 lines in 2 files changed: 1 ins; 0 del; 25 mod Patch: https://git.openjdk.org/jdk/pull/16211.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16211/head:pull/16211 PR: https://git.openjdk.org/jdk/pull/16211
On Sat, 28 Oct 2023 01:52:08 GMT, Mat Carter <macarte@openjdk.org> wrote:
Adding a new periodic jfr event to monitor and output statistics for the compiler queues. You will see one event per compiler queue (c1 and c2)
Passes tier1 on linux (x86) and mac (aarch64)
Mat Carter has updated the pull request incrementally with one additional commit since the last revision:
Addressed review comments
src/hotspot/share/jfr/metadata/metadata.xml line 853:
851: <Event name="CompilerQueueUtilization" category="Java Virtual Machine, Compiler" label="Compiler Queue Utilization" period="everyChunk"> 852: <Field type="CompilerType" name="compiler" label="Compiler" /> 853: <Field type="long" contentType="hertz" name="ingress" label="Ingress" description="Number of added requests per second"/>
Could we use other terms then ingress and egress? Something that is more in general use, src/hotspot/share/jfr/metadata/metadata.xml line 855:
853: <Field type="long" contentType="hertz" name="ingress" label="Ingress" description="Number of added requests per second"/> 854: <Field type="long" contentType="hertz" name="egress" label="Egress" description="Number of removed requests per second"/> 855: <Field type="long" name="size" label="Size"/>
The label could be more clarifying? Queue Size? src/hotspot/share/jfr/metadata/metadata.xml line 856:
854: <Field type="long" contentType="hertz" name="egress" label="Egress" description="Number of removed requests per second"/> 855: <Field type="long" name="size" label="Size"/> 856: <Field type="long" name="peak" label="Peak Size"/>
peakQueueSize? src/hotspot/share/jfr/metadata/metadata.xml line 861:
859: <Field type="long" name="totalAdded" label="Total Added"/> 860: <Field type="long" name="totalRemoved" label="Total Removed"/> 861: <Field type="int" name="compilerThreadCount" label="Number of Compiler Threads for this queue"/>
The label should be short and use headline-style capitalization, how about "Compiler Thread Count"? https://docs.oracle.com/en/java/javase/21/docs/api/jdk.jfr/jdk/jfr/Label.htm... src/hotspot/share/jfr/periodic/jfrCompilerQueueUtilization.cpp line 53:
51: return 0; 52: } 53: return ((current - old) * NANOSECS_PER_SEC) / interval.nanoseconds();
Shouldn't it be ticks per second here? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/16211#discussion_r1375505798 PR Review Comment: https://git.openjdk.org/jdk/pull/16211#discussion_r1375508747 PR Review Comment: https://git.openjdk.org/jdk/pull/16211#discussion_r1375508798 PR Review Comment: https://git.openjdk.org/jdk/pull/16211#discussion_r1375505254 PR Review Comment: https://git.openjdk.org/jdk/pull/16211#discussion_r1375508417
On Sun, 29 Oct 2023 20:19:21 GMT, Erik Gahlin <egahlin@openjdk.org> wrote:
Mat Carter has updated the pull request incrementally with one additional commit since the last revision:
Addressed review comments
src/hotspot/share/jfr/metadata/metadata.xml line 853:
851: <Event name="CompilerQueueUtilization" category="Java Virtual Machine, Compiler" label="Compiler Queue Utilization" period="everyChunk"> 852: <Field type="CompilerType" name="compiler" label="Compiler" /> 853: <Field type="long" contentType="hertz" name="ingress" label="Ingress" description="Number of added requests per second"/>
Could we use other terms then ingress and egress? Something that is more in general use,
Thought about enqueue/dequeue, but went with added/removed so its consistent with the other fields
src/hotspot/share/jfr/metadata/metadata.xml line 861:
859: <Field type="long" name="totalAdded" label="Total Added"/> 860: <Field type="long" name="totalRemoved" label="Total Removed"/> 861: <Field type="int" name="compilerThreadCount" label="Number of Compiler Threads for this queue"/>
The label should be short and use headline-style capitalization, how about "Compiler Thread Count"? https://docs.oracle.com/en/java/javase/21/docs/api/jdk.jfr/jdk/jfr/Label.htm...
Fixed, thank you for the reference
src/hotspot/share/jfr/periodic/jfrCompilerQueueUtilization.cpp line 53:
51: return 0; 52: } 53: return ((current - old) * NANOSECS_PER_SEC) / interval.nanoseconds();
Shouldn't it be ticks per second here?
This resolves to ticks per second; could replace with (current - old) / interval.seconds() but this introduces floats. This was taken from NetworkUtilization, I assume they went this way to keep the math in integers ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/16211#discussion_r1379036697 PR Review Comment: https://git.openjdk.org/jdk/pull/16211#discussion_r1379037454 PR Review Comment: https://git.openjdk.org/jdk/pull/16211#discussion_r1379039332
Adding a new periodic jfr event to monitor and output statistics for the compiler queues. You will see one event per compiler queue (c1 and c2)
Passes tier1 on linux (x86) and mac (aarch64)
Mat Carter has updated the pull request incrementally with one additional commit since the last revision: Naming changes based on review comments ------------- Changes: - all: https://git.openjdk.org/jdk/pull/16211/files - new: https://git.openjdk.org/jdk/pull/16211/files/310ce342..5474ff61 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=16211&range=03 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16211&range=02-03 Stats: 19 lines in 2 files changed: 0 ins; 0 del; 19 mod Patch: https://git.openjdk.org/jdk/pull/16211.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16211/head:pull/16211 PR: https://git.openjdk.org/jdk/pull/16211
Adding a new periodic jfr event to monitor and output statistics for the compiler queues. You will see one event per compiler queue (c1 and c2)
Passes tier1 on linux (x86) and mac (aarch64)
Mat Carter has updated the pull request incrementally with two additional commits since the last revision: - Fixed copyright notice - Fixed copyright notice ------------- Changes: - all: https://git.openjdk.org/jdk/pull/16211/files - new: https://git.openjdk.org/jdk/pull/16211/files/5474ff61..08b2dbc2 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=16211&range=04 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16211&range=03-04 Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/16211.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16211/head:pull/16211 PR: https://git.openjdk.org/jdk/pull/16211
On Thu, 2 Nov 2023 02:04:28 GMT, Mat Carter <macarte@openjdk.org> wrote:
Adding a new periodic jfr event to monitor and output statistics for the compiler queues. You will see one event per compiler queue (c1 and c2)
Passes tier1 on linux (x86) and mac (aarch64)
Mat Carter has updated the pull request incrementally with two additional commits since the last revision:
- Fixed copyright notice - Fixed copyright notice
Hi , we are seeing now the following error in our tests (with this PR added) when running jdk/jfr/event/compiler/TestCompilerQueueUtilization.java java.lang.RuntimeException: Field ingress not in struct at jdk.test.lib.Asserts.fail(Asserts.java:634) at jdk.test.lib.jfr.Events.getValueDescriptor(Events.java:154) at jdk.test.lib.jfr.Events.assertField(Events.java:69) at jdk.jfr.event.compiler.TestCompilerQueueUtilization.main(TestCompilerQueueUtilization.java:55) at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103) at java.base/java.lang.reflect.Method.invoke(Method.java:580) at com.sun.javatest.regtest.agent.MainWrapper$MainTask.run(MainWrapper.java:138) at java.base/java.lang.Thread.run(Thread.java:1570) java.lang.RuntimeException: Field ingress not in event at jdk.test.lib.Asserts.fail(Asserts.java:634) at jdk.test.lib.jfr.Events.assertField(Events.java:81) at jdk.jfr.event.compiler.TestCompilerQueueUtilization.main(TestCompilerQueueUtilization.java:55) at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103) at java.base/java.lang.reflect.Method.invoke(Method.java:580) at com.sun.javatest.regtest.agent.MainWrapper$MainTask.run(MainWrapper.java:138) at java.base/java.lang.Thread.run(Thread.java:1570) ------------- PR Comment: https://git.openjdk.org/jdk/pull/16211#issuecomment-1790667977
On Thu, 2 Nov 2023 12:50:33 GMT, Matthias Baesken <mbaesken@openjdk.org> wrote:
Hi , we are seeing now the following error in our tests (with this PR added) when running jdk/jfr/event/compiler/TestCompilerQueueUtilization.java
java.lang.RuntimeException: Field ingress not in struct at jdk.test.lib.Asserts.fail(Asserts.java:634) at jdk.test.lib.jfr.Events.getValueDescriptor(Events.java:154) at jdk.test.lib.jfr.Events.assertField(Events.java:69) at jdk.jfr.event.compiler.TestCompilerQueueUtilization.main(TestCompilerQueueUtilization.java:55) at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103) at java.base/java.lang.reflect.Method.invoke(Method.java:580) at com.sun.javatest.regtest.agent.MainWrapper$MainTask.run(MainWrapper.java:138) at java.base/java.lang.Thread.run(Thread.java:1570) java.lang.RuntimeException: Field ingress not in event at jdk.test.lib.Asserts.fail(Asserts.java:634) at jdk.test.lib.jfr.Events.assertField(Events.java:81) at jdk.jfr.event.compiler.TestCompilerQueueUtilization.main(TestCompilerQueueUtilization.java:55) at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103) at java.base/java .lang.reflect.Method.invoke(Method.java:580) at com.sun.javatest.regtest.agent.MainWrapper$MainTask.run(MainWrapper.java:138) at java.base/java.lang.Thread.run(Thread.java:1570)
Thank you for bringing this to my attention, I had failed to commit the updated test (to reflect the field name changes), it's not part of tier1 tests and so the github checks passed I have now committed the updated test file ------------- PR Comment: https://git.openjdk.org/jdk/pull/16211#issuecomment-1791065806
Adding a new periodic jfr event to monitor and output statistics for the compiler queues. You will see one event per compiler queue (c1 and c2)
Passes tier1 on linux (x86) and mac (aarch64)
Mat Carter has updated the pull request incrementally with one additional commit since the last revision: Updated test to reflect field name changes ------------- Changes: - all: https://git.openjdk.org/jdk/pull/16211/files - new: https://git.openjdk.org/jdk/pull/16211/files/08b2dbc2..4a1dfbf7 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=16211&range=05 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16211&range=04-05 Stats: 8 lines in 1 file changed: 0 ins; 0 del; 8 mod Patch: https://git.openjdk.org/jdk/pull/16211.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16211/head:pull/16211 PR: https://git.openjdk.org/jdk/pull/16211
On Thu, 2 Nov 2023 16:30:32 GMT, Mat Carter <macarte@openjdk.org> wrote:
Adding a new periodic jfr event to monitor and output statistics for the compiler queues. You will see one event per compiler queue (c1 and c2)
Passes tier1 on linux (x86) and mac (aarch64)
Mat Carter has updated the pull request incrementally with one additional commit since the last revision:
Updated test to reflect field name changes
With the updated test file, the jtreg test error is gone. ------------- PR Comment: https://git.openjdk.org/jdk/pull/16211#issuecomment-1792272259
Thank you all for the reviews and testing. Are there any further changes or questions regarding the implementation or intent of the event? Are there any additional steps required before this PR can be sponsored? Should we wait until after the Nov 28 code freeze for the January PSU? Thanks in advance Mat From: hotspot-jfr-dev <hotspot-jfr-dev-retn@openjdk.org> on behalf of Matthias Baesken <mbaesken@openjdk.org> Date: Friday, November 3, 2023 at 4:28 AM To: hotspot-dev@openjdk.org <hotspot-dev@openjdk.org>, hotspot-jfr-dev@openjdk.org <hotspot-jfr-dev@openjdk.org> Subject: Re: RFR: 8317562: [JFR] Compilation queue statistics [v6] On Thu, 2 Nov 2023 16:30:32 GMT, Mat Carter <macarte@openjdk.org> wrote:
Adding a new periodic jfr event to monitor and output statistics for the compiler queues. You will see one event per compiler queue (c1 and c2)
Passes tier1 on linux (x86) and mac (aarch64)
Mat Carter has updated the pull request incrementally with one additional commit since the last revision:
Updated test to reflect field name changes
With the updated test file, the jtreg test error is gone. ------------- PR Comment: https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.openjdk.org%2Fjdk%2Fpull%2F16211%23issuecomment-1792272259&data=05%7C01%7Cmatthew.carter%40microsoft.com%7C56d1d476d6304c572cb708dbdc6005cb%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C638346077226632897%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=MaHRypOjqcNhvRbmdyN%2FXXKQtIQzZY4jNJm5TOsj9ZU%3D&reserved=0<https://git.openjdk.org/jdk/pull/16211#issuecomment-1792272259>
On Thu, 2 Nov 2023 16:30:32 GMT, Mat Carter <macarte@openjdk.org> wrote:
Adding a new periodic jfr event to monitor and output statistics for the compiler queues. You will see one event per compiler queue (c1 and c2)
Passes tier1 on linux (x86) and mac (aarch64)
Mat Carter has updated the pull request incrementally with one additional commit since the last revision:
Updated test to reflect field name changes
@egahlin / @vnkozlov / @MBaesken / @brianjstafford / @JohnTortugo / @karianna - thank you for your reviews and feedback! ------------- PR Comment: https://git.openjdk.org/jdk/pull/16211#issuecomment-1804169861
On Tue, 17 Oct 2023 00:35:54 GMT, Mat Carter <macarte@openjdk.org> wrote:
Adding a new periodic jfr event to monitor and output statistics for the compiler queues. You will see one event per compiler queue (c1 and c2)
Passes tier1 on linux (x86) and mac (aarch64)
This pull request has now been integrated. Changeset: d9920334 Author: Mat Carter <macarte@openjdk.org> Committer: Vladimir Kozlov <kvn@openjdk.org> URL: https://git.openjdk.org/jdk/commit/d992033439073d35877a2c0296fbd01ad5cbcb07 Stats: 249 lines in 10 files changed: 249 ins; 0 del; 0 mod 8317562: [JFR] Compilation queue statistics Reviewed-by: kvn ------------- PR: https://git.openjdk.org/jdk/pull/16211
participants (8)
-
Brian Stafford
-
Cesar Soares Lucas
-
Erik Gahlin
-
Martijn Verburg
-
Mat Carter
-
Mat Carter
-
Matthias Baesken
-
Vladimir Kozlov