Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent unnecessary MODIFY events in WatchServiceImpl #3524

Merged
merged 2 commits into from
Apr 7, 2023
Merged

Prevent unnecessary MODIFY events in WatchServiceImpl #3524

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8407916
debug tests
J-N-K Apr 5, 2023
257832f
further work
J-N-K Apr 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 22 additions & 23 deletions ...es/org.openhab.core/src/main/java/org/openhab/core/internal/service/WatchServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import java.util.Hashtable;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.CopyOnWriteArrayList;
Expand Down Expand Up @@ -48,6 +49,7 @@
import io.methvin.watcher.DirectoryChangeEvent;
import io.methvin.watcher.DirectoryChangeListener;
import io.methvin.watcher.DirectoryWatcher;
import io.methvin.watcher.hashing.FileHash;

/**
* The {@link WatchServiceImpl} is the implementation of the {@link WatchService}
Expand All @@ -70,6 +72,7 @@ public class WatchServiceImpl implements WatchService, DirectoryChangeListener {

private final List<Listener> dirPathListeners = new CopyOnWriteArrayList<>();
private final List<Listener> subDirPathListeners = new CopyOnWriteArrayList<>();
private final Map<Path, FileHash> hashCache = new ConcurrentHashMap<>();
private final ExecutorService executor;
private final ScheduledExecutorService scheduler;

Expand All @@ -81,7 +84,7 @@ public class WatchServiceImpl implements WatchService, DirectoryChangeListener {
private @Nullable ServiceRegistration<WatchService> reg;

private final Map<Path, ScheduledFuture<?>> scheduledEvents = new HashMap<>();
private final Map<Path, List<Kind>> scheduledEventKinds = new ConcurrentHashMap<>();
private final Map<Path, List<DirectoryChangeEvent>> scheduledEventKinds = new ConcurrentHashMap<>();

@Activate
public WatchServiceImpl(WatchServiceConfiguration config, BundleContext bundleContext) throws IOException {
Expand Down Expand Up @@ -170,6 +173,8 @@ private void closeWatcherAndUnregister() throws IOException {
}
this.reg = null;
}

hashCache.clear();
}

@Override
Expand Down Expand Up @@ -217,52 +222,46 @@ public void onEvent(@Nullable DirectoryChangeEvent directoryChangeEvent) throws
}

Path path = directoryChangeEvent.path();
Kind kind = switch (directoryChangeEvent.eventType()) {
case CREATE -> Kind.CREATE;
case MODIFY -> Kind.MODIFY;
case DELETE -> Kind.DELETE;
case OVERFLOW -> Kind.OVERFLOW;
};

synchronized (scheduledEvents) {
ScheduledFuture<?> future = scheduledEvents.remove(path);
if (future != null && !future.isDone()) {
future.cancel(true);
}
future = scheduler.schedule(() -> notifyListeners(path), PROCESSING_TIME, TimeUnit.MILLISECONDS);
scheduledEventKinds.computeIfAbsent(path, k -> new CopyOnWriteArrayList<>()).add(kind);
scheduledEventKinds.computeIfAbsent(path, k -> new CopyOnWriteArrayList<>()).add(directoryChangeEvent);
scheduledEvents.put(path, future);

}
}

private void notifyListeners(Path path) {
List<Kind> kinds = scheduledEventKinds.remove(path);
if (kinds == null || kinds.isEmpty()) {
List<DirectoryChangeEvent> events = scheduledEventKinds.remove(path);
if (events == null || events.isEmpty()) {
logger.debug("Tried to notify listeners of change events for '{}', but the event list is empty.", path);
return;
}

if (kinds.size() == 1) {
// we have only one event
doNotify(path, kinds.get(0));
return;
}

Kind firstElement = kinds.get(0);
Kind lastElement = kinds.get(kinds.size() - 1);
DirectoryChangeEvent firstElement = events.get(0);
DirectoryChangeEvent lastElement = events.get(events.size() - 1);

// determine final event
if (lastElement == Kind.DELETE) {
if (firstElement == Kind.CREATE) {
logger.debug("Discarding events for '{}' because file was immediately deleted bafter creation", path);
if (lastElement.eventType() == DirectoryChangeEvent.EventType.DELETE) {
if (firstElement.eventType() == DirectoryChangeEvent.EventType.CREATE) {
logger.debug("Discarding events for '{}' because file was immediately deleted after creation", path);
return;
}
hashCache.remove(lastElement.path());
doNotify(path, Kind.DELETE);
} else if (firstElement == Kind.CREATE) {
} else if (firstElement.eventType() == DirectoryChangeEvent.EventType.CREATE) {
hashCache.put(lastElement.path(), lastElement.hash());
doNotify(path, Kind.CREATE);
} else {
doNotify(path, Kind.MODIFY);
FileHash oldHash = hashCache.put(lastElement.path(), lastElement.hash());
if (!Objects.equals(oldHash, lastElement.hash())) {
// only notify if hashes are different, otherwise the file content did not chnge
doNotify(path, Kind.MODIFY);
}
}
}

Expand Down