From 95c72cb1e5cf3ed739a7f991cb884317d66f1718 Mon Sep 17 00:00:00 2001 From: peter Date: Tue, 26 Nov 2013 17:54:33 +0100 Subject: [PATCH] don't drop all caches on second completion invocation in the same place --- .../HeavyNormalCompletionTest.groovy | 8 +++ .../completion/CodeCompletionHandlerBase.java | 50 +++++++++++-------- 2 files changed, 36 insertions(+), 22 deletions(-) diff --git a/java/java-tests/testSrc/com/intellij/codeInsight/completion/HeavyNormalCompletionTest.groovy b/java/java-tests/testSrc/com/intellij/codeInsight/completion/HeavyNormalCompletionTest.groovy index 45f79b2ce770..b9620a18b0ab 100644 --- a/java/java-tests/testSrc/com/intellij/codeInsight/completion/HeavyNormalCompletionTest.groovy +++ b/java/java-tests/testSrc/com/intellij/codeInsight/completion/HeavyNormalCompletionTest.groovy @@ -20,6 +20,7 @@ import com.intellij.openapi.vfs.LocalFileSystem import com.intellij.openapi.vfs.VirtualFile import com.intellij.psi.JavaPsiFacade import com.intellij.psi.PsiClass +import com.intellij.psi.PsiManager import com.intellij.psi.search.GlobalSearchScope import com.intellij.psi.util.PsiTreeUtil import com.intellij.testFramework.PsiTestUtil @@ -141,5 +142,12 @@ public class Test { myFixture.assertPreferredCompletionItems 0, 'getBuilder' } + public void testNoJavaStructureModificationOnSecondInvocation() { + myFixture.configureByText 'a.java', 'class Foo { Xxxxx }' + def oldCount = PsiManager.getInstance(project).modificationTracker.javaStructureModificationCount + assert !myFixture.completeBasic() + assert !myFixture.completeBasic() + assert oldCount == PsiManager.getInstance(project).modificationTracker.javaStructureModificationCount + } } diff --git a/platform/lang-impl/src/com/intellij/codeInsight/completion/CodeCompletionHandlerBase.java b/platform/lang-impl/src/com/intellij/codeInsight/completion/CodeCompletionHandlerBase.java index c5c5d6fe1737..c4ff55b2972a 100644 --- a/platform/lang-impl/src/com/intellij/codeInsight/completion/CodeCompletionHandlerBase.java +++ b/platform/lang-impl/src/com/intellij/codeInsight/completion/CodeCompletionHandlerBase.java @@ -45,8 +45,8 @@ import com.intellij.openapi.project.IndexNotReadyException; import com.intellij.openapi.project.Project; import com.intellij.openapi.util.Disposer; import com.intellij.openapi.util.Key; -import com.intellij.openapi.util.Pair; import com.intellij.openapi.util.Ref; +import com.intellij.openapi.util.Trinity; import com.intellij.openapi.util.text.StringUtil; import com.intellij.openapi.vfs.VirtualFile; import com.intellij.psi.PsiDocumentManager; @@ -457,7 +457,7 @@ public class CodeCompletionHandlerBase { public void run() { AccessToken token = WriteAction.start(); try { - hostCopy[0] = createFileCopy(hostFile); + hostCopy[0] = createFileCopy(hostFile, initContext.getStartOffset(), initContext.getSelectionEndOffset()); } finally { token.finish(); @@ -748,7 +748,7 @@ public class CodeCompletionHandlerBase { } } - public static final Key>> FILE_COPY_KEY = Key.create("CompletionFileCopy"); + private static final Key>> FILE_COPY_KEY = Key.create("CompletionFileCopy"); private static boolean isCopyUpToDate(Document document, @NotNull PsiFile file) { if (!file.isValid()) { @@ -760,31 +760,37 @@ public class CodeCompletionHandlerBase { return current != null && current.getViewProvider().getPsi(file.getLanguage()) == file; } - private static PsiFile createFileCopy(PsiFile file) { + private static PsiFile createFileCopy(PsiFile file, long caret, long selEnd) { final VirtualFile virtualFile = file.getVirtualFile(); - if (file.isPhysical() && virtualFile != null && virtualFile.isInLocalFileSystem() - // must not cache injected file copy, since it does not reflect changes in host document - && !InjectedLanguageManager.getInstance(file.getProject()).isInjectedFragment(file)) { - final SoftReference> reference = file.getUserData(FILE_COPY_KEY); - if (reference != null) { - final Pair pair = reference.get(); - if (pair != null && pair.first.getClass().equals(file.getClass()) && isCopyUpToDate(pair.second, pair.first)) { - final PsiFile copy = pair.first; - if (copy.getViewProvider().getModificationStamp() > file.getViewProvider().getModificationStamp()) { - ((PsiModificationTrackerImpl) file.getManager().getModificationTracker()).incCounter(); - } - final Document document = pair.second; - assert document != null; - document.setText(file.getText()); - return copy; + boolean mayCacheCopy = file.isPhysical() && + // we don't want to cache code fragment copies even if they appear to be physical + virtualFile != null && virtualFile.isInLocalFileSystem(); + long combinedOffsets = caret + (selEnd << 32); + if (mayCacheCopy) { + final Trinity cached = SoftReference.dereference(file.getUserData(FILE_COPY_KEY)); + if (cached != null && cached.first.getClass().equals(file.getClass()) && isCopyUpToDate(cached.second, cached.first)) { + final PsiFile copy = cached.first; + if (copy.getViewProvider().getModificationStamp() > file.getViewProvider().getModificationStamp() && + cached.third.longValue() != combinedOffsets) { + // the copy PSI might have some caches that are not cleared on its modification because there are no events in the copy + // so, clear all the caches + // hopefully it's a rare situation that the user invokes completion in different parts of the file + // without modifying anything physical in between + ((PsiModificationTrackerImpl) file.getManager().getModificationTracker()).incCounter(); } + final Document document = cached.second; + assert document != null; + document.setText(file.getText()); + return copy; } } final PsiFile copy = (PsiFile)file.copy(); - final Document document = copy.getViewProvider().getDocument(); - assert document != null; - file.putUserData(FILE_COPY_KEY, new SoftReference>(Pair.create(copy, document))); + if (mayCacheCopy) { + final Document document = copy.getViewProvider().getDocument(); + assert document != null; + file.putUserData(FILE_COPY_KEY, new SoftReference>(Trinity.create(copy, document, combinedOffsets))); + } return copy; }