From 3bee63685ba90fa662af6d25a265b65d6402a0bf Mon Sep 17 00:00:00 2001 From: Alexey Kudravtsev Date: Fri, 22 Jun 2012 13:55:39 +0400 Subject: [PATCH] unexpected gc in between passes --- .../daemon/impl/PostHighlightingPass.java | 5 +- .../daemon/impl/RefCountHolder.java | 33 +++- .../daemon/impl/GeneralHighlightingPass.java | 165 ++++++++---------- ...efaultHighlightVisitorBasedInspection.java | 6 +- 4 files changed, 104 insertions(+), 105 deletions(-) diff --git a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/PostHighlightingPass.java b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/PostHighlightingPass.java index a875cfff4819..89a0bc7e0443 100644 --- a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/PostHighlightingPass.java +++ b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/PostHighlightingPass.java @@ -143,7 +143,7 @@ public class PostHighlightingPass extends TextEditorHighlightingPass { myInLibrary = fileIndex.isInLibraryClasses(virtualFile) || fileIndex.isInLibrarySource(virtualFile); myRefCountHolder = RefCountHolder.endUsing(myFile); - if (!myRefCountHolder.retrieveUnusedReferencesInfo(new Runnable() { + if (myRefCountHolder == null || !myRefCountHolder.retrieveUnusedReferencesInfo(new Runnable() { @Override public void run() { boolean errorFound = collectHighlights(elementSet, highlights, progress); @@ -650,8 +650,7 @@ public class PostHighlightingPass extends TextEditorHighlightingPass { final PsiClass containingClass = member.getContainingClass(); if (containingClass == null || !(containingClass instanceof PsiClassImpl)) return true; final PsiMethod valuesMethod = ((PsiClassImpl)containingClass).getValuesMethod(); - if (valuesMethod == null) return true; - return isMethodReferenced(valuesMethod, progress, helper); + return valuesMethod == null || isMethodReferenced(valuesMethod, progress, helper); } private static boolean canBeReferencedViaWeirdNames(PsiMember member) { diff --git a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/RefCountHolder.java b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/RefCountHolder.java index d0bc8c86a102..6f1260d77dd6 100644 --- a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/RefCountHolder.java +++ b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/RefCountHolder.java @@ -28,7 +28,9 @@ import com.intellij.psi.util.PsiUtil; import com.intellij.util.ArrayUtil; import com.intellij.util.containers.BidirectionalMap; import com.intellij.util.containers.ConcurrentHashMap; +import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import java.lang.ref.SoftReference; import java.util.Iterator; @@ -65,17 +67,17 @@ public class RefCountHolder { private void makeHardReachable(boolean isHard) { RefCountHolder holder = get(); - assert holder != null; + assert !isHard || holder != null : "hard: "+isHard +"; holder="+holder; myHardRef = isHard ? holder : null; } } private static final Key REF_COUNT_HOLDER_IN_FILE_KEY = Key.create("REF_COUNT_HOLDER_IN_FILE_KEY"); @NotNull - private static Pair getInstance(@NotNull PsiFile file) { + private static Pair getInstance(@NotNull PsiFile file, boolean create) { HolderReference ref = file.getUserData(REF_COUNT_HOLDER_IN_FILE_KEY); RefCountHolder holder = ref == null ? null : ref.get(); - if (holder == null) { + if (holder == null && create) { holder = new RefCountHolder(file); HolderReference newRef = new HolderReference(holder); while (true) { @@ -97,21 +99,26 @@ public class RefCountHolder { @NotNull public static RefCountHolder startUsing(@NotNull PsiFile file) { - Pair pair = getInstance(file); + Pair pair = getInstance(file, true); HolderReference reference = pair.second; reference.makeHardReachable(true); // make sure RefCountHolder won't be gced during highlighting + log("startUsing: " + pair.first.myState+" for "+file); return pair.first; } - @NotNull + + @Nullable("might be gced") public static RefCountHolder endUsing(@NotNull PsiFile file) { - Pair pair = getInstance(file); + Pair pair = getInstance(file, false); HolderReference reference = pair.second; reference.makeHardReachable(false); // no longer needed, can be cleared - return pair.first; + RefCountHolder holder = pair.first; + log("endUsing: " + (holder == null ? null : holder.myState)+" for "+file); + return holder; } private RefCountHolder(@NotNull PsiFile file) { myFile = file; + log("c: created: " + myState.get()+" for "+file); } private void clear() { @@ -267,10 +274,13 @@ public class RefCountHolder { } public boolean analyze(@NotNull PsiFile file, TextRange dirtyScope, @NotNull Runnable analyze) { + State old = myState.get(); myState.compareAndSet(State.READY, State.VIRGIN); if (!myState.compareAndSet(State.VIRGIN, State.BEING_WRITTEN_BY_GHP)) { + log("a: failed to change " + old + "->" + State.BEING_WRITTEN_BY_GHP); return false; } + log("a: changed " + old + "->" + State.BEING_WRITTEN_BY_GHP); boolean finished = false; try { if (dirtyScope != null) { @@ -288,20 +298,29 @@ public class RefCountHolder { finally { boolean set = myState.compareAndSet(State.BEING_WRITTEN_BY_GHP, finished ? State.READY : State.VIRGIN); assert set : myState.get(); + log("a: changed back " + State.BEING_WRITTEN_BY_GHP + "->" + (finished ? State.READY : State.VIRGIN)); } return true; } + private static void log(@NonNls String s) { + //System.err.println("RFC: "+s); + } + public boolean retrieveUnusedReferencesInfo(@NotNull Runnable analyze) { + State old = myState.get(); if (!myState.compareAndSet(State.READY, State.BEING_USED_BY_PHP)) { + log("r: failed to change " + old + "->" + State.BEING_USED_BY_PHP); return false; } + log("r: changed " + old + "->" + State.BEING_USED_BY_PHP); try { analyze.run(); } finally { boolean set = myState.compareAndSet(State.BEING_USED_BY_PHP, State.READY); assert set : myState.get(); + log("r: changed back " + State.BEING_USED_BY_PHP + "->" + State.READY); } return true; } diff --git a/platform/lang-impl/src/com/intellij/codeInsight/daemon/impl/GeneralHighlightingPass.java b/platform/lang-impl/src/com/intellij/codeInsight/daemon/impl/GeneralHighlightingPass.java index 9df986ee40cf..0ca02cd407d6 100644 --- a/platform/lang-impl/src/com/intellij/codeInsight/daemon/impl/GeneralHighlightingPass.java +++ b/platform/lang-impl/src/com/intellij/codeInsight/daemon/impl/GeneralHighlightingPass.java @@ -84,6 +84,7 @@ public class GeneralHighlightingPass extends ProgressableTextEditorHighlightingP private static final Logger LOG = Logger.getInstance("#com.intellij.codeInsight.daemon.impl.GeneralHighlightingPass"); static final String PRESENTABLE_NAME = DaemonBundle.message("pass.syntax"); private static final Key HAS_ERROR_ELEMENT = Key.create("HAS_ERROR_ELEMENT"); + private static final JobLauncher JobUtil = JobLauncher.getInstance(); private final int myStartOffset; private final int myEndOffset; @@ -101,9 +102,8 @@ public class GeneralHighlightingPass extends ProgressableTextEditorHighlightingP return o1.order() - o2.order(); } }; - private Runnable myApplyCommand; + private volatile Runnable myApplyCommand; private final EditorColorsScheme myGlobalScheme; - private boolean myFailFastOnAcquireReadAction = true; public GeneralHighlightingPass(@NotNull Project project, @NotNull PsiFile file, @@ -320,15 +320,17 @@ public class GeneralHighlightingPass extends ProgressableTextEditorHighlightingP } } }; - if (!JobLauncher.getInstance().invokeConcurrentlyUnderProgress(new ArrayList(hosts), progress, false, - new Processor() { - @Override - public boolean process(PsiElement element) { - progress.checkCanceled(); - InjectedLanguageFacadeImpl.enumerate(element, myFile, false, visitor); - return true; - } - })) throw new ProcessCanceledException(); + if (!JobUtil.invokeConcurrentlyUnderProgress(new ArrayList(hosts), progress, false, + new Processor() { + @Override + public boolean process(PsiElement element) { + progress.checkCanceled(); + InjectedLanguageFacadeImpl.enumerate(element, myFile, false, visitor); + return true; + } + })) { + throw new ProcessCanceledException(); + } } // returns false if canceled @@ -339,86 +341,65 @@ public class GeneralHighlightingPass extends ProgressableTextEditorHighlightingP final InjectedLanguageManager injectedLanguageManager = InjectedLanguageManager.getInstance(myProject); final TextAttributes injectedAttributes = myGlobalScheme.getAttributes(EditorColors.INJECTED_LANGUAGE_FRAGMENT); - return JobLauncher.getInstance().invokeConcurrentlyUnderProgress(new ArrayList(injectedFiles), progress, - myFailFastOnAcquireReadAction, - new Processor() { - @Override - public boolean process(final PsiFile injectedPsi) { - DocumentWindow documentWindow = - (DocumentWindow)PsiDocumentManager.getInstance(myProject) - .getCachedDocument(injectedPsi); - if (documentWindow == null) return true; - Place places = InjectedLanguageFacadeImpl.getShreds(injectedPsi); - for (PsiLanguageInjectionHost.Shred place : places) { - TextRange textRange = place.getRangeInsideHost() - .shiftRight(place.getHost().getTextRange().getStartOffset()); - if (textRange.isEmpty()) continue; - String desc = - injectedPsi.getLanguage().getDisplayName() + - ": " + - injectedPsi.getText(); - HighlightInfo info = HighlightInfo - .createHighlightInfo( - HighlightInfoType.INJECTED_LANGUAGE_BACKGROUND, - textRange, null, desc, injectedAttributes); - info.fromInjection = true; - outInfos.add(info); - } + return JobUtil.invokeConcurrentlyUnderProgress(new ArrayList(injectedFiles), progress, isFailFastOnAcquireReadAction(), + new Processor() { + @Override + public boolean process(final PsiFile injectedPsi) { + DocumentWindow documentWindow = (DocumentWindow)PsiDocumentManager.getInstance(myProject).getCachedDocument(injectedPsi); + if (documentWindow == null) return true; + Place places = InjectedLanguageFacadeImpl.getShreds(injectedPsi); + for (PsiLanguageInjectionHost.Shred place : places) { + TextRange textRange = place.getRangeInsideHost().shiftRight(place.getHost().getTextRange().getStartOffset()); + if (textRange.isEmpty()) continue; + String desc = injectedPsi.getLanguage().getDisplayName() + ": " + injectedPsi.getText(); + HighlightInfo info = HighlightInfo.createHighlightInfo(HighlightInfoType.INJECTED_LANGUAGE_BACKGROUND, + textRange, null, desc, injectedAttributes); + info.fromInjection = true; + outInfos.add(info); + } - HighlightInfoHolder holder = createInfoHolder(injectedPsi); - runHighlightVisitorsForInjected(injectedPsi, holder, progress); - for (int i = 0; i < holder.size(); i++) { - HighlightInfo info = holder.get(i); - final int startOffset = - documentWindow.injectedToHost(info.startOffset); - final TextRange fixedTextRange = - getFixedTextRange(documentWindow, startOffset); - addPatchedInfos(info, injectedPsi, documentWindow, - injectedLanguageManager, - fixedTextRange, outInfos); - } - holder.clear(); - highlightInjectedSyntax(injectedPsi, holder); - for (int i = 0; i < holder.size(); i++) { - HighlightInfo info = holder.get(i); - final int startOffset = info.startOffset; - final TextRange fixedTextRange = - getFixedTextRange(documentWindow, startOffset); - if (fixedTextRange == null) { - info.fromInjection = true; - outInfos.add(info); - } - else { - HighlightInfo patched = - new HighlightInfo(info.forcedTextAttributes, - info.forcedTextAttributesKey, - info.type, - fixedTextRange.getStartOffset(), - fixedTextRange.getEndOffset(), - info.description, info.toolTip, - info.type.getSeverity(null), - info.isAfterEndOfLine, null, - false); - patched.fromInjection = true; - outInfos.add(patched); - } - } + HighlightInfoHolder holder = createInfoHolder(injectedPsi); + runHighlightVisitorsForInjected(injectedPsi, holder, progress); + for (int i = 0; i < holder.size(); i++) { + HighlightInfo info = holder.get(i); + final int startOffset = documentWindow.injectedToHost(info.startOffset); + final TextRange fixedTextRange = getFixedTextRange(documentWindow, startOffset); + addPatchedInfos(info, injectedPsi, documentWindow, injectedLanguageManager, fixedTextRange, outInfos); + } + holder.clear(); + highlightInjectedSyntax(injectedPsi, holder); + for (int i = 0; i < holder.size(); i++) { + HighlightInfo info = holder.get(i); + final int startOffset = info.startOffset; + final TextRange fixedTextRange = getFixedTextRange(documentWindow, startOffset); + if (fixedTextRange == null) { + info.fromInjection = true; + outInfos.add(info); + } + else { + HighlightInfo patched = new HighlightInfo(info.forcedTextAttributes, info.forcedTextAttributesKey, + info.type, fixedTextRange.getStartOffset(), fixedTextRange.getEndOffset(), + info.description, info.toolTip, info.type.getSeverity(null), + info.isAfterEndOfLine, null, false); + patched.fromInjection = true; + outInfos.add(patched); + } + } - if (!isDumbMode()) { - List todos = new ArrayList(); - highlightTodos(injectedPsi, injectedPsi.getText(), 0, - injectedPsi.getTextLength(), progress, - myPriorityRange, todos, - todos); - for (HighlightInfo info : todos) { - addPatchedInfos(info, injectedPsi, documentWindow, - injectedLanguageManager, - null, outInfos); - } - } - return true; - } - }); + if (!isDumbMode()) { + List todos = new ArrayList(); + highlightTodos(injectedPsi, injectedPsi.getText(), 0, injectedPsi.getTextLength(), progress, myPriorityRange, todos, todos); + for (HighlightInfo info : todos) { + addPatchedInfos(info, injectedPsi, documentWindow, injectedLanguageManager, null, outInfos); + } + } + return true; + } + }); + } + + protected boolean isFailFastOnAcquireReadAction() { + return true; } private static TextRange getFixedTextRange(@NotNull DocumentWindow documentWindow, int startOffset) { @@ -789,7 +770,7 @@ public class GeneralHighlightingPass extends ProgressableTextEditorHighlightingP return visitorArray; } - static void cancelAndRestartDaemonLater(ProgressIndicator progress, final Project project, TextEditorHighlightingPass pass) { + static void cancelAndRestartDaemonLater(ProgressIndicator progress, final Project project, TextEditorHighlightingPass pass) throws ProcessCanceledException { PassExecutorService.log(progress, pass, "Cancel and restart"); progress.cancel(); ApplicationManager.getApplication().invokeLater(new Runnable() { @@ -893,8 +874,4 @@ public class GeneralHighlightingPass extends ProgressableTextEditorHighlightingP public String toString() { return super.toString() + " updateAll="+myUpdateAll+" range=("+myStartOffset+","+myEndOffset+")"; } - - public void setFailFastOnAcquireReadAction(boolean failFastOnAcquireReadAction) { - myFailFastOnAcquireReadAction = failFastOnAcquireReadAction; - } } diff --git a/platform/lang-impl/src/com/intellij/codeInspection/DefaultHighlightVisitorBasedInspection.java b/platform/lang-impl/src/com/intellij/codeInspection/DefaultHighlightVisitorBasedInspection.java index c7debd891dca..733d5385feb3 100644 --- a/platform/lang-impl/src/com/intellij/codeInspection/DefaultHighlightVisitorBasedInspection.java +++ b/platform/lang-impl/src/com/intellij/codeInspection/DefaultHighlightVisitorBasedInspection.java @@ -192,8 +192,12 @@ public abstract class DefaultHighlightVisitorBasedInspection extends GlobalSimpl @NotNull ProgressIndicator progress) { // do not mess with real editor highlights } + + @Override + protected boolean isFailFastOnAcquireReadAction() { + return false; + } }; - pass.setFailFastOnAcquireReadAction(false); DaemonProgressIndicator progress = new DaemonProgressIndicator(); progress.start(); pass.collectInformation(progress);