8316927: JFR: Move shouldCommit check earlier for socket events

egahlin · egahlin · commit 6aa837eee62c · 2023-10-17T13:11:52.000Z
Reviewed-by: alanb, dfuchs, mgronlun
diff --git a/src/java.base/share/classes/java/net/Socket.java b/src/java.base/share/classes/java/net/Socket.java
@@ -1096,7 +1096,10 @@ public int read(byte[] b, int off, int len) throws IOException {
             }
             long start = SocketReadEvent.timestamp();
             int nbytes = implRead(b, off, len);
-            SocketReadEvent.offer(start, nbytes, parent.getRemoteSocketAddress(), getSoTimeout());
+            long duration = SocketReadEvent.timestamp() - start;
+            if (SocketReadEvent.shouldCommit(duration)) {
+                SocketReadEvent.emit(start, duration, nbytes, parent.getRemoteSocketAddress(), getSoTimeout());
+            }
             return nbytes;
         }
 
@@ -1209,7 +1212,10 @@ public void write(byte[] b, int off, int len) throws IOException {
             }
             long start = SocketWriteEvent.timestamp();
             implWrite(b, off, len);
-            SocketWriteEvent.offer(start, len, parent.getRemoteSocketAddress());
+            long duration = SocketWriteEvent.timestamp() - start;
+            if (SocketWriteEvent.shouldCommit(duration)) {
+                SocketWriteEvent.emit(start, duration, len, parent.getRemoteSocketAddress());
+            }
         }
 
         private void implWrite(byte[] b, int off, int len) throws IOException {
diff --git a/src/java.base/share/classes/jdk/internal/event/SocketReadEvent.java b/src/java.base/share/classes/jdk/internal/event/SocketReadEvent.java
@@ -113,21 +113,19 @@ public static long timestamp() {
      * {@link #commit(long, long, String, String, int, long, long, boolean)}.
      *
      * @param start  the start time
+     * @param duration the duration
      * @param nbytes  how many bytes were transferred
      * @param remote  the address of the remote socket
      * @param timeout  maximum time to wait
      */
-    public static void offer(long start, long nbytes, SocketAddress remote, long timeout) {
-        long duration = timestamp() - start;
-        if (shouldCommit(duration)) {
-            boolean eof = nbytes < 0 ? true : false;
-            nbytes = nbytes < 0 ? 0 : nbytes;
-            if (remote instanceof InetSocketAddress isa) {
-                commit(start, duration, isa.getHostString(), isa.getAddress().getHostAddress(), isa.getPort(), timeout, nbytes, eof);
-            } else if (remote instanceof UnixDomainSocketAddress udsa) {
-                String path = "[" + udsa.getPath().toString() + "]";
-                commit(start, duration, "Unix domain socket", path, 0, timeout, nbytes, eof);
-            }
+    public static void emit(long start, long duration, long nbytes, SocketAddress remote, long timeout) {
+        boolean eof = nbytes < 0 ? true : false;
+        nbytes = nbytes < 0 ? 0 : nbytes;
+        if (remote instanceof InetSocketAddress isa) {
+            commit(start, duration, isa.getHostString(), isa.getAddress().getHostAddress(), isa.getPort(), timeout, nbytes, eof);
+        } else if (remote instanceof UnixDomainSocketAddress udsa) {
+            String path = "[" + udsa.getPath().toString() + "]";
+            commit(start, duration, "Unix domain socket", path, 0, timeout, nbytes, eof);
         }
     }
 
diff --git a/src/java.base/share/classes/jdk/internal/event/SocketWriteEvent.java b/src/java.base/share/classes/jdk/internal/event/SocketWriteEvent.java
@@ -108,19 +108,17 @@ public static long timestamp() {
      * {@link #commit(long, long, String, String, int, long)}.
      *
      * @param start  the start time
+     * @param duration the duration
      * @param bytesWritten  how many bytes were sent
      * @param remote  the address of the remote socket being written to
      */
-    public static void offer(long start, long bytesWritten, SocketAddress remote) {
-        long duration = timestamp() - start;
-        if (shouldCommit(duration)) {
-            long bytes = bytesWritten < 0 ? 0 : bytesWritten;
-            if (remote instanceof InetSocketAddress isa) {
-                commit(start, duration, isa.getHostString(), isa.getAddress().getHostAddress(), isa.getPort(), bytes);
-            } else if (remote instanceof UnixDomainSocketAddress udsa) {
-                String path = "[" + udsa.getPath().toString() + "]";
-                commit(start, duration, "Unix domain socket", path, 0, bytes);
-            }
+    public static void emit(long start, long duration, long bytesWritten, SocketAddress remote) {
+        long bytes = bytesWritten < 0 ? 0 : bytesWritten;
+        if (remote instanceof InetSocketAddress isa) {
+            commit(start, duration, isa.getHostString(), isa.getAddress().getHostAddress(), isa.getPort(), bytes);
+        } else if (remote instanceof UnixDomainSocketAddress udsa) {
+            String path = "[" + udsa.getPath().toString() + "]";
+            commit(start, duration, "Unix domain socket", path, 0, bytes);
         }
     }
 }
diff --git a/src/java.base/share/classes/sun/nio/ch/SocketChannelImpl.java b/src/java.base/share/classes/sun/nio/ch/SocketChannelImpl.java
@@ -494,7 +494,10 @@ public int read(ByteBuffer buf) throws IOException {
         }
         long start = SocketReadEvent.timestamp();
         int nbytes = implRead(buf);
-        SocketReadEvent.offer(start, nbytes, remoteAddress(), 0);
+        long duration = SocketReadEvent.timestamp() - start;
+        if (SocketReadEvent.shouldCommit(duration)) {
+            SocketReadEvent.emit(start, duration, nbytes, remoteAddress(), 0);
+        }
         return nbytes;
     }
 
@@ -508,7 +511,10 @@ public long read(ByteBuffer[] dsts, int offset, int length)
         }
         long start = SocketReadEvent.timestamp();
         long nbytes = implRead(dsts, offset, length);
-        SocketReadEvent.offer(start, nbytes, remoteAddress(), 0);
+        long duration = SocketReadEvent.timestamp() - start;
+        if (SocketReadEvent.shouldCommit(duration)) {
+            SocketReadEvent.emit(start, duration, nbytes, remoteAddress(), 0);
+        }
         return nbytes;
     }
 
@@ -619,7 +625,10 @@ public int write(ByteBuffer buf) throws IOException {
         }
         long start = SocketWriteEvent.timestamp();
         int nbytes = implWrite(buf);
-        SocketWriteEvent.offer(start, nbytes, remoteAddress());
+        long duration = SocketWriteEvent.timestamp() - start;
+        if (SocketWriteEvent.shouldCommit(duration)) {
+            SocketWriteEvent.emit(start, duration, nbytes, remoteAddress());
+        }
         return nbytes;
     }
 
@@ -632,7 +641,10 @@ public long write(ByteBuffer[] srcs, int offset, int length)
         }
         long start = SocketWriteEvent.timestamp();
         long nbytes = implWrite(srcs, offset, length);
-        SocketWriteEvent.offer(start, nbytes, remoteAddress());
+        long duration = SocketWriteEvent.timestamp() - start;
+        if (SocketWriteEvent.shouldCommit(duration)) {
+            SocketWriteEvent.emit(start, duration, nbytes, remoteAddress());
+        }
         return nbytes;
     }
 
diff --git a/test/micro/org/openjdk/bench/java/net/SocketEventOverhead.java b/test/micro/org/openjdk/bench/java/net/SocketEventOverhead.java
@@ -137,7 +137,10 @@ public int write() {
             try {
                 nbytes = write0();
             } finally {
-                SocketWriteEvent.offer(start, nbytes, getRemoteAddress());
+                long duration = start - SocketWriteEvent.timestamp();
+                if (SocketWriteEvent.shouldCommit(duration)) {
+                    SocketWriteEvent.emit(start, duration, nbytes, getRemoteAddress());
+                }
             }
             return nbytes;
         }
@@ -155,7 +158,10 @@ public int read() {
             try {
                 nbytes = read0();
             } finally {
-                SocketReadEvent.offer(start, nbytes, getRemoteAddress(), 0);
+                long duration = start - SocketReadEvent.timestamp();
+                if (SocketReadEvent.shouldCommit(duration)) {
+                    SocketReadEvent.emit(start, duration, nbytes, getRemoteAddress(), 0);
+                }
             }
             return nbytes;
         }

Original file line number	Diff line number	Diff line change
`@@ -1096,7 +1096,10 @@ public int read(byte[] b, int off, int len) throws IOException {`
`1096`	`1096`	`}`
`1097`	`1097`	`long start = SocketReadEvent.timestamp();`
`1098`	`1098`	`int nbytes = implRead(b, off, len);`
`1099`		`- SocketReadEvent.offer(start, nbytes, parent.getRemoteSocketAddress(), getSoTimeout());`
	`1099`	`+ long duration = SocketReadEvent.timestamp() - start;`
	`1100`	`+ if (SocketReadEvent.shouldCommit(duration)) {`
	`1101`	`+ SocketReadEvent.emit(start, duration, nbytes, parent.getRemoteSocketAddress(), getSoTimeout());`
	`1102`	`+ }`
`1100`	`1103`	`return nbytes;`
`1101`	`1104`	`}`
`1102`	`1105`
`@@ -1209,7 +1212,10 @@ public void write(byte[] b, int off, int len) throws IOException {`
`1209`	`1212`	`}`
`1210`	`1213`	`long start = SocketWriteEvent.timestamp();`
`1211`	`1214`	`implWrite(b, off, len);`
`1212`		`- SocketWriteEvent.offer(start, len, parent.getRemoteSocketAddress());`
	`1215`	`+ long duration = SocketWriteEvent.timestamp() - start;`
	`1216`	`+ if (SocketWriteEvent.shouldCommit(duration)) {`
	`1217`	`+ SocketWriteEvent.emit(start, duration, len, parent.getRemoteSocketAddress());`
	`1218`	`+ }`
`1213`	`1219`	`}`
`1214`	`1220`
`1215`	`1221`	`private void implWrite(byte[] b, int off, int len) throws IOException {`
Original file line number	Diff line number	Diff line change
`@@ -494,7 +494,10 @@ public int read(ByteBuffer buf) throws IOException {`
`494`	`494`	`}`
`495`	`495`	`long start = SocketReadEvent.timestamp();`
`496`	`496`	`int nbytes = implRead(buf);`
`497`		`- SocketReadEvent.offer(start, nbytes, remoteAddress(), 0);`
	`497`	`+ long duration = SocketReadEvent.timestamp() - start;`
	`498`	`+ if (SocketReadEvent.shouldCommit(duration)) {`
	`499`	`+ SocketReadEvent.emit(start, duration, nbytes, remoteAddress(), 0);`
	`500`	`+ }`
`498`	`501`	`return nbytes;`
`499`	`502`	`}`
`500`	`503`
`@@ -508,7 +511,10 @@ public long read(ByteBuffer[] dsts, int offset, int length)`
`508`	`511`	`}`
`509`	`512`	`long start = SocketReadEvent.timestamp();`
`510`	`513`	`long nbytes = implRead(dsts, offset, length);`
`511`		`- SocketReadEvent.offer(start, nbytes, remoteAddress(), 0);`
	`514`	`+ long duration = SocketReadEvent.timestamp() - start;`
	`515`	`+ if (SocketReadEvent.shouldCommit(duration)) {`
	`516`	`+ SocketReadEvent.emit(start, duration, nbytes, remoteAddress(), 0);`
	`517`	`+ }`
`512`	`518`	`return nbytes;`
`513`	`519`	`}`
`514`	`520`
`@@ -619,7 +625,10 @@ public int write(ByteBuffer buf) throws IOException {`
`619`	`625`	`}`
`620`	`626`	`long start = SocketWriteEvent.timestamp();`
`621`	`627`	`int nbytes = implWrite(buf);`
`622`		`- SocketWriteEvent.offer(start, nbytes, remoteAddress());`
	`628`	`+ long duration = SocketWriteEvent.timestamp() - start;`
	`629`	`+ if (SocketWriteEvent.shouldCommit(duration)) {`
	`630`	`+ SocketWriteEvent.emit(start, duration, nbytes, remoteAddress());`
	`631`	`+ }`
`623`	`632`	`return nbytes;`
`624`	`633`	`}`
`625`	`634`
`@@ -632,7 +641,10 @@ public long write(ByteBuffer[] srcs, int offset, int length)`
`632`	`641`	`}`
`633`	`642`	`long start = SocketWriteEvent.timestamp();`
`634`	`643`	`long nbytes = implWrite(srcs, offset, length);`
`635`		`- SocketWriteEvent.offer(start, nbytes, remoteAddress());`
	`644`	`+ long duration = SocketWriteEvent.timestamp() - start;`
	`645`	`+ if (SocketWriteEvent.shouldCommit(duration)) {`
	`646`	`+ SocketWriteEvent.emit(start, duration, nbytes, remoteAddress());`
	`647`	`+ }`
`636`	`648`	`return nbytes;`
`637`	`649`	`}`
`638`	`650`
Original file line number	Diff line number	Diff line change
`@@ -137,7 +137,10 @@ public int write() {`
`137`	`137`	`try {`
`138`	`138`	`nbytes = write0();`
`139`	`139`	`} finally {`
`140`		`- SocketWriteEvent.offer(start, nbytes, getRemoteAddress());`
	`140`	`+ long duration = start - SocketWriteEvent.timestamp();`
	`141`	`+ if (SocketWriteEvent.shouldCommit(duration)) {`
	`142`	`+ SocketWriteEvent.emit(start, duration, nbytes, getRemoteAddress());`
	`143`	`+ }`
`141`	`144`	`}`
`142`	`145`	`return nbytes;`
`143`	`146`	`}`
`@@ -155,7 +158,10 @@ public int read() {`
`155`	`158`	`try {`
`156`	`159`	`nbytes = read0();`
`157`	`160`	`} finally {`
`158`		`- SocketReadEvent.offer(start, nbytes, getRemoteAddress(), 0);`
	`161`	`+ long duration = start - SocketReadEvent.timestamp();`
	`162`	`+ if (SocketReadEvent.shouldCommit(duration)) {`
	`163`	`+ SocketReadEvent.emit(start, duration, nbytes, getRemoteAddress(), 0);`
	`164`	`+ }`
`159`	`165`	`}`
`160`	`166`	`return nbytes;`
`161`	`167`	`}`