-
Notifications
You must be signed in to change notification settings - Fork 6.1k
8361640: JFR: RandomAccessFile::readLine emits events for each character #26210
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
👋 Welcome back egahlin! A progress list of the required criteria for merging this PR into |
❗ This change is not yet ready to be integrated. |
/label add hotspot-jfr |
@egahlin |
The tests for this area are in tier2 (not tier1). |
I think we'll need to see if a test can be added as it's way too easy to refactor this code and re-introduce the issue. |
I'm planning a follow-up PR that will check the top frame. Here's the draft: That test will count the number of events also, so it can detect if the code is refactored. |
I ran tier2. It was fine. |
Could I have a review of the change that prevents RandomAccessFile::readLine from emitting an event per character? This leads to unnecessary overhead, both with or without JFR enabled.
Testing: tier1 + tier2 + jdk/jdk/jfr
Thanks
Erik
Progress
Issue
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/26210/head:pull/26210
$ git checkout pull/26210
Update a local copy of the PR:
$ git checkout pull/26210
$ git pull https://git.openjdk.org/jdk.git pull/26210/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 26210
View PR using the GUI difftool:
$ git pr show -t 26210
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/26210.diff
Using Webrev
Link to Webrev Comment