From 1b55f1f620dcd292dcde7ed6c0c8460bd0b8b6c1 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Wed, 25 Oct 2023 10:28:00 -0500 Subject: [PATCH 1/3] Re-read from start if a read from proc doesn't fill the buffer --- .../java/io/deephaven/stats/StatsCPUCollector.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/Stats/src/main/java/io/deephaven/stats/StatsCPUCollector.java b/Stats/src/main/java/io/deephaven/stats/StatsCPUCollector.java index 7af2a416434..b29683b8552 100644 --- a/Stats/src/main/java/io/deephaven/stats/StatsCPUCollector.java +++ b/Stats/src/main/java/io/deephaven/stats/StatsCPUCollector.java @@ -226,6 +226,9 @@ private void readToBuffer(FileChannel fileChannel, String fileName) throws IOExc int nb = 0; fileChannel.position(0); + // Filesystem entries in /proc can only be read all at once to avoid races, so using too big of a buffer isn't a + // problem, but too small is. Attempt to read with the current buffer. If we filled the buffer we resize it and + // read again from start. while (true) { final int thisNb = fileChannel.read(statBuffer); @@ -234,10 +237,9 @@ private void readToBuffer(FileChannel fileChannel, String fileName) throws IOExc } nb += thisNb; if (!statBuffer.hasRemaining()) { - // allocate larger read-buffer, and continue reading - ByteBuffer resized = ByteBuffer.allocate(statBuffer.capacity() * 2); - resized.put(statBuffer.flip()); - statBuffer = resized; + // allocate larger read-buffer, and read again from start + statBuffer = ByteBuffer.allocate(statBuffer.capacity() * 2); + fileChannel.position(0); } } From a5ebbd40a4b45dac5af641f440e0e00d332a3918 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Wed, 25 Oct 2023 15:21:51 -0500 Subject: [PATCH 2/3] review feedback --- .../io/deephaven/stats/StatsCPUCollector.java | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/Stats/src/main/java/io/deephaven/stats/StatsCPUCollector.java b/Stats/src/main/java/io/deephaven/stats/StatsCPUCollector.java index b29683b8552..973e1ce2f3b 100644 --- a/Stats/src/main/java/io/deephaven/stats/StatsCPUCollector.java +++ b/Stats/src/main/java/io/deephaven/stats/StatsCPUCollector.java @@ -3,6 +3,7 @@ */ package io.deephaven.stats; +import io.deephaven.base.verify.Assert; import io.deephaven.configuration.Configuration; import io.deephaven.internal.log.LoggerFactory; import io.deephaven.io.logger.Logger; @@ -223,32 +224,31 @@ private boolean peekNextLong() { */ private void readToBuffer(FileChannel fileChannel, String fileName) throws IOException { statBuffer.clear(); - int nb = 0; fileChannel.position(0); // Filesystem entries in /proc can only be read all at once to avoid races, so using too big of a buffer isn't a // problem, but too small is. Attempt to read with the current buffer. If we filled the buffer we resize it and // read again from start. while (true) { - final int thisNb = fileChannel.read(statBuffer); + final int nb = fileChannel.read(statBuffer); - if (thisNb == -1) { - break; + if (nb == -1) { + // EOF means success, set position to zero and limit to the data read + statBuffer.flip(); + return; + } + if (nb == 0) { + // zero bytes read is an error, proc isn't working correctly? + throw new IOException(fileName + " zero read"); } - nb += thisNb; if (!statBuffer.hasRemaining()) { // allocate larger read-buffer, and read again from start statBuffer = ByteBuffer.allocate(statBuffer.capacity() * 2); fileChannel.position(0); + } else { + Assert.eq(statBuffer.position(), "statBuffer.position()", nb, "nb"); } } - - if (nb == 0) { - throw new IOException(fileName + " zero read"); - } else { - // Success, set position and limit to the data read - statBuffer.flip(); - } } /** From e3596137109f6cbaf9a7abf9f97757e078c63d68 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Wed, 25 Oct 2023 17:16:11 -0500 Subject: [PATCH 3/3] review feedback --- .../io/deephaven/stats/StatsCPUCollector.java | 23 ++++++++----------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/Stats/src/main/java/io/deephaven/stats/StatsCPUCollector.java b/Stats/src/main/java/io/deephaven/stats/StatsCPUCollector.java index 973e1ce2f3b..1f003b5fb1e 100644 --- a/Stats/src/main/java/io/deephaven/stats/StatsCPUCollector.java +++ b/Stats/src/main/java/io/deephaven/stats/StatsCPUCollector.java @@ -232,22 +232,19 @@ private void readToBuffer(FileChannel fileChannel, String fileName) throws IOExc while (true) { final int nb = fileChannel.read(statBuffer); - if (nb == -1) { - // EOF means success, set position to zero and limit to the data read - statBuffer.flip(); - return; - } - if (nb == 0) { - // zero bytes read is an error, proc isn't working correctly? - throw new IOException(fileName + " zero read"); + if (nb <= 0) { + // zero bytes read is an error, -1 is EOF + throw new IOException(fileName + " could not be read, or was empty"); } - if (!statBuffer.hasRemaining()) { - // allocate larger read-buffer, and read again from start - statBuffer = ByteBuffer.allocate(statBuffer.capacity() * 2); - fileChannel.position(0); - } else { + if (statBuffer.hasRemaining()) { Assert.eq(statBuffer.position(), "statBuffer.position()", nb, "nb"); + statBuffer.flip(); + return; } + + // allocate larger read-buffer, and read again from start + statBuffer = ByteBuffer.allocate(statBuffer.capacity() * 2); + fileChannel.position(0); } }