RFR: 8295350: JFR: Add stop methods for recording streams
Could I have a review of PR that adds stop() methods to RecordingStream and RemoteRecordingStream. Purpose is to provide better control of what is being recorded. Testing: test/jdk/jdk/jfr Thanks Erik ------------- Commit messages: - Set stream end for parser barrier - Initial Changes: https://git.openjdk.org/jdk/pull/11337/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=11337&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8295350 Stats: 641 lines in 12 files changed: 636 ins; 2 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/11337.diff Fetch: git fetch https://git.openjdk.org/jdk pull/11337/head:pull/11337 PR: https://git.openjdk.org/jdk/pull/11337
Could I have a review of PR that adds stop() methods to RecordingStream and RemoteRecordingStream. Purpose is to provide better control of what is being recorded.
Testing: test/jdk/jdk/jfr
Thanks Erik
Erik Gahlin has updated the pull request incrementally with one additional commit since the last revision: Update comments ------------- Changes: - all: https://git.openjdk.org/jdk/pull/11337/files - new: https://git.openjdk.org/jdk/pull/11337/files/271f1b45..8baf0641 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=11337&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=11337&range=00-01 Stats: 3 lines in 3 files changed: 1 ins; 1 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/11337.diff Fetch: git fetch https://git.openjdk.org/jdk pull/11337/head:pull/11337 PR: https://git.openjdk.org/jdk/pull/11337
On Thu, 24 Nov 2022 07:59:43 GMT, Erik Gahlin <egahlin@openjdk.org> wrote:
Could I have a review of PR that adds stop() methods to RecordingStream and RemoteRecordingStream. Purpose is to provide better control of what is being recorded.
Testing: test/jdk/jdk/jfr
Thanks Erik
Erik Gahlin has updated the pull request incrementally with one additional commit since the last revision:
Update comments
src/jdk.jfr/share/classes/jdk/jfr/consumer/RecordingStream.java line 404:
402: * @throws IllegalStateException if the recording is not started or is already stopped 403: */ 404: public boolean stop() {
Drive-by comment, I assume you should add `@since 20` here. ------------- PR: https://git.openjdk.org/jdk/pull/11337
Could I have a review of PR that adds stop() methods to RecordingStream and RemoteRecordingStream. Purpose is to provide better control of what is being recorded.
Testing: test/jdk/jdk/jfr
Thanks Erik
Erik Gahlin has updated the pull request incrementally with one additional commit since the last revision: Add @since ------------- Changes: - all: https://git.openjdk.org/jdk/pull/11337/files - new: https://git.openjdk.org/jdk/pull/11337/files/8baf0641..eba58c7d Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=11337&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=11337&range=01-02 Stats: 4 lines in 2 files changed: 4 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/11337.diff Fetch: git fetch https://git.openjdk.org/jdk pull/11337/head:pull/11337 PR: https://git.openjdk.org/jdk/pull/11337
On Fri, 25 Nov 2022 23:44:38 GMT, Erik Gahlin <egahlin@openjdk.org> wrote:
Could I have a review of PR that adds stop() methods to RecordingStream and RemoteRecordingStream. Purpose is to provide better control of what is being recorded.
Testing: test/jdk/jdk/jfr
Thanks Erik
Erik Gahlin has updated the pull request incrementally with one additional commit since the last revision:
Add @since
src/jdk.management.jfr/share/classes/jdk/management/jfr/RemoteRecordingStream.java line 615:
613: stopped = mbean.stopRecording(recordingId); 614: ManagementSupport.setCloseOnComplete(stream, false); 615: long stopTime = getRecordingInfo(mbean.getRecordings(), recordingId).getStopTime(); pb.setStreamEnd(stopTime);
Why `pb.setStreamEnd(stopTime);` is placed at the end of line here? ------------- PR: https://git.openjdk.org/jdk/pull/11337
On Sat, 26 Nov 2022 12:52:14 GMT, Andrey Turbanov <aturbanov@openjdk.org> wrote:
Erik Gahlin has updated the pull request incrementally with one additional commit since the last revision:
Add @since
src/jdk.management.jfr/share/classes/jdk/management/jfr/RemoteRecordingStream.java line 615:
613: stopped = mbean.stopRecording(recordingId); 614: ManagementSupport.setCloseOnComplete(stream, false); 615: long stopTime = getRecordingInfo(mbean.getRecordings(), recordingId).getStopTime(); pb.setStreamEnd(stopTime);
Why `pb.setStreamEnd(stopTime);` is placed at the end of line here?
I wondered why "pb.setStreamEnd(stopTime);" disappeared. I will remove it. ------------- PR: https://git.openjdk.org/jdk/pull/11337
Could I have a review of PR that adds stop() methods to RecordingStream and RemoteRecordingStream. Purpose is to provide better control of what is being recorded.
Testing: test/jdk/jdk/jfr
Thanks Erik
Erik Gahlin has updated the pull request incrementally with one additional commit since the last revision: Remove duplicated line ------------- Changes: - all: https://git.openjdk.org/jdk/pull/11337/files - new: https://git.openjdk.org/jdk/pull/11337/files/eba58c7d..5424afe3 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=11337&range=03 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=11337&range=02-03 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/11337.diff Fetch: git fetch https://git.openjdk.org/jdk pull/11337/head:pull/11337 PR: https://git.openjdk.org/jdk/pull/11337
On Sat, 26 Nov 2022 13:17:43 GMT, Erik Gahlin <egahlin@openjdk.org> wrote:
Could I have a review of PR that adds stop() methods to RecordingStream and RemoteRecordingStream. Purpose is to provide better control of what is being recorded.
Testing: test/jdk/jdk/jfr
Thanks Erik
Erik Gahlin has updated the pull request incrementally with one additional commit since the last revision:
Remove duplicated line
Marked as reviewed by mgronlun (Reviewer). ------------- PR: https://git.openjdk.org/jdk/pull/11337
On Wed, 23 Nov 2022 23:33:01 GMT, Erik Gahlin <egahlin@openjdk.org> wrote:
Could I have a review of PR that adds stop() methods to RecordingStream and RemoteRecordingStream. Purpose is to provide better control of what is being recorded.
Testing: test/jdk/jdk/jfr
Thanks Erik
This pull request has now been integrated. Changeset: eec24aa2 Author: Erik Gahlin <egahlin@openjdk.org> URL: https://git.openjdk.org/jdk/commit/eec24aa2039658afd6d2fde790174d982eae6479 Stats: 645 lines in 12 files changed: 640 ins; 2 del; 3 mod 8295350: JFR: Add stop methods for recording streams Reviewed-by: mgronlun ------------- PR: https://git.openjdk.org/jdk/pull/11337
participants (4)
-
Alan Bateman
-
Andrey Turbanov
-
Erik Gahlin
-
Markus Grönlund