RFR: 8343026: JFR: Index into fields in the topFrame
Could I have a review of a change that allows 'jfr view' queries to reference nested fields for a top frame. Testing: jdk/jdk/jfr and diff of 'jfr view all-views' before and after the change. This change will not impact the output of the exiting views, but it helps OpenJDK developers diagnose issues, for example, print 'lineNumber', (frame) 'type' and 'bytecodeIndex' of the top frame. Thanks Erik ------------- Commit messages: - Initial Changes: https://git.openjdk.org/jdk/pull/21705/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=21705&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8343026 Stats: 175 lines in 4 files changed: 88 ins; 81 del; 6 mod Patch: https://git.openjdk.org/jdk/pull/21705.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21705/head:pull/21705 PR: https://git.openjdk.org/jdk/pull/21705
On Fri, 25 Oct 2024 07:30:56 GMT, Erik Gahlin <egahlin@openjdk.org> wrote:
Could I have a review of a change that allows 'jfr view' queries to reference nested fields for a top frame.
Testing: jdk/jdk/jfr and diff of 'jfr view all-views' before and after the change.
This change will not impact the output of the exiting views, but it helps OpenJDK developers diagnose issues, for example, print 'lineNumber', (frame) 'type' and 'bytecodeIndex' of the top frame.
Thanks Erik
Looks good! ------------- Marked as reviewed by mgronlun (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/21705#pullrequestreview-2397517772
On Fri, 25 Oct 2024 07:30:56 GMT, Erik Gahlin <egahlin@openjdk.org> wrote:
Could I have a review of a change that allows 'jfr view' queries to reference nested fields for a top frame.
Testing: jdk/jdk/jfr and diff of 'jfr view all-views' before and after the change.
This change will not impact the output of the exiting views, but it helps OpenJDK developers diagnose issues, for example, print 'lineNumber', (frame) 'type' and 'bytecodeIndex' of the top frame.
Thanks Erik
src/jdk.jfr/share/classes/jdk/jfr/internal/query/FieldBuilder.java line 177:
175: 176: private static RecordedFrame topFrame(RecordedEvent event) { 177: return findJavaFrame(event, x -> true);
Suggestion: return findJavaFrame(event, x -> true); ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/21705#discussion_r1819741115
Could I have a review of a change that allows 'jfr view' queries to reference nested fields for a top frame.
Testing: jdk/jdk/jfr and diff of 'jfr view all-views' before and after the change.
This change will not impact the output of the exiting views, but it helps OpenJDK developers diagnose issues, for example, print 'lineNumber', (frame) 'type' and 'bytecodeIndex' of the top frame.
Thanks Erik
Erik Gahlin has updated the pull request incrementally with one additional commit since the last revision: Remove whitespace ------------- Changes: - all: https://git.openjdk.org/jdk/pull/21705/files - new: https://git.openjdk.org/jdk/pull/21705/files/2fc98da1..c90c82e0 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=21705&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=21705&range=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/21705.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21705/head:pull/21705 PR: https://git.openjdk.org/jdk/pull/21705
On Tue, 29 Oct 2024 14:06:54 GMT, Erik Gahlin <egahlin@openjdk.org> wrote:
Could I have a review of a change that allows 'jfr view' queries to reference nested fields for a top frame.
Testing: jdk/jdk/jfr and diff of 'jfr view all-views' before and after the change.
This change will not impact the output of the exiting views, but it helps OpenJDK developers diagnose issues, for example, print 'lineNumber', (frame) 'type' and 'bytecodeIndex' of the top frame.
Thanks Erik
Erik Gahlin has updated the pull request incrementally with one additional commit since the last revision:
Remove whitespace
Marked as reviewed by mgronlun (Reviewer). ------------- PR Review: https://git.openjdk.org/jdk/pull/21705#pullrequestreview-2402045307
On Tue, 29 Oct 2024 14:06:54 GMT, Erik Gahlin <egahlin@openjdk.org> wrote:
Could I have a review of a change that allows 'jfr view' queries to reference nested fields for a top frame.
Testing: jdk/jdk/jfr and diff of 'jfr view all-views' before and after the change.
This change will not impact the output of the exiting views, but it helps OpenJDK developers diagnose issues, for example, print 'lineNumber', (frame) 'type' and 'bytecodeIndex' of the top frame.
Thanks Erik
Erik Gahlin has updated the pull request incrementally with one additional commit since the last revision:
Remove whitespace
Thanks for the review of the whitespace removal. ------------- PR Comment: https://git.openjdk.org/jdk/pull/21705#issuecomment-2444400874
On Fri, 25 Oct 2024 07:30:56 GMT, Erik Gahlin <egahlin@openjdk.org> wrote:
Could I have a review of a change that allows 'jfr view' queries to reference nested fields for a top frame.
Testing: jdk/jdk/jfr and diff of 'jfr view all-views' before and after the change.
This change will not impact the output of the exiting views, but it helps OpenJDK developers diagnose issues, for example, print 'lineNumber', (frame) 'type' and 'bytecodeIndex' of the top frame.
Thanks Erik
Thanks for the reviews. ------------- PR Comment: https://git.openjdk.org/jdk/pull/21705#issuecomment-2444374279
On Fri, 25 Oct 2024 07:30:56 GMT, Erik Gahlin <egahlin@openjdk.org> wrote:
Could I have a review of a change that allows 'jfr view' queries to reference nested fields for a top frame.
Testing: jdk/jdk/jfr and diff of 'jfr view all-views' before and after the change.
This change will not impact the output of the exiting views, but it helps OpenJDK developers diagnose issues, for example, print 'lineNumber', (frame) 'type' and 'bytecodeIndex' of the top frame.
Thanks Erik
This pull request has now been integrated. Changeset: 7c800e6b Author: Erik Gahlin <egahlin@openjdk.org> URL: https://git.openjdk.org/jdk/commit/7c800e6bae388dd87986f366787398fe99b4e2ee Stats: 175 lines in 4 files changed: 88 ins; 81 del; 6 mod 8343026: JFR: Index into fields in the topFrame Reviewed-by: mgronlun ------------- PR: https://git.openjdk.org/jdk/pull/21705
participants (3)
-
Andrey Turbanov
-
Erik Gahlin
-
Markus Grönlund