From 3350bbc56a3fd1dc044b60dfa0ca9f145cee2700 Mon Sep 17 00:00:00 2001 From: Joachim Ansorg Date: Tue, 22 Oct 2024 11:14:11 +0200 Subject: [PATCH] fix: debounce requests for a filesystem refresh This reduces the flickering of the "Indexing..." progress indicator in the status bar. --- .../cli/DefaultCommandLineService.java | 11 +--- .../appland/cli/DefaultVfsRefreshService.java | 52 ++++++++++++++++ .../java/appland/cli/VfsRefreshService.java | 23 ++++++++ .../main/resources/META-INF/appmap-core.xml | 8 ++- .../src/test/java/appland/AppMapBaseTest.java | 4 +- .../cli/DefaultCommandLineServiceTest.java | 4 +- .../appland/cli/TestCommandLineService.java | 41 +------------ .../appland/cli/TestVfsRefreshService.java | 59 +++++++++++++++++++ 8 files changed, 148 insertions(+), 54 deletions(-) create mode 100644 plugin-core/src/main/java/appland/cli/DefaultVfsRefreshService.java create mode 100644 plugin-core/src/main/java/appland/cli/VfsRefreshService.java create mode 100644 plugin-core/src/test/java/appland/cli/TestVfsRefreshService.java diff --git a/plugin-core/src/main/java/appland/cli/DefaultCommandLineService.java b/plugin-core/src/main/java/appland/cli/DefaultCommandLineService.java index f5b0b753..038eca6e 100644 --- a/plugin-core/src/main/java/appland/cli/DefaultCommandLineService.java +++ b/plugin-core/src/main/java/appland/cli/DefaultCommandLineService.java @@ -1,7 +1,6 @@ package appland.cli; import appland.config.AppMapConfigFile; -import appland.config.AppMapConfigFileListener; import appland.files.AppMapFiles; import appland.files.AppMapVfsUtils; import appland.settings.AppMapApplicationSettingsService; @@ -21,7 +20,6 @@ import com.intellij.openapi.util.Key; import com.intellij.openapi.util.SystemInfo; import com.intellij.openapi.util.text.StringUtil; -import com.intellij.openapi.vfs.VfsUtil; import com.intellij.openapi.vfs.VfsUtilCore; import com.intellij.openapi.vfs.VirtualFile; import com.intellij.openapi.vfs.ex.temp.TempFileSystemMarker; @@ -272,11 +270,6 @@ public void dispose() { } } - // extracted as method to allow overriding and testing it - protected void requestVirtualFileRefresh(@NotNull Path path) { - VfsUtil.markDirtyAndRefresh(true, false, false, path.toFile()); - } - @TestOnly synchronized @Nullable CliProcesses getProcesses(@NotNull VirtualFile directory) { return processes.get(directory); @@ -658,7 +651,7 @@ boolean isEmpty() { /** * Listen to index events of the indexer. */ - private class IndexEventsProcessListener extends ProcessAdapter { + private static class IndexEventsProcessListener extends ProcessAdapter { @Override public void onTextAvailable(@NotNull ProcessEvent event, @NotNull Key outputType) { if (LOG.isTraceEnabled()) { @@ -675,7 +668,7 @@ public void onTextAvailable(@NotNull ProcessEvent event, @NotNull Key outputType // Refresh parent directory of the indexed AppMap, because it contains both // myAppMap.appmap.json file and the corresponding metadata directory myAppMap/ - requestVirtualFileRefresh(Path.of(filePath).getParent()); + VfsRefreshService.getInstance().requestVirtualFileRefresh(Path.of(filePath).getParent()); } catch (InvalidPathException e) { if (LOG.isDebugEnabled()) { LOG.debug("Error parsing indexed file path: " + filePath, e); diff --git a/plugin-core/src/main/java/appland/cli/DefaultVfsRefreshService.java b/plugin-core/src/main/java/appland/cli/DefaultVfsRefreshService.java new file mode 100644 index 00000000..de427dc4 --- /dev/null +++ b/plugin-core/src/main/java/appland/cli/DefaultVfsRefreshService.java @@ -0,0 +1,52 @@ +package appland.cli; + +import com.intellij.openapi.Disposable; +import com.intellij.openapi.vfs.LocalFileSystem; +import com.intellij.openapi.vfs.VfsUtil; +import com.intellij.openapi.vfs.VirtualFile; +import com.intellij.util.Alarm; +import com.intellij.util.SingleAlarm; +import com.intellij.util.containers.ContainerUtil; +import org.jetbrains.annotations.NotNull; + +import java.nio.file.Path; +import java.util.Set; +import java.util.concurrent.CopyOnWriteArraySet; +import java.util.concurrent.atomic.AtomicReference; + +public class DefaultVfsRefreshService implements VfsRefreshService, Disposable { + // Cumulate requests for filesystem refreshes of the last 5s. + // A new requests during the 5s delay restarts the delay. + private final @NotNull SingleAlarm refreshAlarm = new SingleAlarm( + this::debouncedRefresh, + 5_000, + this, + Alarm.ThreadToUse.POOLED_THREAD, + null); + // The Set value must be thread-safe. + private final @NotNull AtomicReference<@NotNull Set> pendingRefresh = new AtomicReference<>(new CopyOnWriteArraySet<>()); + + @Override + public void dispose() { + pendingRefresh.getAndSet(Set.of()); + } + + @Override + public void requestVirtualFileRefresh(@NotNull Path path) { + pendingRefresh.get().add(path); + refreshAlarm.cancelAndRequest(); + } + + private void debouncedRefresh() { + var pending = pendingRefresh.getAndSet(new CopyOnWriteArraySet<>()); + if (pending.isEmpty()) { + return; + } + + var fs = LocalFileSystem.getInstance(); + var pendingVirtualFiles = ContainerUtil.mapNotNull(pending, fs::refreshAndFindFileByNioFile); + if (!pendingVirtualFiles.isEmpty()) { + VfsUtil.markDirtyAndRefresh(true, false, false, pendingVirtualFiles.toArray(new VirtualFile[0])); + } + } +} diff --git a/plugin-core/src/main/java/appland/cli/VfsRefreshService.java b/plugin-core/src/main/java/appland/cli/VfsRefreshService.java new file mode 100644 index 00000000..2f01529d --- /dev/null +++ b/plugin-core/src/main/java/appland/cli/VfsRefreshService.java @@ -0,0 +1,23 @@ +package appland.cli; + +import com.intellij.openapi.application.ApplicationManager; +import org.jetbrains.annotations.NotNull; + +import java.nio.file.Path; + +/** + * Service interface to allow testing of our Vfs refresh handing of AppMap data. + * Requests for a filesystem refresh must be debounced by the service implementation. + */ +public interface VfsRefreshService { + static @NotNull VfsRefreshService getInstance() { + return ApplicationManager.getApplication().getService(VfsRefreshService.class); + } + + /** + * Request a refresh of the given path. + * + * @param path File or directory to refresh. + */ + void requestVirtualFileRefresh(@NotNull Path path); +} diff --git a/plugin-core/src/main/resources/META-INF/appmap-core.xml b/plugin-core/src/main/resources/META-INF/appmap-core.xml index 7ec6cc6e..0f21b0f0 100644 --- a/plugin-core/src/main/resources/META-INF/appmap-core.xml +++ b/plugin-core/src/main/resources/META-INF/appmap-core.xml @@ -46,7 +46,8 @@ - + @@ -80,6 +81,11 @@ serviceImplementation="appland.cli.DefaultAppLandDownloadService" testServiceImplementation="appland.cli.TestAppLandDownloadService"/> + + { // copying AppMaps into a watched directory must trigger a file refresh based on the output of the AppMap process - var refreshCondition = TestCommandLineService.newVfsRefreshCondition(getProject(), getTestRootDisposable()); + var refreshCondition = TestVfsRefreshService.newVfsRefreshCondition(getProject(), getTestRootDisposable()); myFixture.copyDirectoryToProject("projects/without_existing_index", "test-project"); assertTrue(refreshCondition.await(30, TimeUnit.SECONDS)); - var refreshedFiles = TestCommandLineService.getInstance().getRefreshedFiles(); + var refreshedFiles = TestVfsRefreshService.getInstance().getRefreshedFiles(); assertSize(1, refreshedFiles); var refreshedPath = refreshedFiles.iterator().next(); assertTrue("The parent directory of the AppMap must be refreshed", refreshedPath.endsWith("appmap")); diff --git a/plugin-core/src/test/java/appland/cli/TestCommandLineService.java b/plugin-core/src/test/java/appland/cli/TestCommandLineService.java index 2063f834..08ab32bc 100644 --- a/plugin-core/src/test/java/appland/cli/TestCommandLineService.java +++ b/plugin-core/src/test/java/appland/cli/TestCommandLineService.java @@ -1,53 +1,14 @@ package appland.cli; -import com.intellij.openapi.Disposable; -import com.intellij.openapi.application.ApplicationManager; -import com.intellij.openapi.project.Project; -import com.intellij.util.containers.ContainerUtil; -import com.intellij.util.messages.Topic; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.TestOnly; -import java.nio.file.Path; -import java.util.Collections; -import java.util.Set; -import java.util.concurrent.CountDownLatch; - @TestOnly public class TestCommandLineService extends DefaultCommandLineService { - private static final Topic VFS_REFRESH_TOPIC = Topic.create("appmap.test.vfsRefresh", Runnable.class); - - private final Set refreshedFiles = ContainerUtil.newConcurrentSet(); - /** - * Condition to wait for the request to refresh a file path based on STDOUT of the AppMap indexer. + * We're exposing this to allow tests to access fields of DefaultCommandLineService. */ - public static @NotNull CountDownLatch newVfsRefreshCondition(@NotNull Project project, - @NotNull Disposable parentDisposable) { - var latch = new CountDownLatch(1); - project.getMessageBus().connect(parentDisposable).subscribe(VFS_REFRESH_TOPIC, (Runnable) latch::countDown); - return latch; - } - public static @NotNull TestCommandLineService getInstance() { return (TestCommandLineService) AppLandCommandLineService.getInstance(); } - - public Set getRefreshedFiles() { - return Collections.unmodifiableSet(refreshedFiles); - } - - public void reset() { - refreshedFiles.clear(); - } - - @Override - protected void requestVirtualFileRefresh(@NotNull Path filePath) { - try { - refreshedFiles.add(filePath); - super.requestVirtualFileRefresh(filePath); - } finally { - ApplicationManager.getApplication().getMessageBus().syncPublisher(VFS_REFRESH_TOPIC).run(); - } - } } diff --git a/plugin-core/src/test/java/appland/cli/TestVfsRefreshService.java b/plugin-core/src/test/java/appland/cli/TestVfsRefreshService.java new file mode 100644 index 00000000..8dcc8bea --- /dev/null +++ b/plugin-core/src/test/java/appland/cli/TestVfsRefreshService.java @@ -0,0 +1,59 @@ +package appland.cli; + +import com.intellij.openapi.Disposable; +import com.intellij.openapi.application.ApplicationManager; +import com.intellij.openapi.project.Project; +import com.intellij.util.containers.ContainerUtil; +import com.intellij.util.messages.Topic; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.TestOnly; + +import java.nio.file.Path; +import java.util.Collections; +import java.util.Set; +import java.util.concurrent.CountDownLatch; + +@TestOnly +public class TestVfsRefreshService extends DefaultVfsRefreshService { + private static final Topic VFS_REFRESH_TOPIC = Topic.create("appmap.test.vfsRefresh", Runnable.class); + + private final Set refreshedFiles = ContainerUtil.newConcurrentSet(); + + public static @NotNull TestVfsRefreshService getInstance() { + return (TestVfsRefreshService) VfsRefreshService.getInstance(); + } + + /** + * Condition to wait for the request to refresh a file path based on STDOUT of the AppMap indexer. + */ + public static @NotNull CountDownLatch newVfsRefreshCondition(@NotNull Project project, + @NotNull Disposable parentDisposable) { + var latch = new CountDownLatch(1); + project.getMessageBus().connect(parentDisposable).subscribe(VFS_REFRESH_TOPIC, (Runnable) latch::countDown); + return latch; + } + + public @NotNull Set getRefreshedFiles() { + return Collections.unmodifiableSet(refreshedFiles); + } + + @Override + public void dispose() { + reset(); + super.dispose(); + } + + public void reset() { + refreshedFiles.clear(); + } + + @Override + public void requestVirtualFileRefresh(@NotNull Path path) { + try { + refreshedFiles.add(path); + super.requestVirtualFileRefresh(path); + } finally { + ApplicationManager.getApplication().getMessageBus().syncPublisher(VFS_REFRESH_TOPIC).run(); + } + } +} \ No newline at end of file