From 6287d2f876ca438ebaccc3a95ba3bb6f723ab92e Mon Sep 17 00:00:00 2001 From: "Andrei.Kuznetsov" Date: Mon, 20 Oct 2025 17:03:12 +0200 Subject: [PATCH] IJPL-7536 GCWatcher now uses JBR.fullGC (if available) to collect soft references GitOrigin-RevId: d5406b22a1c63e875d7a2c3761722254ba31e570 --- .../java/index/ConcurrentIndexTest.java | 10 +- .../SmartPsiElementPointersTest.java | 4 +- .../com/intellij/util/indexing/IndexTest.java | 4 +- .../psi/impl/PsiDocumentManagerImplTest.java | 15 +- .../ContainerUtilCollectionsTest.java | 15 +- platform/util-ex/BUILD.bazel | 1 + .../util-ex/intellij.platform.util.ex.iml | 1 + .../src/com/intellij/util/ref/GCWatcher.java | 132 +++++++++++++++--- .../src/com/intellij/util/ref/GCUtil.java | 2 +- .../com/intellij/util/ref/GCWatcherTest.java | 78 +++++++++++ 10 files changed, 226 insertions(+), 36 deletions(-) rename platform/{util => util-ex}/src/com/intellij/util/ref/GCWatcher.java (52%) create mode 100644 platform/util/testSrc/com/intellij/util/ref/GCWatcherTest.java diff --git a/java/java-tests/testSrc/com/intellij/java/index/ConcurrentIndexTest.java b/java/java-tests/testSrc/com/intellij/java/index/ConcurrentIndexTest.java index ee6baef97746..5715b40d66d8 100644 --- a/java/java-tests/testSrc/com/intellij/java/index/ConcurrentIndexTest.java +++ b/java/java-tests/testSrc/com/intellij/java/index/ConcurrentIndexTest.java @@ -110,7 +110,8 @@ public class ConcurrentIndexTest extends JavaCodeInsightFixtureTestCase { ((PsiJavaFile)file).getImportList() .add(JavaPsiFacade.getElementFactory(getProject()).createImportStatementOnDemand("foo.bar" + finalI)); }); - GCWatcher.tracking(file.getNode()).ensureCollected(); + // Wait for com.intellij.codeInsight.daemon.impl.PsiChangeHandler$Change to release the reference + GCWatcher.tracking(file.getNode()).ensureCollectedWithinTimeout(5_000); assertFalse(file.isContentsLoaded()); List> futuresToWait = new ArrayList<>(); @@ -170,7 +171,8 @@ public class ConcurrentIndexTest extends JavaCodeInsightFixtureTestCase { ((PsiJavaFile)file).getImportList() .add(JavaPsiFacade.getElementFactory(getProject()).createImportStatementOnDemand("foo.bar" + finalI)); }); - GCWatcher.tracking(file.getNode()).ensureCollected(); + + GCWatcher.tracking(file.getNode()).ensureCollectedWithinTimeout(5_000); // wait for document commit queue assertFalse(file.isContentsLoaded()); myFixture.addFileToProject("Foo" + i + ".java", @@ -233,7 +235,9 @@ public class ConcurrentIndexTest extends JavaCodeInsightFixtureTestCase { document.insertString(document.getText().indexOf("(null") + 1, " "); PsiDocumentManager.getInstance(getProject()).commitAllDocuments(); }); - GCWatcher.tracking(file.getNode()).ensureCollected(); + + // Wait for com.intellij.codeInsight.daemon.impl.PsiChangeHandler$Change to release the reference + GCWatcher.tracking(file.getNode()).ensureCollectedWithinTimeout(5_000); assertFalse(file.isContentsLoaded()); assertTrue(file.getNode().getLighterAST() instanceof FCTSBackedLighterAST); diff --git a/java/java-tests/testSrc/com/intellij/psi/impl/smartPointers/SmartPsiElementPointersTest.java b/java/java-tests/testSrc/com/intellij/psi/impl/smartPointers/SmartPsiElementPointersTest.java index 2f8ef68df24d..acb8358f07af 100644 --- a/java/java-tests/testSrc/com/intellij/psi/impl/smartPointers/SmartPsiElementPointersTest.java +++ b/java/java-tests/testSrc/com/intellij/psi/impl/smartPointers/SmartPsiElementPointersTest.java @@ -212,7 +212,9 @@ public class SmartPsiElementPointersTest extends JavaCodeInsightTestCase { PsiDocumentManager.getInstance(myProject).commitAllDocuments(); }); - GCWatcher.tracking(myFile.getNode()).ensureCollected(); + // Wait for com.intellij.codeInsight.daemon.impl.PsiChangeHandler$Change to release the reference + GCWatcher.tracking(myFile.getNode()).ensureCollectedWithinTimeout(5_000); + assertEquals(myFile.getFirstChild(), pointer.getElement()); } diff --git a/java/java-tests/testSrc/com/intellij/util/indexing/IndexTest.java b/java/java-tests/testSrc/com/intellij/util/indexing/IndexTest.java index 8efe8d7ab67e..d69319383e88 100644 --- a/java/java-tests/testSrc/com/intellij/util/indexing/IndexTest.java +++ b/java/java-tests/testSrc/com/intellij/util/indexing/IndexTest.java @@ -75,7 +75,6 @@ import com.intellij.util.io.EnumeratorStringDescriptor; import com.intellij.util.io.PersistentMapImpl; import com.intellij.util.ref.GCUtil; import com.intellij.util.ref.GCWatcher; -import com.intellij.util.ui.UIUtil; import com.intellij.workspaceModel.ide.impl.WorkspaceEntityLifecycleSupporterUtils; import com.siyeh.ig.JavaOverridingMethodUtil; import kotlin.Unit; @@ -519,8 +518,9 @@ public class IndexTest extends JavaCodeInsightFixtureTestCase { //Let's help GC, even in interpreter mode //noinspection UnusedAssignment psiFile = null; + GCWatcher.tracking(getPsiManager().getFileManager().getCachedPsiFile(vFile)) - .ensureCollected(() -> UIUtil.dispatchAllInvocationEvents()); + .ensureCollectedWithinTimeout(5_000, PlatformTestUtil::dispatchAllInvocationEventsInIdeEventQueue); // wait for document commit queue assertNull(getPsiManager().getFileManager().getCachedPsiFile(vFile)); WriteAction.run(() -> VfsUtil.saveText(vFile, "class Foo3 {}")); diff --git a/platform/platform-tests/testSrc/com/intellij/psi/impl/PsiDocumentManagerImplTest.java b/platform/platform-tests/testSrc/com/intellij/psi/impl/PsiDocumentManagerImplTest.java index 024cbf1ea941..dfa3e3f987b5 100644 --- a/platform/platform-tests/testSrc/com/intellij/psi/impl/PsiDocumentManagerImplTest.java +++ b/platform/platform-tests/testSrc/com/intellij/psi/impl/PsiDocumentManagerImplTest.java @@ -1110,7 +1110,8 @@ public class PsiDocumentManagerImplTest extends HeavyPlatformTestCase { while (addRunOnCommitEntered.get() == 0) { Thread.onSpinWait(); } - GCWatcher.tracking(Arrays.asList(ReadAction.compute(()->getPsiDocumentManager().getUncommittedDocuments()))).ensureCollected(); + GCWatcher.tracking(Arrays.asList(ReadAction.compute(()->getPsiDocumentManager().getUncommittedDocuments()))) + .ensureCollectedWithinTimeout(5_000); // wait for document commit queue assertEquals(0, committed.get()); } finally { @@ -1118,7 +1119,7 @@ public class PsiDocumentManagerImplTest extends HeavyPlatformTestCase { } }); while (!future.isDone()) { - PlatformTestUtil.dispatchAllEventsInIdeEventQueue(); + PlatformTestUtil.dispatchAllInvocationEventsInIdeEventQueue(); } future.get(); } @@ -1152,11 +1153,15 @@ public class PsiDocumentManagerImplTest extends HeavyPlatformTestCase { modifyAndSaveDocument(virtualFile); // no dispatching here, to ensure the doc is not committed but stored in some queue in DCT - GCWatcher.tracking(FileDocumentManager.getInstance().getDocument(virtualFile)).ensureCollected(); + // TODO: ^^ DCT uses async WA now (document.async.commit.with.coroutines=true), + // so it is probably already working on the document in the background. + GCWatcher.tracking(FileDocumentManager.getInstance().getDocument(virtualFile)) + .ensureCollectedWithinTimeout(5_000); // wait for document commit queue while (!psiDocumentManager.isCommitted(FileDocumentManager.getInstance().getDocument(virtualFile))) { - UIUtil.dispatchAllInvocationEvents(); - GCWatcher.tracking(FileDocumentManager.getInstance().getDocument(virtualFile)).ensureCollected(); + PlatformTestUtil.dispatchAllInvocationEventsInIdeEventQueue(); + GCWatcher.tracking(FileDocumentManager.getInstance().getDocument(virtualFile)) + .ensureCollectedWithinTimeout(5_000); // wait for document commit queue } } } diff --git a/platform/platform-tests/testSrc/com/intellij/util/containers/ContainerUtilCollectionsTest.java b/platform/platform-tests/testSrc/com/intellij/util/containers/ContainerUtilCollectionsTest.java index bd1fc3530941..2b27c0f45920 100644 --- a/platform/platform-tests/testSrc/com/intellij/util/containers/ContainerUtilCollectionsTest.java +++ b/platform/platform-tests/testSrc/com/intellij/util/containers/ContainerUtilCollectionsTest.java @@ -577,10 +577,16 @@ public class ContainerUtilCollectionsTest extends Assert { map.put("a", ref1.get()); map.put("b", ref2.get()); - GCWatcher.fromClearedRef(ref2).ensureCollected(); + GCWatcher gcwRef2 = GCWatcher.fromClearedRef(ref2); + gcwRef2.ensureCollected(); + + gcwRef2.waitForReferenceQueue(); // map.size() will only be updated after gc-ed objects are processed by the ReferenceQueue assertEquals(1, map.size()); - GCWatcher.fromClearedRef(ref1).ensureCollected(); + GCWatcher gcw1 = GCWatcher.fromClearedRef(ref1); + gcw1.ensureCollected(); + + gcw1.waitForReferenceQueue(); // map.size() will only be updated after gc-ed objects are processed by the ReferenceQueue assertTrue(map.toString(), map.isEmpty()); } @@ -750,12 +756,13 @@ public class ContainerUtilCollectionsTest extends Assert { Ref ref = Ref.create(new Object()); set.add(ref.get()); - GCWatcher.fromClearedRef(ref).ensureCollected(); + GCWatcher watcher = GCWatcher.fromClearedRef(ref); + watcher.ensureCollected(); + watcher.waitForReferenceQueue(); assertFalse(set.remove(this)); // to run processQueue assertTrue(set.isEmpty()); assertTrue(set.add(this)); // to run processQueues(); - //noinspection ConstantValue -- set contract is tested, not implied here assertFalse(set.isEmpty()); assertTrue(set.remove(this)); diff --git a/platform/util-ex/BUILD.bazel b/platform/util-ex/BUILD.bazel index dae994f622d2..3701a58617bb 100644 --- a/platform/util-ex/BUILD.bazel +++ b/platform/util-ex/BUILD.bazel @@ -22,6 +22,7 @@ jvm_library( "//platform/diagnostic", "//platform/util/coroutines", "//libraries/mvstore", + "//libraries/jbr", ] ) ### auto-generated section `build intellij.platform.util.ex` end \ No newline at end of file diff --git a/platform/util-ex/intellij.platform.util.ex.iml b/platform/util-ex/intellij.platform.util.ex.iml index 77a8a6f48502..c57d49d918ad 100644 --- a/platform/util-ex/intellij.platform.util.ex.iml +++ b/platform/util-ex/intellij.platform.util.ex.iml @@ -22,5 +22,6 @@ + \ No newline at end of file diff --git a/platform/util/src/com/intellij/util/ref/GCWatcher.java b/platform/util-ex/src/com/intellij/util/ref/GCWatcher.java similarity index 52% rename from platform/util/src/com/intellij/util/ref/GCWatcher.java rename to platform/util-ex/src/com/intellij/util/ref/GCWatcher.java index faf4eb6e4cb0..d5769219c79e 100644 --- a/platform/util/src/com/intellij/util/ref/GCWatcher.java +++ b/platform/util-ex/src/com/intellij/util/ref/GCWatcher.java @@ -6,7 +6,9 @@ import com.intellij.openapi.util.LowMemoryWatcher; import com.intellij.openapi.util.Ref; import com.intellij.reference.SoftReference; import com.intellij.util.MemoryDumpHelper; +import com.intellij.util.TimeoutUtil; import com.intellij.util.containers.ContainerUtil; +import com.jetbrains.JBR; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.Contract; import org.jetbrains.annotations.NotNull; @@ -22,6 +24,7 @@ import java.util.Collection; import java.util.Collections; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.locks.LockSupport; /** * A utility to garbage-collect specified objects in tests. Create a GCWatcher using {@link #tracking} or {@link #fromClearedRef} @@ -32,8 +35,10 @@ import java.util.concurrent.ConcurrentHashMap; @TestOnly @ApiStatus.Internal public final class GCWatcher { + private static final int NO_TIMEOUT = Integer.MAX_VALUE; private final ReferenceQueue myQueue = new ReferenceQueue<>(); private final Set> myReferences = ConcurrentHashMap.newKeySet(); + private boolean generateHeapDump = true; private GCWatcher(@NotNull Collection objects) { for (Object o : objects) { @@ -106,10 +111,18 @@ public final class GCWatcher { return result; } + public void setGenerateHeapDump(boolean generateHeapDump) { + this.generateHeapDump = generateHeapDump; + } + private boolean isEverythingCollected() { + return ContainerUtil.and(myReferences, e -> e.refersTo(null)); + } + + private void removeQueuedObjects() { while (true) { Reference ref = myQueue.poll(); - if (ref == null) return myReferences.isEmpty(); + if (ref == null) return; boolean removed = myReferences.remove(ref); assert removed; @@ -117,49 +130,128 @@ public final class GCWatcher { } public boolean tryCollect(int timeoutMs) { - return LowMemoryWatcher.runWithNotificationsSuppressed(() -> { - long startTime = System.currentTimeMillis(); + return tryCollect(new StringBuilder(), timeoutMs, EmptyRunnable.getInstance()); + } + + private boolean tryCollect(StringBuilder log, long timeoutDeadline, @NotNull Runnable runWhileWaiting) { + if (JBR.isSystemUtilsSupported()) { + tryCollectOnJbr(); + } + else { + tryCollectNoJbr(log, timeoutDeadline, runWhileWaiting); + } + + return isEverythingCollected(); + } + + @SuppressWarnings("MethodMayBeStatic") + private void tryCollectOnJbr() { + assert JBR.isSystemUtilsSupported() : "JBR system utils are not supported"; + JBR.getSystemUtils().fullGC(); // JBR.fullGC also collects soft references + } + + private void tryCollectNoJbr(StringBuilder log, long timeoutDeadline, @NotNull Runnable runWhileWaiting) { + LowMemoryWatcher.runWithNotificationsSuppressed(() -> { try { - GCUtil.allocateTonsOfMemory(new StringBuilder(), EmptyRunnable.getInstance(), - () -> isEverythingCollected() || System.currentTimeMillis() - startTime > timeoutMs); - } catch (OutOfMemoryError e) { + GCUtil.allocateTonsOfMemory(log, runWhileWaiting, + () -> isEverythingCollected() || System.currentTimeMillis() < timeoutDeadline); + } + catch (OutOfMemoryError e) { // IDEA-310426 // Ignore possible OOME that can raise during GC forcing // It's already been logged within GCUtil in case it appears. } - return isEverythingCollected(); + return null; }); } /** - * Attempt to run garbage collector repeatedly until all the objects passed when creating this GCWatcher are GC-ed. If that's impossible, - * this method gives up after some time and throws {@link IllegalStateException}. + * When runs on JBR, invokes Full GC synchronously and guarantees that soft references are also collected. + * Otherwise, attempts to run garbage collector repeatedly until all the objects passed when creating this GCWatcher are GC-ed or + * some timeout elapsed. + *

+ * If passed objects are not collected after all the operations described above, this method throws {@link IllegalStateException}. + *

+ * @see #waitForReferenceQueue() */ @TestOnly public void ensureCollected() throws IllegalStateException { - ensureCollected(EmptyRunnable.getInstance()); + StringBuilder log = new StringBuilder(); + boolean collected = tryCollect(log, NO_TIMEOUT, EmptyRunnable.getInstance()); + if (!collected) { + throwISE(log.toString()); + } } + /** - * Attempt to run garbage collector repeatedly until all the objects passed when creating this GCWatcher are GC-ed. If that's impossible, - * this method gives up after some time and throws {@link IllegalStateException}. + * Runs garbage collector repeatedly until all the objects passed when creating this GCWatcher are GC-ed or + * timeout elapsed. + *

+ * If passed objects are not collected after timeout, this method throws {@link IllegalStateException}. + * + * @see #waitForReferenceQueue() */ @TestOnly - public void ensureCollected(@NotNull Runnable runWhileWaiting) throws IllegalStateException { + public void ensureCollectedWithinTimeout(int timeoutMs) throws IllegalStateException { + ensureCollectedWithinTimeout(timeoutMs, EmptyRunnable.getInstance()); + } + + /** + * Runs garbage collector repeatedly until all the objects passed when creating this GCWatcher are GC-ed or + * timeout elapsed. + *

+ * If passed objects are not collected after timeout, this method throws {@link IllegalStateException}. + * + * @see #waitForReferenceQueue() + */ + @TestOnly + public void ensureCollectedWithinTimeout(int timeoutMs, @NotNull Runnable runWhileWaiting) throws IllegalStateException { StringBuilder log = new StringBuilder(); - if (GCUtil.allocateTonsOfMemory(log, runWhileWaiting, this::isEverythingCollected)) { - return; + long startTime = System.currentTimeMillis(); + long timeoutDeadline = startTime + timeoutMs; + boolean collected = tryCollect(log, timeoutMs, runWhileWaiting); + + while (!collected && System.currentTimeMillis() < timeoutDeadline) { + runWhileWaiting.run(); + TimeoutUtil.sleep(300); // let other threads to do some progress + collected = tryCollect(log, timeoutDeadline, runWhileWaiting); } + if (!collected) { + throwISE(log.toString()); + } + } + + /** + * Waits for GC-ed objects to be picked by the {@link ReferenceQueue}, which may or may not happen at the same time + * as objects are GC-ed (see javadoc for {@link WeakReference} class). + */ + @TestOnly + public void waitForReferenceQueue() { + ensureCollected(); + removeQueuedObjects(); + for (int i = 0; i < 10_000 && !myReferences.isEmpty(); i++) { + LockSupport.parkNanos(1_000_000); + removeQueuedObjects(); + } + if (!myReferences.isEmpty()) { + throwISE("Objects were collected, but still not placed to the ReferenceQueue. This might be a bug in the GCWatcher itself."); + } + } + + private void throwISE(String log) { String message = "Couldn't garbage-collect some objects, they might still be reachable from GC roots: " + ContainerUtil.mapNotNull(myReferences, SoftReference::dereference); try { - Path file = Paths.get(System.getProperty("teamcity.build.tempDir", System.getProperty("java.io.tmpdir")), "GCWatcher.hprof.zip"); - MemoryDumpHelper.captureMemoryDumpZipped(file); + if (generateHeapDump) { + Path file = Paths.get(System.getProperty("teamcity.build.tempDir", System.getProperty("java.io.tmpdir")), "GCWatcher.hprof.zip"); + MemoryDumpHelper.captureMemoryDumpZipped(file); - message += "\nMemory snapshot is available at " + file + "\n"; - //noinspection UseOfSystemOutOrSystemErr - System.out.println("##teamcity[publishArtifacts '" + file + "']"); + message += "\nMemory snapshot is available at " + file + "\n"; + //noinspection UseOfSystemOutOrSystemErr + System.out.println("##teamcity[publishArtifacts '" + file + "']"); + } } catch (Exception e) { throw new RuntimeException(e); diff --git a/platform/util/src/com/intellij/util/ref/GCUtil.java b/platform/util/src/com/intellij/util/ref/GCUtil.java index 17ea221b8df4..9ae697ed9088 100644 --- a/platform/util/src/com/intellij/util/ref/GCUtil.java +++ b/platform/util/src/com/intellij/util/ref/GCUtil.java @@ -50,7 +50,7 @@ public final class GCUtil { } @SuppressWarnings({"UseOfSystemOutOrSystemErr", "StringConcatenationInsideStringBufferAppend"}) - static boolean allocateTonsOfMemory(@NotNull StringBuilder log, @NotNull Runnable runWhileWaiting, @NotNull BooleanSupplier until) { + public static boolean allocateTonsOfMemory(@NotNull StringBuilder log, @NotNull Runnable runWhileWaiting, @NotNull BooleanSupplier until) { long freeMemory = Runtime.getRuntime().freeMemory(); log.append("Free memory: " + freeMemory + "\n"); diff --git a/platform/util/testSrc/com/intellij/util/ref/GCWatcherTest.java b/platform/util/testSrc/com/intellij/util/ref/GCWatcherTest.java new file mode 100644 index 000000000000..b1c29a974e35 --- /dev/null +++ b/platform/util/testSrc/com/intellij/util/ref/GCWatcherTest.java @@ -0,0 +1,78 @@ +// Copyright 2000-2025 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +package com.intellij.util.ref; + +import junit.framework.TestCase; +import org.junit.Test; + +import java.lang.ref.Reference; +import java.lang.ref.SoftReference; +import java.lang.ref.WeakReference; + +public class GCWatcherTest { + + public volatile Object o; + public volatile Reference ref; + + @Test + public void testWeakReferenceCollected() { + o = new Object(); + ref = new WeakReference<>(o); + GCWatcher watcher = GCWatcher.tracking(o); + o = null; + watcher.setGenerateHeapDump(false); + watcher.ensureCollected(); + } + + + @Test + public void testWeakReferenceNotCollected() { + o = new Object(); + ref = new WeakReference<>(o); + GCWatcher watcher = GCWatcher.tracking(o); + try { + watcher.setGenerateHeapDump(false); + watcher.ensureCollected(); + TestCase.assertNotNull("Wrong test setup: ref should not be GC-ed yet", ref.get()); + TestCase.fail("Should throw IllegalStateException"); + } + catch (IllegalStateException ignored) { + // expected + } + } + + @Test + public void testSoftReferenceCollected() { + o = new Object(); + ref = new SoftReference<>(o); + GCWatcher watcher = GCWatcher.tracking(o); + o = null; + watcher.setGenerateHeapDump(false); + watcher.ensureCollected(); + } + + @Test + public void testSoftReferenceNotCollected() { + o = new Object(); + ref = new SoftReference<>(o); + GCWatcher watcher = GCWatcher.tracking(o); + try { + watcher.setGenerateHeapDump(false); + watcher.ensureCollected(); + TestCase.assertNotNull("Wrong test setup: ref should not be GC-ed yet", ref.get()); + TestCase.fail("Should throw IllegalStateException"); + } + catch (IllegalStateException ignored) { + // expected + } + } + + @Test + public void testJBSoftReferenceCollected() { + o = new Object(); + ref = new com.intellij.reference.SoftReference<>(o); + GCWatcher watcher = GCWatcher.tracking(o); + o = null; + watcher.setGenerateHeapDump(false); + watcher.ensureCollected(); + } +}