7169050: (se) Selector.select slow on Solaris due to insertion of POLLREMOVE and 0 events

Reviewed-by: chegar, coffeys

src/solaris/classes/sun/nio/ch/DevPollArrayWrapper.java

@@ -25,9 +25,7 @@
 
 package sun.nio.ch;
 
-import sun.misc.*;
 import java.io.IOException;
-import java.util.LinkedList;
 
 
 /**
@@ -66,6 +64,9 @@ class DevPollArrayWrapper {
     static final short EVENT_OFFSET  = 4;
     static final short REVENT_OFFSET = 6;
 
+    // Special value to indicate that an update should be ignored
+    static final byte  CANCELLED     = (byte)-1;
+
     // Maximum number of open file descriptors
     static final int   OPEN_MAX      = fdLimit();
 
@@ -74,13 +75,16 @@ class DevPollArrayWrapper {
     static final int   NUM_POLLFDS   = Math.min(OPEN_MAX-1, 8192);
 
     // Base address of the native pollArray
-    private long pollArrayAddress;
+    private final long pollArrayAddress;
 
     // Array of pollfd structs used for driver updates
-    private AllocatedNativeObject updatePollArray;
+    private final AllocatedNativeObject updatePollArray;
 
     // Maximum number of POLL_FD structs to update at once
-    private int MAX_UPDATE_SIZE = Math.min(OPEN_MAX, 10000);
+    private final int MAX_UPDATE_SIZE = Math.min(OPEN_MAX, 512);
+
+    // Initial size of arrays for fd registration changes
+    private final int INITIAL_PENDING_UPDATE_SIZE = 64;
 
     DevPollArrayWrapper() {
         int allocationSize = NUM_POLLFDS * SIZE_POLLFD;
@@ -91,19 +95,6 @@ class DevPollArrayWrapper {
         wfd = init();
     }
 
-    // Machinery for remembering fd registration changes
-    // A hashmap could be used but the number of changes pending
-    // is expected to be small
-    private static class Updator {
-        int fd;
-        int mask;
-        Updator(int fd, int mask) {
-            this.fd = fd;
-            this.mask = mask;
-        }
-    }
-    private LinkedList<Updator> updateList = new LinkedList<Updator>();
-
     // The pollfd array for results from devpoll driver
     private AllocatedNativeObject pollArray;
 
@@ -122,6 +113,20 @@ class DevPollArrayWrapper {
     // Number of updated pollfd entries
     int updated;
 
+    // object to synchronize fd registration changes
+    private final Object updateLock = new Object();
+
+    // number of file descriptors with registration changes pending
+    private int updateCount;
+
+    // file descriptors with registration changes pending
+    private int[] updateDescriptors = new int[INITIAL_PENDING_UPDATE_SIZE];
+
+    // events for file descriptors with registration changes pending, indexed
+    // by file descriptor and stored as bytes for efficiency reasons.
+    private byte[] updateEvents = new byte[OPEN_MAX];
+
+
     void initInterrupt(int fd0, int fd1) {
         outgoingInterruptFD = fd1;
         incomingInterruptFD = fd0;
@@ -149,14 +154,32 @@ class DevPollArrayWrapper {
     }
 
     void setInterest(int fd, int mask) {
-        synchronized (updateList) {
-            updateList.add(new Updator(fd, mask));
+        synchronized (updateLock) {
+            // record the file descriptor and events, expanding the
+            // respective arrays first if necessary.
+            int oldCapacity = updateDescriptors.length;
+            if (updateCount >= oldCapacity) {
+                int newCapacity = oldCapacity + INITIAL_PENDING_UPDATE_SIZE;
+                int[] newDescriptors = new int[newCapacity];
+                System.arraycopy(updateDescriptors, 0, newDescriptors, 0, oldCapacity);
+                updateDescriptors = newDescriptors;
+            }
+            updateDescriptors[updateCount++] = fd;
+
+            // events are stored as bytes for efficiency reasons
+            byte b = (byte)mask;
+            assert (b == mask) && (b != CANCELLED);
+            updateEvents[fd] = b;
         }
     }
 
     void release(int fd) {
-        synchronized (updateList) {
-            updateList.add(new Updator(fd, POLLREMOVE));
+        synchronized (updateLock) {
+            // cancel any pending update for this file descriptor
+            updateEvents[fd] = CANCELLED;
+
+            // remove from /dev/poll
+            register(wfd, fd, POLLREMOVE);
         }
     }
 
@@ -181,32 +204,37 @@ class DevPollArrayWrapper {
 
     void updateRegistrations() throws IOException {
         // Populate pollfd array with updated masks
-        synchronized (updateList) {
-            while (updateList.size() > 0) {
-                // We have to insert a dummy node in between each
-                // real update to use POLLREMOVE on the fd first because
-                // otherwise the changes are simply OR'd together
-                int index = 0;
-                Updator u = null;
-                while ((u = updateList.poll()) != null) {
-                    // First add pollfd struct to clear out this fd
-                    putPollFD(updatePollArray, index, u.fd, POLLREMOVE);
+        synchronized (updateLock) {
+
+            int j = 0;
+            int index = 0;
+            while (j < updateCount) {
+                int fd = updateDescriptors[j];
+                short events = updateEvents[fd];
+
+                // skip update if key has been cancelled
+                if (events != CANCELLED) {
+                    // remove from /dev/poll when the interest ops changes to 0
+                    if (events == 0)
+                        events = POLLREMOVE;
+
+                    // populate pollfd array with updated event
+                    putPollFD(updatePollArray, index, fd, events);
                     index++;
-                    // Now add pollfd to update this fd, if necessary
-                    if (u.mask != POLLREMOVE) {
-                        putPollFD(updatePollArray, index, u.fd, (short)u.mask);
-                        index++;
+                    if (index >= MAX_UPDATE_SIZE) {
+                        registerMultiple(wfd, updatePollArray.address(), index);
+                        index = 0;
                     }
-
-                    // Check against the max update size; these are
-                    // all we will process. Valid index ranges from 0 to
-                    // (MAX_UPDATE_SIZE - 1) and we can use up to 2 per loop
-                    if (index >  MAX_UPDATE_SIZE - 2)
-                        break;
                 }
-                // Register the changes with /dev/poll
+                j++;
+
+            }
+
+            // write any remaining updates
+            if (index > 0)
                 registerMultiple(wfd, updatePollArray.address(), index);
-             }
+
+            updateCount = 0;
         }
     }
 

src/solaris/native/sun/nio/ch/DevPollArrayWrapper.c

@@ -118,27 +118,20 @@ JNIEXPORT void JNICALL
 Java_sun_nio_ch_DevPollArrayWrapper_register(JNIEnv *env, jobject this,
                                              jint wfd, jint fd, jint mask)
 {
-    struct pollfd a[2];
-    unsigned char *pollBytes = (unsigned char *)&a[0];
-    unsigned char *pollEnd = pollBytes + sizeof(struct pollfd) * 2;
+    struct pollfd a[1];
+    int n;
 
-    /* We clear it first, otherwise any entries between poll invocations
-       get OR'd together */
     a[0].fd = fd;
-    a[0].events = POLLREMOVE;
+    a[0].events = mask;
     a[0].revents = 0;
 
-    a[1].fd = fd;
-    a[1].events = mask;
-    a[1].revents = 0;
-
-    while (pollBytes < pollEnd) {
-        int bytesWritten = write(wfd, pollBytes, (int)(pollEnd - pollBytes));
-        if (bytesWritten < 0) {
+    n = write(wfd, &a[0], sizeof(a));
+    if (n != sizeof(a)) {
+        if (n < 0) {
             JNU_ThrowIOExceptionWithLastError(env, "Error writing pollfds");
-            return;
+        } else {
+            JNU_ThrowIOException(env, "Unexpected number of bytes written");
         }
-        pollBytes += bytesWritten;
     }
 }