6929532: (file) WatchService should avoid queuing new modify events when lots of files are changing

Reviewed-by: alanb
Contributed-by: sebastian.sickelmann@gmx.de

jdk/src/share/classes/sun/nio/fs/AbstractWatchKey.java

@@ -59,10 +59,15 @@ abstract class AbstractWatchKey extends WatchKey {
     // pending events
     private List<WatchEvent<?>> events;
 
+    // maps a context to the last event for the context (iff the last queued
+    // event for the context is an ENTRY_MODIFY event).
+    private Map<Object,WatchEvent<?>> lastModifyEvents;
+
     protected AbstractWatchKey(AbstractWatchService watcher) {
         this.watcher = watcher;
         this.state = State.READY;
         this.events = new ArrayList<WatchEvent<?>>();
+        this.lastModifyEvents = new HashMap<Object,WatchEvent<?>>();
     }
 
     final AbstractWatchService watcher() {
@@ -86,6 +91,7 @@ abstract class AbstractWatchKey extends WatchKey {
      */
     @SuppressWarnings("unchecked")
     final void signalEvent(WatchEvent.Kind<?> kind, Object context) {
+        boolean isModify = (kind == StandardWatchEventKind.ENTRY_MODIFY);
         synchronized (this) {
             int size = events.size();
             if (size > 0) {
@@ -100,17 +106,43 @@ abstract class AbstractWatchKey extends WatchKey {
                     return;
                 }
 
+                // if this is a modify event and the last entry for the context
+                // is a modify event then we simply increment the count
+                if (!lastModifyEvents.isEmpty()) {
+                    if (isModify) {
+                        WatchEvent<?> ev = lastModifyEvents.get(context);
+                        if (ev != null) {
+                            assert ev.kind() == StandardWatchEventKind.ENTRY_MODIFY;
+                            ((Event<?>)ev).increment();
+                            return;
+                        }
+                    } else {
+                        // not a modify event so remove from the map as the
+                        // last event will no longer be a modify event.
+                        lastModifyEvents.remove(context);
+                    }
+                }
+
                 // if the list has reached the limit then drop pending events
                 // and queue an OVERFLOW event
                 if (size >= MAX_EVENT_LIST_SIZE) {
-                    events.clear();
                     kind = StandardWatchEventKind.OVERFLOW;
+                    isModify = false;
                     context = null;
                 }
             }
 
             // non-repeated event
-            events.add(new Event<Object>((WatchEvent.Kind<Object>)kind, context));
+            Event<Object> ev =
+                new Event<Object>((WatchEvent.Kind<Object>)kind, context);
+            if (isModify) {
+                lastModifyEvents.put(context, ev);
+            } else if (kind == StandardWatchEventKind.OVERFLOW) {
+                // drop all pending events
+                events.clear();
+                lastModifyEvents.clear();
+            }
+            events.add(ev);
             signal();
         }
     }
@@ -120,6 +152,7 @@ abstract class AbstractWatchKey extends WatchKey {
         synchronized (this) {
             List<WatchEvent<?>> result = events;
             events = new ArrayList<WatchEvent<?>>();
+            lastModifyEvents.clear();
             return result;
         }
     }

jdk/test/java/nio/file/WatchService/OverflowEventIsLoner.java → jdk/test/java/nio/file/WatchService/LotsOfEvents.java

@@ -22,22 +22,74 @@
  */
 
 /* @test
- * @bug 6907760
- * @summary Check that the OVERFLOW event is not retrieved with other events
+ * @bug 6907760 6929532
+ * @summary Tests WatchService behavior when lots of events are pending
  * @library ..
+ * @run main/timeout=180 LotsOfEvents
  */
 
 import java.nio.file.*;
 import static java.nio.file.StandardWatchEventKind.*;
 import java.io.IOException;
-import java.util.List;
+import java.io.OutputStream;
+import java.util.*;
 import java.util.concurrent.TimeUnit;
 
-public class OverflowEventIsLoner {
+public class LotsOfEvents {
 
-    static void drainEvents(WatchService watcher,
-                            WatchEvent.Kind<?> expectedKind,
-                            int count)
+    static final Random rand = new Random();
+
+    public static void main(String[] args) throws Exception {
+        Path dir = TestUtil.createTemporaryDirectory();
+        try {
+            testOverflowEvent(dir);
+            testModifyEventsQueuing(dir);
+        } finally {
+            TestUtil.removeAll(dir);
+        }
+    }
+
+    /**
+     * Tests that OVERFLOW events are not retreived with other events.
+     */
+    static void testOverflowEvent(Path dir)
+        throws IOException, InterruptedException
+    {
+        WatchService watcher = dir.getFileSystem().newWatchService();
+        try {
+            dir.register(watcher, ENTRY_CREATE, ENTRY_DELETE);
+
+            // create a lot of files
+            int n = 1024;
+            Path[] files = new Path[n];
+            for (int i=0; i<n; i++) {
+                files[i] = dir.resolve("foo" + i).createFile();
+            }
+
+            // give time for events to accumulate (improve chance of overflow)
+            Thread.sleep(1000);
+
+            // check that we see the create events (or overflow)
+            drainAndCheckOverflowEvents(watcher, ENTRY_CREATE, n);
+
+            // delete the files
+            for (int i=0; i<n; i++) {
+                files[i].delete();
+            }
+
+            // give time for events to accumulate (improve chance of overflow)
+            Thread.sleep(1000);
+
+            // check that we see the delete events (or overflow)
+            drainAndCheckOverflowEvents(watcher, ENTRY_DELETE, n);
+        } finally {
+            watcher.close();
+        }
+    }
+
+    static void drainAndCheckOverflowEvents(WatchService watcher,
+                                            WatchEvent.Kind<?> expectedKind,
+                                            int count)
         throws IOException, InterruptedException
     {
         // wait for key to be signalled - the timeout is long to allow for
@@ -77,46 +129,94 @@ public class OverflowEventIsLoner {
             throw new RuntimeException("Insufficient events");
     }
 
+    /**
+     * Tests that check that ENTRY_MODIFY events are queued efficiently
+     */
+    static void testModifyEventsQueuing(Path dir)
+        throws IOException, InterruptedException
+    {
+        // this test uses a random number of files
+        final int nfiles = 5 + rand.nextInt(10);
+        DirectoryEntry[] entries = new DirectoryEntry[nfiles];
+        for (int i=0; i<nfiles; i++) {
+            entries[i] = new DirectoryEntry(dir.resolve("foo" + i));
+
+            // "some" of the files exist, some do not.
+            entries[i].deleteIfExists();
+            if (rand.nextBoolean())
+                entries[i].create();
+        }
 
-    static void test(Path dir) throws IOException, InterruptedException {
         WatchService watcher = dir.getFileSystem().newWatchService();
         try {
-            WatchKey key = dir.register(watcher, ENTRY_CREATE, ENTRY_DELETE);
+            dir.register(watcher, ENTRY_CREATE, ENTRY_DELETE, ENTRY_MODIFY);
 
-            // create a lot of files
-            int n = 1024;
-            Path[] files = new Path[n];
-            for (int i=0; i<n; i++) {
-                files[i] = dir.resolve("foo" + i).createFile();
-            }
+            // do several rounds of noise and test
+            for (int round=0; round<10; round++) {
 
-            // give time for events to accumulate (improve chance of overflow)
-            Thread.sleep(1000);
-
-            // check that we see the create events (or overflow)
-            drainEvents(watcher, ENTRY_CREATE, n);
+                // make some noise!!!
+                for (int i=0; i<100; i++) {
+                    DirectoryEntry entry = entries[rand.nextInt(nfiles)];
+                    int action = rand.nextInt(10);
+                    switch (action) {
+                        case 0 : entry.create(); break;
+                        case 1 : entry.deleteIfExists(); break;
+                        default: entry.modifyIfExists();
+                    }
+                }
 
-            // delete the files
-            for (int i=0; i<n; i++) {
-                files[i].delete();
+                // process events and ensure that we don't get repeated modify
+                // events for the same file.
+                WatchKey key = watcher.poll(15, TimeUnit.SECONDS);
+                do {
+                    Set<Path> modified = new HashSet<Path>();
+                    for (WatchEvent<?> event: key.pollEvents()) {
+                        WatchEvent.Kind<?> kind = event.kind();
+                        Path file = (kind == OVERFLOW) ? null : (Path)event.context();
+                        if (kind == ENTRY_MODIFY) {
+                            boolean added = modified.add(file);
+                            if (!added) {
+                                throw new RuntimeException(
+                                    "ENTRY_MODIFY events not queued efficiently");
+                            }
+                        } else {
+                            if (file != null) modified.remove(file);
+                        }
+                    }
+                    if (!key.reset())
+                        throw new RuntimeException("Key is no longer valid");
+                    key = watcher.poll(2, TimeUnit.SECONDS);
+                } while (key != null);
             }
 
-            // give time for events to accumulate (improve chance of overflow)
-            Thread.sleep(1000);
-
-            // check that we see the delete events (or overflow)
-            drainEvents(watcher, ENTRY_DELETE, n);
         } finally {
             watcher.close();
         }
     }
 
-    public static void main(String[] args) throws Exception {
-        Path dir = TestUtil.createTemporaryDirectory();
-        try {
-            test(dir);
-        } finally {
-            TestUtil.removeAll(dir);
+    static class DirectoryEntry {
+        private final Path file;
+        DirectoryEntry(Path file) {
+            this.file = file;
+        }
+        void create() throws IOException {
+            if (file.notExists())
+                file.createFile();
+
+        }
+        void deleteIfExists() throws IOException {
+            file.deleteIfExists();
+        }
+        void modifyIfExists() throws IOException {
+            if (file.exists()) {
+                OutputStream out = file.newOutputStream(StandardOpenOption.APPEND);
+                try {
+                    out.write("message".getBytes());
+                } finally {
+                    out.close();
+                }
+            }
         }
     }
+
 }