Reduce busy-waiting CPU load in waitEvents() (#201)

The {@link #waitEvents()} implementation on non-windows systems
usually returns immediately. This is unfortunate for the callers
which want to await events in an infinite-loop. As doing so would
burn lot of CPU time.

For example: Code in `LinuxEventThread.run()` (in `SerialPort.java`)
calls us in an infinite-loop. As a work-around, it uses a (very) small
sleep, to not utilize a full CPU core all the time. But still, this
permanently wastes a lot of CPU cycles (that many, that it is a problem
in our production use-case). The win32 code uses `OVERLAPPED` structs
and `WaitSingleObject()` which already provide that kind of "wait"
mechanism.

Not perfect, but this patch at least provides a way to wait if nothing
is ready. That "feature" is off by default and can be enabled
individually.

Author: hiddenalpha

.github/workflows/build.yml

@@ -52,7 +52,7 @@ jobs:
       - run: mvn -P "${{ matrix.profile }}" --batch-mode
 
   macos:
-    runs-on: [macos-latest]
+    runs-on: [macos-14]
     strategy:
       fail-fast: false
       matrix:
@@ -106,4 +106,4 @@ jobs:
           java-version: 11
           distribution: temurin
 
-      - run: mvn -P "${{ matrix.profile }}" --batch-mode
+      - run: mvn -P "${{ matrix.profile }}" --batch-mode

CMakeLists.txt

@@ -1,4 +1,4 @@
-cmake_minimum_required(VERSION 3.0)
+cmake_minimum_required(VERSION 3.5)
 cmake_policy(SET CMP0048 NEW)
 cmake_policy(SET CMP0042 NEW)
 

src/main/cpp/_nix_based/jssc.cpp

@@ -907,13 +907,52 @@ const jint events[] = {INTERRUPT_BREAK,
                        //EV_RXFLAG, //Not supported
                        EV_TXEMPTY};
 
+
 /* OK */
 /*
  * Collecting data for EventListener class (Linux have no implementation of "WaitCommEvent" function from Windows)
  * 
  */
 JNIEXPORT jobjectArray JNICALL Java_jssc_SerialNativeInterface_waitEvents
-  (JNIEnv *env, jobject, jlong portHandle) {
+  ( JNIEnv*env, jobject, jlong portHandle, jint waitEventsTimeoutMs) {
+    int err;
+
+    /* Code in `LinuxEventThread.run()` (in `SerialPort.java`) calls us
+     * in an infinite-loop. As a work-around, it uses a (very) small
+     * sleep, to not utilize a full CPU all the time. But still, this
+     * permanently wastes a lot of CPU cycles (that many, that it is a
+     * problem in our production use-case). The win32 code uses
+     * `OVERLAPPED` structs and `WaitSingleObject()` which already
+     * provide that kind of "wait" mechanism. But we do not have a
+     * win32-API here. As this impl here returns immediately, we'll
+     * first ask `poll()` (if available and enabled). This way we can
+     * "emulate" to actually wait if nothing is ready.
+     * See also JavaDoc of `SerialPort.setWaitEventsTimeoutMs(int)`. */
+    int const isFeatureEnabled = (waitEventsTimeoutMs >= 1);
+    if( isFeatureEnabled ){
+#if !HAVE_POLL
+        static unsigned cnt = 0;
+        if( ((cnt++) & 0xFFFF) == 0 ){
+            fprintf(stderr, "WARN: waitEventsTimeoutMs not available on your platform, as `poll()` not available.\n");
+        }
+#else
+        struct pollfd pfd = {0};
+        pfd.fd = portHandle;
+        pfd.events = POLLIN | POLLPRI | POLLRDHUP;
+        err = poll(&pfd, 1, waitEventsTimeoutMs);
+        if( err == -1 ) switch( errno ){
+        case EINTR:
+            /* Got interrupted by signal. Go report events we have so far. */
+            break;
+        default:
+            /* some error occurred. */
+            err = errno; /* bkup `errno` before calling into `FindClass()` */
+            jclass exClz = env->FindClass("java/lang/RuntimeException");
+            if( exClz ) env->ThrowNew(exClz, strerror(err));
+            return NULL;
+        }
+#endif
+    }
 
     jclass intClass = env->FindClass("[I");
     if( intClass == NULL ) return NULL;

src/main/cpp/jssc_SerialNativeInterface.h

@@ -447,12 +447,13 @@ JNIEXPORT jboolean JNICALL Java_jssc_SerialNativeInterface_sendBreak
 	return returnValue;
 }
 
+
 /*
  * Wait event
  * portHandle - port handle
  */
 JNIEXPORT jobjectArray JNICALL Java_jssc_SerialNativeInterface_waitEvents
-  (JNIEnv *env, jobject, jlong portHandle) {
+  ( JNIEnv*env, jobject, jlong portHandle, jint/*unused on windows*/ ){
     HANDLE hComm = (HANDLE)portHandle;
     DWORD lpEvtMask = 0;
     DWORD lpNumberOfBytesTransferred = 0;

src/main/cpp/windows/jssc.cpp

@@ -234,10 +234,12 @@ public static String getLibraryVersion() {
      *
      * @param handle handle of opened port
      *
+     * @param waitEventsTimeoutMs See {@link SerialPort#setWaitEventsTimeoutMs(int)}.
+     *
      * @return Method returns two-dimensional array containing event types and their values
      * (<b>events[i][0] - event type</b>, <b>events[i][1] - event value</b>).
      */
-    public native int[][] waitEvents(long handle);
+    public native int[][] waitEvents(long handle, int waitEventsTimeoutMs);
 
     /**
      * Change RTS line state

src/main/java/jssc/SerialNativeInterface.java

@@ -41,6 +41,7 @@ public class SerialPort {
     private final String portName;
     private volatile boolean portOpened = false;
     private boolean maskAssigned = false;
+    private volatile int waitEventsTimeoutMs = -1;
 
     //since 2.2.0 ->
     private volatile Method methodErrorOccurred = null;
@@ -920,6 +921,43 @@ public boolean setFlowControlMode(int mask) throws SerialPortException {
         return serialInterface.setFlowControlMode(portHandle, mask);
     }
 
+    /**
+     * Reduce busy-waiting CPU load in {@link #waitEvents()}.
+     *
+     * (This is irrelevant for windows)
+     *
+     * The {@link #waitEvents()} implementation on non-windows systems
+     * usually returns immediately. This is unfortunate for the callers
+     * which want to await events in an infinite-loop. As doing so would
+     * burn lot of CPU time.
+     *
+     * This setting can be used to reduce that load. For regular
+     * incoming data events this does not cause any further delays.
+     * {@link #waitEvents()} still will reports most of the events as
+     * soon they become available, even before the specified timeout got
+     * reached.
+     *
+     * Choosing "good value" solely depends on the callers use-case. I
+     * know of a project which works perfectly fine using 100ms.
+     *
+     * Special values: Pass `-1` to explicitly disable the feature
+     * (You'll likely not need this, as feature is disabled by default
+     * anyway). Passing any other negative values is NOT allowed.
+     * Passing `0` is NOT allowed. Instead, disable the feature if you
+     * need "no timeout".
+     *
+     * BUT BE AWARE: Enabling this might delay delivery of some special
+     * serial-events (like 'DCD line changed' or 'RI line changed') by
+     * the amount of time specified. So you have to decide yourself if
+     * you can/will afford this trade.
+     */
+    public void setWaitEventsTimeoutMs(int waitEventsTimeoutMs) {
+        if (waitEventsTimeoutMs <= 0 && waitEventsTimeoutMs != -1) {
+            throw new IllegalArgumentException(String.valueOf(waitEventsTimeoutMs));
+        }
+        this.waitEventsTimeoutMs = waitEventsTimeoutMs;
+    }
+
     /**
      * Get flow control mode
      *
@@ -951,7 +989,7 @@ public boolean sendBreak(int duration)throws SerialPortException {
     }
 
     private int[][] waitEvents() {
-        return serialInterface.waitEvents(portHandle);
+        return serialInterface.waitEvents(portHandle, waitEventsTimeoutMs);
     }
 
     /**
@@ -1263,7 +1301,7 @@ private class LinuxEventThread extends EventThread {
 
         //Need to get initial states
         public LinuxEventThread(){
-            int[][] eventArray = waitEvents();
+            int[][] eventArray = serialInterface.waitEvents(portHandle, -1);
             for(int[] event : eventArray){
                 int eventType = event[0];
                 int eventValue = event[1];

src/main/java/jssc/SerialPort.java

@@ -0,0 +1,78 @@
+package jssc;
+
+import jssc.junit.rules.DisplayMethodNameRule;
+import org.junit.Test;
+import org.slf4j.Logger;
+
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+import static org.slf4j.LoggerFactory.getLogger;
+
+
+public class SerialPortTest extends DisplayMethodNameRule {
+
+    private static final Logger log = getLogger(SerialPortTest.class);
+
+
+    @Test
+    public void expectSettingToBeSetSuccessfully() {
+        SerialPort serial = new SerialPort("ttyS0");
+        /* 100ms proved to be a reasonable value in the use-case our using
+         * project had. */
+        serial.setWaitEventsTimeoutMs(100);
+    }
+
+
+    /**
+     * Cannot really test this deeply. Just make sure the setter accepts the
+     * value.
+     */
+    @Test
+    public void disableFeatureByPassingMinusOne() {
+        SerialPort serial = new SerialPort("ttyS0");
+        serial.setWaitEventsTimeoutMs(-1);
+    }
+
+
+    /**
+     * configuring a zero-length timeout doesn't make any sense. I'd expect
+     * this to have the same effect as using no timeout in the 1st place.
+     * With the difference, we will have nonsense calls to `poll`.
+     *
+     * As soon someone really has the need to pass zero, then inverse this
+     * test and EXPLAIN CLEARLY by replacing this comment why this is the case.
+     */
+    @Test
+    public void mustNotPassZero() {
+        SerialPort serial = new SerialPort("ttyS0");
+        try {
+            serial.setWaitEventsTimeoutMs(0);
+            fail("Where's the exception?");
+        } catch (IllegalArgumentException e) {
+            assertTrue(e.getMessage().contains("0"));
+        }
+    }
+
+
+    /**
+     * Prefer to tell the user right away whenever nonsense values are passed.
+     * Makes bugs to appear early in place of them hiding silent.
+     */
+    @Test
+    public void mustNotPassAnyOtherNegativeValues() {
+        /* just try a bunch of illegal values (testing ALL possible cases might
+         * take a bit too long) */
+        SerialPort serial = new SerialPort("ttyS0");
+        for(int badTimeoutMs = -42 ; badTimeoutMs <= -2 ; ++badTimeoutMs ){
+            log.debug("setWaitEventsTimeoutMs({})", badTimeoutMs);
+            try {
+                serial.setWaitEventsTimeoutMs(badTimeoutMs);
+                fail("Where's the exception for "+ badTimeoutMs +"?");
+            } catch (IllegalArgumentException e) {
+                assertTrue(e.getMessage().contains(String.valueOf(badTimeoutMs)));
+            }
+        }
+    }
+
+
+}