From a494959a11181d23c350ab2cedefb955c15f899e Mon Sep 17 00:00:00 2001 From: peter Date: Fri, 30 May 2014 21:30:11 +0200 Subject: [PATCH] WeakReference objects used for vfile->document mapping should not stay in memory forever --- .../mock/MockFileDocumentManagerImpl.java | 10 +++------ .../impl/FileDocumentManagerImpl.java | 22 ++++++++----------- .../psi/PsiDocumentManagerImplTest.java | 8 +++---- .../testFramework/ParsingTestCase.java | 2 +- 4 files changed, 16 insertions(+), 26 deletions(-) diff --git a/platform/core-impl/src/com/intellij/mock/MockFileDocumentManagerImpl.java b/platform/core-impl/src/com/intellij/mock/MockFileDocumentManagerImpl.java index cae738a775b3..ffaba2b21b10 100644 --- a/platform/core-impl/src/com/intellij/mock/MockFileDocumentManagerImpl.java +++ b/platform/core-impl/src/com/intellij/mock/MockFileDocumentManagerImpl.java @@ -23,20 +23,17 @@ import com.intellij.openapi.fileTypes.FileType; import com.intellij.openapi.project.Project; import com.intellij.openapi.util.Key; import com.intellij.openapi.vfs.VirtualFile; -import com.intellij.reference.SoftReference; import com.intellij.util.Function; import com.intellij.util.containers.WeakFactoryMap; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import java.lang.ref.Reference; - public class MockFileDocumentManagerImpl extends FileDocumentManager { private static final Key MOCK_VIRTUAL_FILE_KEY = Key.create("MockVirtualFile"); private final Function myFactory; - @Nullable private final Key> myCachedDocumentKey; + @Nullable private final Key myCachedDocumentKey; - public MockFileDocumentManagerImpl(Function factory, @Nullable Key> cachedDocumentKey) { + public MockFileDocumentManagerImpl(Function factory, @Nullable Key cachedDocumentKey) { myFactory = factory; myCachedDocumentKey = cachedDocumentKey; } @@ -66,8 +63,7 @@ public class MockFileDocumentManagerImpl extends FileDocumentManager { @Override public Document getCachedDocument(@NotNull VirtualFile file) { if (myCachedDocumentKey != null) { - Reference reference = file.getUserData(myCachedDocumentKey); - return SoftReference.dereference(reference); + return file.getUserData(myCachedDocumentKey); } return null; } diff --git a/platform/platform-impl/src/com/intellij/openapi/fileEditor/impl/FileDocumentManagerImpl.java b/platform/platform-impl/src/com/intellij/openapi/fileEditor/impl/FileDocumentManagerImpl.java index 89521379cc29..5def1e1fc161 100644 --- a/platform/platform-impl/src/com/intellij/openapi/fileEditor/impl/FileDocumentManagerImpl.java +++ b/platform/platform-impl/src/com/intellij/openapi/fileEditor/impl/FileDocumentManagerImpl.java @@ -67,6 +67,7 @@ import com.intellij.util.Function; import com.intellij.util.PairProcessor; import com.intellij.util.ThrowableRunnable; import com.intellij.util.containers.ConcurrentHashSet; +import com.intellij.util.containers.ConcurrentWeakValueHashMap; import com.intellij.util.messages.MessageBus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -76,9 +77,6 @@ import javax.swing.*; import java.awt.*; import java.awt.event.ActionEvent; import java.io.IOException; -import java.lang.ref.Reference; -import java.lang.ref.SoftReference; -import java.lang.ref.WeakReference; import java.lang.reflect.InvocationHandler; import java.lang.reflect.Method; import java.lang.reflect.Proxy; @@ -90,11 +88,12 @@ public class FileDocumentManagerImpl extends FileDocumentManager implements Virt private static final Logger LOG = Logger.getInstance("#com.intellij.openapi.fileEditor.impl.FileDocumentManagerImpl"); private static final Key LINE_SEPARATOR_KEY = Key.create("LINE_SEPARATOR_KEY"); - public static final Key> DOCUMENT_KEY = Key.create("DOCUMENT_KEY"); + public static final Key HARD_REF_TO_DOCUMENT_KEY = Key.create("HARD_REF_TO_DOCUMENT_KEY"); private static final Key FILE_KEY = Key.create("FILE_KEY"); private static final Key MUST_RECOMPUTE_FILE_TYPE = Key.create("Must recompute file type"); private final Set myUnsavedDocuments = new ConcurrentHashSet(); + private final Map myDocuments = new ConcurrentWeakValueHashMap(); private final MessageBus myBus; @@ -174,7 +173,7 @@ public class FileDocumentManagerImpl extends FileDocumentManager implements Virt document.setModificationStamp(file.getModificationStamp()); final FileType fileType = file.getFileType(); document.setReadOnly(!file.isWritable() || fileType.isBinary()); - file.putUserData(DOCUMENT_KEY, new WeakReference(document)); + myDocuments.put(file, document); document.putUserData(FILE_KEY, file); if (!(file instanceof LightVirtualFile || file.getFileSystem() instanceof DummyFileSystem)) { @@ -223,17 +222,13 @@ public class FileDocumentManagerImpl extends FileDocumentManager implements Virt @Override @Nullable public Document getCachedDocument(@NotNull VirtualFile file) { - return com.intellij.reference.SoftReference.dereference(file.getUserData(DOCUMENT_KEY)); + Document hard = file.getUserData(HARD_REF_TO_DOCUMENT_KEY); + return hard != null ? hard : myDocuments.get(file); } public static void registerDocument(@NotNull final Document document, @NotNull VirtualFile virtualFile) { synchronized (lock) { - virtualFile.putUserData(DOCUMENT_KEY, new SoftReference(document) { - @Override - public Document get() { - return document; - } - }); + virtualFile.putUserData(HARD_REF_TO_DOCUMENT_KEY, document); document.putUserData(FILE_KEY, virtualFile); } } @@ -550,7 +545,8 @@ public class FileDocumentManagerImpl extends FileDocumentManager implements Virt if (document != null) { // a file is linked to a document - chances are it is an "unknown text file" now if (isBinaryWithoutDecompiler(file)) { - file.putUserData(DOCUMENT_KEY, null); + myDocuments.remove(file); + file.putUserData(HARD_REF_TO_DOCUMENT_KEY, null); document.putUserData(FILE_KEY, null); } } diff --git a/platform/platform-tests/testSrc/com/intellij/psi/PsiDocumentManagerImplTest.java b/platform/platform-tests/testSrc/com/intellij/psi/PsiDocumentManagerImplTest.java index 6ccb0581ae96..718ee8ebe5f7 100644 --- a/platform/platform-tests/testSrc/com/intellij/psi/PsiDocumentManagerImplTest.java +++ b/platform/platform-tests/testSrc/com/intellij/psi/PsiDocumentManagerImplTest.java @@ -23,7 +23,6 @@ import com.intellij.openapi.editor.Document; import com.intellij.openapi.editor.impl.DocumentImpl; import com.intellij.openapi.editor.impl.event.DocumentEventImpl; import com.intellij.openapi.fileEditor.FileDocumentManager; -import com.intellij.openapi.fileEditor.impl.FileDocumentManagerImpl; import com.intellij.openapi.project.Project; import com.intellij.openapi.project.ex.ProjectManagerEx; import com.intellij.openapi.vfs.LocalFileSystem; @@ -34,12 +33,12 @@ import com.intellij.psi.impl.source.PsiFileImpl; import com.intellij.testFramework.LeakHunter; import com.intellij.testFramework.LightVirtualFile; import com.intellij.testFramework.PlatformLangTestCase; +import com.intellij.testFramework.PlatformTestUtil; import com.intellij.util.Processor; import com.intellij.util.concurrency.Semaphore; import com.intellij.util.ui.UIUtil; import java.io.File; -import java.lang.ref.Reference; import java.util.concurrent.atomic.AtomicInteger; public class PsiDocumentManagerImplTest extends PlatformLangTestCase { @@ -94,11 +93,10 @@ public class PsiDocumentManagerImplTest extends PlatformLangTestCase { }); //Class.forName("com.intellij.util.ProfilingUtil").getDeclaredMethod("forceCaptureMemorySnapshot").invoke(null); - Reference reference = vFile.getUserData(FileDocumentManagerImpl.DOCUMENT_KEY); - assertNotNull(reference); for (int i=0;i<1000;i++) { + PlatformTestUtil.tryGcSoftlyReachableObjects(); UIUtil.dispatchAllInvocationEvents(); - if (reference.get() == null) break; + if (documentManager.getCachedDocument(getPsiManager().findFile(vFile)) == null) break; System.gc(); } assertNull(documentManager.getCachedDocument(getPsiManager().findFile(vFile))); diff --git a/platform/testFramework/src/com/intellij/testFramework/ParsingTestCase.java b/platform/testFramework/src/com/intellij/testFramework/ParsingTestCase.java index c76c0bc0c3b5..f000733e8881 100644 --- a/platform/testFramework/src/com/intellij/testFramework/ParsingTestCase.java +++ b/platform/testFramework/src/com/intellij/testFramework/ParsingTestCase.java @@ -111,7 +111,7 @@ public abstract class ParsingTestCase extends PlatformLiteFixture { public Document fun(CharSequence charSequence) { return editorFactory.createDocument(charSequence); } - }, FileDocumentManagerImpl.DOCUMENT_KEY)); + }, FileDocumentManagerImpl.HARD_REF_TO_DOCUMENT_KEY)); registerComponentInstance(appContainer, PsiDocumentManager.class, new MockPsiDocumentManager()); myLanguage = myLanguage == null && myDefinitions.length > 0? myDefinitions[0].getFileNodeType().getLanguage() : myLanguage; registerComponentInstance(appContainer, FileTypeManager.class, new MockFileTypeManager(new MockLanguageFileType(myLanguage, myFileExt)));