RFR: 8331876: JFR: Move file read and write events to java.base

2024-05-07 Thread Erik Gahlin
Hi, Could I have a review of a change that moves the jdk.FileRead and jdk.FileWrite events to java.base to remove the use of the ASM instrumentation. Testing: jdk/jdk/jfr Thanks Erik - Commit messages: - Update comment - Initial Changes: https://git.openjdk.org/jdk/pull/19129/f

Re: RFR: JDK-8327474 Review use of java.io.tmpdir in jdk tests [v2]

2024-03-20 Thread Erik Gahlin
On Tue, 19 Mar 2024 17:58:46 GMT, Bill Huang wrote: >> This task addresses an essential aspect of our testing infrastructure: the >> proper handling and cleanup of temporary files and socket files created >> during test execution. The motivation behind these changes is to prevent the >> accumu

Re: RFR: 8275338: Add JFR events for notable serialization situations [v15]

2024-01-15 Thread Erik Gahlin
On Mon, 15 Jan 2024 14:17:40 GMT, Raffaello Giulietti wrote: >> Adds serialization misdeclaration events to JFR. > > Raffaello Giulietti has updated the pull request incrementally with one > additional commit since the last revision: > > Removed useless event settings in test. Marked as rev

Re: RFR: 8275338: Add JFR events for notable serialization situations [v5]

2024-01-15 Thread Erik Gahlin
On Tue, 19 Dec 2023 17:53:49 GMT, Erik Gahlin wrote: >> Raffaello Giulietti has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Changes according to reviewer's comments. > >> You mean, in the @descrip

Re: RFR: 8275338: Add JFR events for notable serialization situations [v9]

2023-12-21 Thread Erik Gahlin
On Thu, 21 Dec 2023 09:36:06 GMT, Raffaello Giulietti wrote: >> Adds serialization misdeclaration events to JFR. > > Raffaello Giulietti has updated the pull request incrementally with one > additional commit since the last revision: > > Removed @module from test. src/jdk.jfr/share/classes/

Re: RFR: 8275338: Add JFR events for notable serialization situations [v5]

2023-12-19 Thread Erik Gahlin
On Tue, 19 Dec 2023 17:37:50 GMT, Raffaello Giulietti wrote: >> src/java.base/share/classes/java/io/SerializationMisdeclarationChecker.java >> line 39: >> >>> 37: import static java.lang.reflect.Modifier.*; >>> 38: >>> 39: final class SerializationMisdeclarationChecker { >> >> Is there a rea

Re: RFR: 8275338: Add JFR events for notable serialization situations [v5]

2023-12-19 Thread Erik Gahlin
On Tue, 19 Dec 2023 16:45:04 GMT, Raffaello Giulietti wrote: >> Adds serialization misdeclaration events to JFR. > > Raffaello Giulietti has updated the pull request incrementally with one > additional commit since the last revision: > > Changes according to reviewer's comments. > You mean,

Re: RFR: 8275338: Add JFR events for notable serialization situations [v4]

2023-12-19 Thread Erik Gahlin
On Tue, 19 Dec 2023 16:28:03 GMT, Raffaello Giulietti wrote: > However, the cache can be emptied under high memory pressure, so the > `ObjectStreamClass` instance might be recreated later, thus re-invoking the > serialization checker once again. I think it would be good to state in the descri

Re: RFR: 8275338: Add JFR events for notable serialization situations [v4]

2023-12-19 Thread Erik Gahlin
On Tue, 19 Dec 2023 16:00:59 GMT, Raffaello Giulietti wrote: >> test/jdk/jdk/jfr/event/io/TestSerializationMisdeclarationEvent.java line 50: >> >>> 48: * @requires vm.hasJFR >>> 49: * @library /test/lib >>> 50: * @run junit/othervm >>> jdk.jfr.event.io.TestSerializationMisdeclarationEvent >

Re: RFR: 8275338: Add JFR events for notable serialization situations [v5]

2023-12-19 Thread Erik Gahlin
On Tue, 19 Dec 2023 16:45:04 GMT, Raffaello Giulietti wrote: >> Adds serialization misdeclaration events to JFR. > > Raffaello Giulietti has updated the pull request incrementally with one > additional commit since the last revision: > > Changes according to reviewer's comments. src/java.ba

Re: RFR: 8275338: Add JFR events for notable serialization situations [v3]

2023-12-19 Thread Erik Gahlin
On Tue, 19 Dec 2023 12:17:38 GMT, Raffaello Giulietti wrote: >> src/jdk.jfr/share/classes/jdk/jfr/events/SerializationMisdeclarationEvent.java >> line 48: >> >>> 46: >>> 47: @Label("Kind") >>> 48: public int kind; >> >> What is the use case for error codes? Are they public or an impl

Re: RFR: 8275338: Add JFR events for notable serialization situations [v3]

2023-12-19 Thread Erik Gahlin
On Mon, 18 Dec 2023 17:49:04 GMT, Raffaello Giulietti wrote: >> Adds serialization misdeclaration events to JFR. > > Raffaello Giulietti has updated the pull request incrementally with one > additional commit since the last revision: > > Event enabled on profile.jfc but disabled on default.j

Re: RFR: 8275338: Add JFR events for notable serialization situations [v2]

2023-12-18 Thread Erik Gahlin
On Sat, 16 Dec 2023 01:27:17 GMT, Erik Gahlin wrote: >> Raffaello Giulietti has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Restrict accessibility of nested classes. > > It seems correct to have the event d

Re: RFR: 8275338: Add JFR events for notable serialization situations [v2]

2023-12-15 Thread Erik Gahlin
On Fri, 15 Dec 2023 22:26:04 GMT, Raffaello Giulietti wrote: >> Adds serialization misdeclaration events to JFR. > > Raffaello Giulietti has updated the pull request incrementally with one > additional commit since the last revision: > > Restrict accessibility of nested classes. It seems co

Re: RFR: 8319374: JFR: Remove instrumentation for exception events [v3]

2023-11-08 Thread Erik Gahlin
On Wed, 8 Nov 2023 13:39:10 GMT, Daniel Fuchs wrote: >> I agree, and I have looked into it, but I think it's better to do that >> refactorization separately as it will impact other events. > > Just for my own understanding: in this particular case the time stamp is > meaningless because the dur

Integrated: 8319374: JFR: Remove instrumentation for exception events

2023-11-08 Thread Erik Gahlin
On Fri, 3 Nov 2023 12:19:07 GMT, Erik Gahlin wrote: > Could I have a review of a PR that removes the bytecode instrumentation for > the exception events. > > Testing: jdk/jdk/jfr + tier1 + tier2 This pull request has now been integrated. Changeset: e8418972 Author: Erik

Re: RFR: 8319374: JFR: Remove instrumentation for exception events [v3]

2023-11-07 Thread Erik Gahlin
On Wed, 8 Nov 2023 05:13:04 GMT, David Holmes wrote: >> Erik Gahlin has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Rename field from tracing to jfrTracing > > src/java.base/share/classes/jdk/internal/even

Re: RFR: 8319374: JFR: Remove instrumentation for exception events [v3]

2023-11-07 Thread Erik Gahlin
> Could I have a review of a PR that removes the bytecode instrumentation for > the exception events. > > Testing: jdk/jdk/jfr + tier1 + tier2 Erik Gahlin has updated the pull request incrementally with one additional commit since the last revision: Rename field from tracing t

Re: RFR: 8319374: JFR: Remove instrumentation for exception events [v2]

2023-11-07 Thread Erik Gahlin
On Tue, 7 Nov 2023 09:27:51 GMT, Alan Bateman wrote: >> I filed an issue to investigate if there is a problem with SOE, or if the >> OOM check is really needed now. >> https://bugs.openjdk.org/browse/JDK-8319579 >> >> Regardless of outcome, It would be good to document the results of the >> in

Re: RFR: 8319374: JFR: Remove instrumentation for exception events [v2]

2023-11-07 Thread Erik Gahlin
On Tue, 7 Nov 2023 09:24:20 GMT, Alan Bateman wrote: >> Erik Gahlin has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Remove SecurityException and IllegalArgumentException from throws clause > > src/jav

Re: RFR: 8319374: JFR: Remove instrumentation for exception events [v2]

2023-11-06 Thread Erik Gahlin
On Mon, 6 Nov 2023 22:41:17 GMT, Erik Gahlin wrote: >> src/java.base/share/classes/jdk/internal/event/ThrowableTracer.java line 44: >> >>> 42: >>> 43: public static void traceError(Class clazz, String message) { >>> 44: if (OutOfM

Re: RFR: 8319374: JFR: Remove instrumentation for exception events [v2]

2023-11-06 Thread Erik Gahlin
> Could I have a review of a PR that removes the bytecode instrumentation for > the exception events. > > Testing: jdk/jdk/jfr + tier1 + tier2 Erik Gahlin has updated the pull request incrementally with one additional commit since the last revision: Remove SecurityE

Re: RFR: 8319374: JFR: Remove instrumentation for exception events

2023-11-06 Thread Erik Gahlin
On Mon, 6 Nov 2023 15:49:02 GMT, Alan Bateman wrote: >> Could I have a review of a PR that removes the bytecode instrumentation for >> the exception events. >> >> Testing: jdk/jdk/jfr + tier1 + tier2 > > src/java.base/share/classes/jdk/internal/event/ThrowableTracer.java line 37: > >> 35:

RFR: 8319374: JFR: Remove instrumentation for exception events

2023-11-06 Thread Erik Gahlin
Could I have a review of a PR that removes the bytecode instrumentation for the exception events. Testing: jdk/jdk/jfr + tier1 + tier2 - Commit messages: - Remove Throwable and Error from instrumentation list - Initial Changes: https://git.openjdk.org/jdk/pull/16493/files Webrev

Re: RFR: 8308995: Update Network IO JFR events to be static mirror events [v6]

2023-09-22 Thread Erik Gahlin
On Tue, 19 Sep 2023 15:35:11 GMT, Tim Prinzing wrote: >> The socket read/write JFR events currently use instrumentation of java.base >> code using templates in the jdk.jfr modules. This results in some java.base >> code residing in the jdk.jfr module which is undesirable. >> >> JDK19 added sta

Re: RFR: 8308995: Update Network IO JFR events to be static mirror events [v4]

2023-08-22 Thread Erik Gahlin
On Wed, 28 Jun 2023 18:53:12 GMT, Tim Prinzing wrote: >> The socket read/write JFR events currently use instrumentation of java.base >> code using templates in the jdk.jfr modules. This results in some java.base >> code residing in the jdk.jfr module which is undesirable. >> >> JDK19 added sta

Re: RFR: 8308995: Update Network IO JFR events to be static mirror events [v4]

2023-08-02 Thread Erik Gahlin
On Wed, 28 Jun 2023 18:53:12 GMT, Tim Prinzing wrote: >> The socket read/write JFR events currently use instrumentation of java.base >> code using templates in the jdk.jfr modules. This results in some java.base >> code residing in the jdk.jfr module which is undesirable. >> >> JDK19 added sta

Re: [jdk21] RFR: 8300937: Update nroff pages in JDK 21 before RC

2023-07-31 Thread Erik Gahlin
On Mon, 31 Jul 2023 08:33:07 GMT, David Holmes wrote: > Main changes are to use 21 instead of 21-ea. In addition the following files > contain additional updates from the closed sources: > > - src/java.base/share/man/java.1 > > [JDK-8273131](https://bugs.openjdk.org/browse/JDK-8273131): Updat

Re: [External] : Re: PrivilegedAction et al and JEP411

2023-06-22 Thread Erik Gahlin
> I'd suggest cost of maintenance also appears overestimated It isn’t. We have nothing to gain from removing a feature whose benefits outweigh its cost The cost is very high. I've spent probably a year on the SM just for jdk.jfr module. Erik From: security-dev

Re: RFR: 8308995: Update Network IO JFR events to be static mirror events

2023-06-22 Thread Erik Gahlin
On Thu, 22 Jun 2023 11:53:59 GMT, Daniel Fuchs wrote: > The new code seems to accurately correspond to what the various > `*Instrumentor` classes were doing, so that is good. I agree with Alan that > potential exception that may arise when generating the event are an issue > (e.g. call to getR

Re: RFR: 8308995: Update Network IO JFR events to be static mirror events

2023-06-22 Thread Erik Gahlin
On Tue, 6 Jun 2023 19:39:31 GMT, Tim Prinzing wrote: > The socket read/write JFR events currently use instrumentation of java.base > code using templates in the jdk.jfr modules. This results in some java.base > code residing in the jdk.jfr module which is undesirable. > > JDK19 added static su

Withdrawn: 8296229: JFR: jfr tool should print unsigned values correctly

2022-11-10 Thread Erik Gahlin
On Tue, 8 Nov 2022 12:03:06 GMT, Erik Gahlin wrote: > Could I have a review of PR that fixes so unsigned numbers are printed > correctly in the jfr tool. > > Testing: > test/jdk/jdk/jfr > test/jdk/jdk/security/logging/ > > Thanks > Erik This pull request ha

Re: RFR: 8296229: JFR: jfr tool should print unsigned values correctly [v2]

2022-11-10 Thread Erik Gahlin
> Could I have a review of PR that fixes so unsigned numbers are printed > correctly in the jfr tool. > > Testing: > test/jdk/jdk/jfr > test/jdk/jdk/security/logging/ > > Thanks > Erik Erik Gahlin has updated the pull request with a new target base due to a merge or

RFR: 8296229: JFR: jfr tool should print unsigned values correctly

2022-11-08 Thread Erik Gahlin
Could I have a review of PR that fixes so unsigned numbers are printed correctly in the jfr tool. Testing: test/jdk/jdk/jfr test/jdk/jdk/security/logging/ Thanks Erik - Commit messages: - Initial Changes: https://git.openjdk.org/jdk/pull/11039/files Webrev: https://webrevs.openj

Re: RFR: 8292177: InitialSecurityProperty JFR event [v3]

2022-10-10 Thread Erik Gahlin
On Mon, 10 Oct 2022 08:04:28 GMT, Jaikiran Pai wrote: >> Sean Coffey has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Check for 0 security events > > src/jdk.jfr/share/classes/jdk/jfr/events/InitialSecurityPropertyEvent.java > line 33: >

Re: RFR: 8292177: InitialSecurityProperty JFR event [v3]

2022-10-10 Thread Erik Gahlin
On Mon, 10 Oct 2022 08:06:45 GMT, Jaikiran Pai wrote: >> Sean Coffey has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Check for 0 security events > > src/jdk.jfr/share/classes/jdk/jfr/events/InitialSecurityPropertyEvent.java > line 35: >

Re: RFR: 8292177: InitialSecurityProperty JFR event [v2]

2022-09-29 Thread Erik Gahlin
On Thu, 29 Sep 2022 10:02:05 GMT, Sean Coffey wrote: >> How does it capture the event if JFR was started before the security >> properties were read? I would think you still need some additional code in >> Security.java to record the properties if the event is enabled. > > As per yesterday's st

Re: RFR: 8254711: Add java.security.Provider.getService JFR Event

2022-09-19 Thread Erik Gahlin
On Mon, 19 Sep 2022 15:46:46 GMT, Sean Coffey wrote: > This new event is disabled by default just like the other crypto related > events that were added some time back (e.g. `TLSHandshakeEvent`). My > assumption is that these events will be enabled for audit mode when one is > interested in fi

Re: RFR: 8254711: Add java.security.Provider.getService JFR Event

2022-09-19 Thread Erik Gahlin
On Wed, 27 Jul 2022 13:14:39 GMT, Sean Coffey wrote: > Add a JFR Event for `java.security.Provider.getService(String type, String > algorithm)` calls. I noticed that the event is disabled by default. Is it because of concerns of too many events, or too much overhead? Or something else? I thi