8211238: @Deprecated JFR event #16931

mgronlun · 2023-12-01T21:15:49Z

Greetings,

please help review this enhancement to add a JFR event to report runtime invocations of methods that have been declared deprecated in the JDK.

Testing: jdk_jfr, CI 1-6, stress testing

Thanks
Markus

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8211238: @Deprecated JFR event (Enhancement - P3)

Reviewers

Erik Gahlin (@egahlin - Reviewer)
Jaroslav Bachorik (@jbachorik - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/16931/head:pull/16931
$ git checkout pull/16931

Update a local copy of the PR:
$ git checkout pull/16931
$ git pull https://git.openjdk.org/jdk.git pull/16931/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 16931

View PR using the GUI difftool:
$ git pr show -t 16931

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/16931.diff

Webrev

Link to Webrev Comment

bridgekeeper · 2023-12-01T21:16:45Z

👋 Welcome back mgronlun! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2023-12-01T21:17:59Z

@mgronlun The following labels will be automatically applied to this pull request:

build
hotspot

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.

mgronlun · 2023-12-01T21:34:53Z

/label add hotspot-jfr

openjdk · 2023-12-01T21:36:26Z

@mgronlun
The hotspot-jfr label was successfully added.

mlbridge · 2023-12-02T11:33:49Z

Webrevs

openjdk · 2023-12-02T17:18:33Z

⚠️ @mgronlun This pull request contains merges that bring in commits not present in the target repository. Since this is not a "merge style" pull request, these changes will be squashed when this pull request in integrated. If this is your intention, then please ignore this message. If you want to preserve the commit structure, you must change the title of this pull request to Merge <project>:<branch> where <project> is the name of another project in the OpenJDK organization (for example Merge jdk:master).

src/jdk.jfr/share/classes/jdk/jfr/internal/test/DeprecatedThing.java

AlanBateman · 2023-12-03T16:33:48Z

A question about "level". Is the intention that the value can be anything, e.g. some new event next month might use the values "1", "2, "3"? Just asking because ordinarily deprecated vs. terminally deprecated is very specific to the manner in which a program element is deprecated and I assume you don't want this event grabbing the general name for a very specific event setting.

mgronlun · 2023-12-03T16:44:28Z

A question about "level". Is the intention that the value can be anything, e.g. some new event next month might use the values "1", "2, "3"? Just asking because ordinarily deprecated vs. terminally deprecated is very specific to the manner in which a program element is deprecated and I assume you don't want this event grabbing the general name for a very specific event setting.

Yes, the design is generic. An event control/setting to be used also for other events.

jbachorik · 2023-12-04T10:15:32Z

Hi @mgronlun - sorry for opening a design discussion in PR :(

I wonder - will this report each single one invocation of a deprecated method conforming to the rules (JDK method called from non-JDK code)? Can this, potentially, flood the recording if the deprecated method gets called from a hot loop?

mgronlun · 2023-12-04T10:37:53Z

Hi @mgronlun - sorry for opening a design discussion in PR :(

I wonder - will this report each single one invocation of a deprecated method conforming to the rules (JDK method called from non-JDK code)? Can this, potentially, flood the recording if the deprecated method gets called from a hot loop?

Hi @jbachorik, it will only report one event per unique call site, during link time. Its not a function of hotness, only unique edge discovery.

magicus · 2023-12-04T11:32:57Z

/label -build

openjdk · 2023-12-04T11:33:38Z

@magicus
The build label was successfully removed.

jbachorik · 2023-12-04T15:08:55Z

Hi @jbachorik, it will only report one event per unique call site, during link time. Its not a function of hotness, only unique edge discovery.

Excellent! Thanks!

src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointManager.cpp

src/jdk.jfr/share/classes/jdk/jfr/internal/Level.java

src/jdk.jfr/share/classes/jdk/jfr/internal/test/DeprecatedMethods.java

src/hotspot/share/jfr/support/jfrNativeLibraryLoadEvent.cpp

openjdk · 2023-12-06T16:11:13Z

@mgronlun This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8211238: @Deprecated JFR event

Reviewed-by: egahlin, jbachorik

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 81 new commits pushed to the master branch:

656b446: 8320969: Shenandoah: Enforce stable number of GC workers
9a87e52: 8320538: Obsolete CSS styles in collection framework doc-file
03c4595: 8321164: javac with annotation processor throws AssertionError: Filling jrt:/... during JarFileObject[/...]
632a3c5: 8305825: getBounds API returns wrong value resulting in multiple Regression Test Failures on Ubuntu 23.04
75a7c19: 8315827: Kitchensink.java and RenaissanceStressTest.java time out with jvmti module errors
91ffdfb: 8320365: IPPPrintService.getAttributes() causes blanket re-initialisation
50baaf4: 8321013: Parallel: Refactor ObjectStartArray
afb8964: 8320443: [macos] Test java/awt/print/PrinterJob/PrinterDevice.java fails on macOS
82796bd: 8320570: NegativeArraySizeException decoding >1G UTF8 bytes with non-ascii characters
781775d: 8321484: Make TestImplicitlyDeclaredClasses release independent
... and 71 more: https://git.openjdk.org/jdk/compare/5dee2a36756caba28cf12920dc562278576f6de0...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

mgronlun · 2023-12-07T10:44:57Z

Many thanks, @jbachorik and @egahlin, for your reviews - much appreciated!

mgronlun · 2023-12-07T10:45:31Z

/integrate

openjdk · 2023-12-07T10:46:01Z

Going to push as commit 49fff01.
Since your change was applied there have been 81 commits pushed to the master branch:

656b446: 8320969: Shenandoah: Enforce stable number of GC workers
9a87e52: 8320538: Obsolete CSS styles in collection framework doc-file
03c4595: 8321164: javac with annotation processor throws AssertionError: Filling jrt:/... during JarFileObject[/...]
632a3c5: 8305825: getBounds API returns wrong value resulting in multiple Regression Test Failures on Ubuntu 23.04
75a7c19: 8315827: Kitchensink.java and RenaissanceStressTest.java time out with jvmti module errors
91ffdfb: 8320365: IPPPrintService.getAttributes() causes blanket re-initialisation
50baaf4: 8321013: Parallel: Refactor ObjectStartArray
afb8964: 8320443: [macos] Test java/awt/print/PrinterJob/PrinterDevice.java fails on macOS
82796bd: 8320570: NegativeArraySizeException decoding >1G UTF8 bytes with non-ascii characters
781775d: 8321484: Make TestImplicitlyDeclaredClasses release independent
... and 71 more: https://git.openjdk.org/jdk/compare/5dee2a36756caba28cf12920dc562278576f6de0...master

Your commit was automatically rebased without conflicts.

openjdk · 2023-12-07T10:46:07Z

@mgronlun Pushed as commit 49fff01.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

8211238

9b301b2

openjdk bot added build build-dev@openjdk.org hotspot hotspot-dev@openjdk.org labels Dec 1, 2023

mgronlun added 2 commits December 1, 2023 22:20

remove assert

8ed988e

whitespace

9df8cf5

openjdk bot added the hotspot-jfr hotspot-jfr-dev@openjdk.org label Dec 1, 2023

mgronlun added 2 commits December 1, 2023 23:12

emergency dump support

f61ea42

whitespace

4e496d9

openjdk bot added the rfr Pull request is ready for review label Dec 2, 2023

Merge branch 'openjdk:master' into 8211238

cdedbb2

reflection support

2fdd6e7

Merge branch '8211238' of github.com:mgronlun/jdk into 8211238

1d0b8a9

AlanBateman reviewed Dec 3, 2023

View reviewed changes

src/jdk.jfr/share/classes/jdk/jfr/internal/test/DeprecatedThing.java Show resolved Hide resolved

openjdk bot removed the rfr Pull request is ready for review label Dec 3, 2023

mgronlun added 2 commits December 3, 2023 14:26

restructured testcase

f8ef13c

whitespace

2239ce2

openjdk bot added the rfr Pull request is ready for review label Dec 3, 2023

mgronlun added 4 commits December 3, 2023 16:44

tighter lock scopes

a5717e1

tuning

628651b

adjustements

5d76503

minor adjustment

4e78e89

backpatching hook

e5aeb3e

openjdk bot removed the build build-dev@openjdk.org label Dec 4, 2023

JFR conditionals in backpatching code

12d9bca

mgronlun added 5 commits December 4, 2023 20:18

reviewer feedback and fixes

f1c8cd1

fix unloading

91ec681

max limit number of recorded edges

b4766c3

description and format specifier

0943b29

update event description

9f6bc68

jbachorik reviewed Dec 5, 2023

View reviewed changes

src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointManager.cpp Outdated Show resolved Hide resolved

jbachorik reviewed Dec 5, 2023

View reviewed changes

src/jdk.jfr/share/classes/jdk/jfr/internal/Level.java Outdated Show resolved Hide resolved

jbachorik reviewed Dec 5, 2023

View reviewed changes

src/jdk.jfr/share/classes/jdk/jfr/internal/test/DeprecatedMethods.java Show resolved Hide resolved

jbachorik reviewed Dec 5, 2023

View reviewed changes

src/hotspot/share/jfr/support/jfrNativeLibraryLoadEvent.cpp Show resolved Hide resolved

reviewer feedback

db96aeb

egahlin approved these changes Dec 6, 2023

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Dec 6, 2023

jbachorik approved these changes Dec 7, 2023

View reviewed changes

openjdk bot added the integrated Pull request has been integrated label Dec 7, 2023

openjdk bot closed this Dec 7, 2023

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Dec 7, 2023

roberttoyonaga mentioned this pull request Dec 18, 2023

Update JFR whitelisted log lines Karm/mandrel-integration-tests#238

Merged

8211238: @Deprecated JFR event #16931

8211238: @Deprecated JFR event #16931

Uh oh!

Conversation

mgronlun commented Dec 1, 2023 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Webrev

Uh oh!

bridgekeeper bot commented Dec 1, 2023

Uh oh!

openjdk bot commented Dec 1, 2023

Uh oh!

mgronlun commented Dec 1, 2023

Uh oh!

openjdk bot commented Dec 1, 2023

Uh oh!

mlbridge bot commented Dec 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

openjdk bot commented Dec 2, 2023

Uh oh!

Uh oh!

AlanBateman commented Dec 3, 2023

Uh oh!

mgronlun commented Dec 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jbachorik commented Dec 4, 2023

Uh oh!

mgronlun commented Dec 4, 2023

Uh oh!

magicus commented Dec 4, 2023

Uh oh!

openjdk bot commented Dec 4, 2023

Uh oh!

jbachorik commented Dec 4, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

openjdk bot commented Dec 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mgronlun commented Dec 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mgronlun commented Dec 7, 2023

Uh oh!

openjdk bot commented Dec 7, 2023

Uh oh!

openjdk bot commented Dec 7, 2023

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

5 participants

mgronlun commented Dec 1, 2023 •

edited by openjdk bot

Loading

mlbridge bot commented Dec 2, 2023 •

edited

Loading

mgronlun commented Dec 3, 2023 •

edited

Loading

openjdk bot commented Dec 6, 2023 •

edited

Loading

mgronlun commented Dec 7, 2023 •

edited

Loading