From 2c91836c8d4232f26a5b764ea91b7cdb47240b7e Mon Sep 17 00:00:00 2001 From: Damien Martin-Guillerez Date: Thu, 13 Oct 2016 19:18:13 +0000 Subject: Rollback of commit 2891ec527eed27d0c6460d66f51cb66a43373b6a. *** Reason for rollback *** Causes our integration tests on Darwin to time out *** Original change description *** Make --watchfs a common command option. Adding an options parameter to DiffAwareness#getCurrentView seems like the simplest way to achieve that. Alternatives considered: 1. Making the diff awareness modules stateful. However, I did not want to do so as I've also been working on improving the module API to reduce state, or at least to have a proper lifecycle management for any necessary state. 2. Making the watchFs flag a constructor parameter. However, that would also invalidate any implement... *** -- MOS_MIGRATED_REVID=136070807 --- .../lib/skyframe/MacOSXFsEventsDiffAwareness.java | 71 ++++++---------------- 1 file changed, 17 insertions(+), 54 deletions(-) (limited to 'src/main/java/com/google/devtools/build/lib/skyframe/MacOSXFsEventsDiffAwareness.java') diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/MacOSXFsEventsDiffAwareness.java b/src/main/java/com/google/devtools/build/lib/skyframe/MacOSXFsEventsDiffAwareness.java index f56611aca3..cf0fc621b5 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/MacOSXFsEventsDiffAwareness.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/MacOSXFsEventsDiffAwareness.java @@ -17,7 +17,7 @@ package com.google.devtools.build.lib.skyframe; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.UnixJniLoader; -import com.google.devtools.common.options.OptionsClassProvider; + import java.io.File; import java.nio.file.Path; @@ -25,11 +25,11 @@ import java.nio.file.Path; * A {@link DiffAwareness} that use fsevents to watch the filesystem to use in lieu of * {@link LocalDiffAwareness}. * - *

On OS X, the local diff awareness cannot work because WatchService is dummy and do polling, - * which is slow (https://bugs.openjdk.java.net/browse/JDK-7133447). + *

+ * On OS X, the local diff awareness cannot work because WatchService is dummy and do polling, which + * is slow (https://bugs.openjdk.java.net/browse/JDK-7133447). */ public final class MacOSXFsEventsDiffAwareness extends LocalDiffAwareness { - private final double latency; private boolean closed; @@ -37,15 +37,23 @@ public final class MacOSXFsEventsDiffAwareness extends LocalDiffAwareness { // structure). private long nativePointer; - private boolean opened; - /** * Watch changes on the file system under watchRoot with a granularity of * delay seconds. */ MacOSXFsEventsDiffAwareness(String watchRoot, double latency) { super(watchRoot); - this.latency = latency; + create(new String[] {watchRootPath.toAbsolutePath().toString()}, latency); + + // Start a thread that just contains the OS X run loop. + new Thread( + new Runnable() { + @Override + public void run() { + MacOSXFsEventsDiffAwareness.this.run(); + } + }) + .start(); } /** @@ -65,35 +73,15 @@ public final class MacOSXFsEventsDiffAwareness extends LocalDiffAwareness { */ private native void run(); - private void init() { - // The code below is based on the assumption that init() can never fail, which is currently the - // case; if you change init(), then you also need to update {@link #getCurrentView}. - Preconditions.checkState(!opened); - opened = true; - create(new String[] {watchRootPath.toAbsolutePath().toString()}, latency); - // Start a thread that just contains the OS X run loop. - new Thread( - new Runnable() { - @Override - public void run() { - MacOSXFsEventsDiffAwareness.this.run(); - } - }) - .start(); - } - /** * Close this watch service, this service should not be used any longer after closing. */ - @Override public synchronized void close() { Preconditions.checkState(!closed); closed = true; doClose(); } - private static final boolean JNI_AVAILABLE; - /** * JNI code stopping the main loop and shutting down listening to FSEvents. */ @@ -105,36 +93,11 @@ public final class MacOSXFsEventsDiffAwareness extends LocalDiffAwareness { private native String[] poll(); static { - boolean loadJniWorked = false; - try { - UnixJniLoader.loadJni(); - loadJniWorked = true; - } catch (UnsatisfiedLinkError ignored) { - // Unfortunately, we compile this class into the Bazel bootstrap binary, which doesn't have - // access to the JNI code (to simplify bootstrap). This is the quick and dirty way to - // hard-disable --watchfs in the bootstrap binary. - } - JNI_AVAILABLE = loadJniWorked; + UnixJniLoader.loadJni(); } @Override - public synchronized View getCurrentView(OptionsClassProvider options) - throws BrokenDiffAwarenessException { - if (!JNI_AVAILABLE) { - return EVERYTHING_MODIFIED; - } - // See WatchServiceDiffAwareness#getCurrentView for an explanation of this logic. - boolean watchFs = options.getOptions(Options.class).watchFS; - if (watchFs && !opened) { - init(); - } else if (!watchFs && opened) { - close(); - throw new BrokenDiffAwarenessException("Switched off --watchfs again"); - } else if (!opened) { - // The only difference with WatchServiceDiffAwareness#getCurrentView is this if; the init() - // call above can never fail, so we don't need to re-check the opened flag after init(). - return EVERYTHING_MODIFIED; - } + public synchronized View getCurrentView() throws BrokenDiffAwarenessException { Preconditions.checkState(!closed); ImmutableSet.Builder paths = ImmutableSet.builder(); for (String path : poll()) { -- cgit v1.2.3