[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [Dltk-dev] SourceParserUtil cache miss

It should be something like the following:

### Eclipse Workspace Patch 1.0
#P org.eclipse.dltk.core
Index: model/org/eclipse/dltk/internal/core/SourceModule.java
===================================================================
RCS file: /cvsroot/technology/org.eclipse.dltk/core/plugins/org.eclipse.dltk.core/model/org/eclipse/dltk/internal/core/SourceModule.java,v
retrieving revision 1.37
diff -u -r1.37 SourceModule.java
--- model/org/eclipse/dltk/internal/core/SourceModule.java	29 Nov 2011
14:26:10 -0000	1.37
+++ model/org/eclipse/dltk/internal/core/SourceModule.java	19 Apr 2012
11:44:20 -0000
@@ -8,6 +8,9 @@
  *******************************************************************************/
 package org.eclipse.dltk.internal.core;

+import java.util.HashSet;
+import java.util.Set;
+
 import org.eclipse.core.resources.IContainer;
 import org.eclipse.core.resources.IFile;
 import org.eclipse.core.resources.IResource;
@@ -21,7 +24,6 @@
 import org.eclipse.dltk.core.IProblemRequestor;
 import org.eclipse.dltk.core.IProjectFragment;
 import org.eclipse.dltk.core.ISourceModule;
-import org.eclipse.dltk.core.ISourceModuleInfoCache;
 import org.eclipse.dltk.core.ModelException;
 import org.eclipse.dltk.core.WorkingCopyOwner;
 import org.eclipse.dltk.internal.core.util.Messages;
@@ -236,17 +238,45 @@
 		return !isPrimary() || (getPerWorkingCopyInfo() != null);
 	}

+	private static final Set<SourceModule> locks = new HashSet<SourceModule>();
+
+	private boolean acquire() {
+		final long stop = System.currentTimeMillis() + 10000;
+		synchronized (locks) {
+			while (locks.contains(this)) {
+				final long now = System.currentTimeMillis();
+				if (now > stop) {
+					return false;
+				}
+				try {
+					locks.wait(stop - now);
+				} catch (InterruptedException e) {
+					return false;
+				}
+			}
+			locks.add(this);
+			return true;
+		}
+	}
+
+	private void release() {
+		synchronized (locks) {
+			locks.remove(this);
+			locks.notifyAll();
+		}
+	}
+
 	@Override
 	public void makeConsistent(IProgressMonitor monitor) throws ModelException {
-		if (isConsistent())
-			return;
-		// makeConsistent(false/*don't create AST*/, 0, monitor);
-
-		// Remove AST Cache element
-		ISourceModuleInfoCache sourceModuleInfoCache = ModelManager
-				.getModelManager().getSourceModuleInfoCache();
-		sourceModuleInfoCache.remove(this);
-		openWhenClosed(createElementInfo(), monitor);
+		if (acquire()) {
+			try {
+				if (isConsistent())
+					return;
+				openWhenClosed(createElementInfo(), monitor);
+			} finally {
+				release();
+			}
+		}
 	}

 	/*


On Thu, Apr 19, 2012 at 18:28, Johan Compagner <jcompagner@xxxxxxxxx> wrote:
> Yes that's true, i think that's what i am doing right now, 1 thread waits on
> the 2 one, if you have a better place for it that's fine by me
>
> On Apr 19, 2012 12:38 PM, "Alexey Panchenko" <alex.panchenko@xxxxxxxxx>
> wrote:
>>
>> @Jae
>> Double parsing on editor open was caused by the clearing the cache on
>> changing the source module to working copy.
>> I checked how it works and as cache is specially cleared during the
>> operations, so additional clear is not necessary.
>> I remove the corresponding check from the SourceModuleInfoCache and it
>> doesn't parse twice.
>>
>> @Johan
>> The double parsing in makeConsistent() is happening with a change
>> followed by a quick save, if I recall correctly.
>> I think we should add kind of synchronization there, so the second
>> thread just waits.
>> Do you agree on the approach?
>>
>> Regards,
>> Alex
>>
>> On Wed, Apr 18, 2012 at 21:46, Johan Compagner <jcompagner@xxxxxxxxx>
>> wrote:
>> > i haven't seen any side effects, and it is now already for quite some
>> > time
>> > in real end user releases
>> >
>> >
>> >
>> >
>> > On Wed, Apr 18, 2012 at 16:30, Jae Gangemi <jgangemi@xxxxxxxxx> wrote:
>> >>
>> >>
>> >> Â are there any other possible ways to address this?
>> >>
>> >> Â johan: at this point, have you seen any negative side effects?
>> >>
>> >> Â if no other solutions have been thought off, perhaps this could be
>> >> added
>> >> w/ a way to turn it on/off (perhaps via a language toolkit method)
>> >> until
>> >> something better comes along?
>> >>
>> >>
>> >> On Wed, Apr 18, 2012 at 10:22 AM, Alexey Panchenko
>> >> <alex.panchenko@xxxxxxxxx> wrote:
>> >>>
>> >>> I remember we discussed it and I wasn't sure it should be fixed this
>> >>> way.
>> >>>
>> >>> On Wed, Apr 18, 2012 at 21:04, Jae Gangemi <jgangemi@xxxxxxxxx> wrote:
>> >>> >
>> >>> > Â is there a reason you haven't pushed the patch upstream?
>> >>> >
>> >>> > Â i'd prefer not to be in the business of rolling my own core plugin
>> >>> > sets,
>> >>> > at least not yet, i've got enough on my plate w/ this thing as it
>> >>> > is.
>> >>> > :)
>> >>> >
>> >>> >
>> >>> > On Mon, Apr 16, 2012 at 10:24 AM, Johan Compagner
>> >>> > <jcompagner@xxxxxxxxx>
>> >>> > wrote:
>> >>> >>
>> >>> >> local, i have some patches to the dltk and we ship our own plugins
>> >>> >> that we
>> >>> >> build our self.
>> >>> >>
>> >>> >>
>> >>> >>
>> >>> >> On Mon, Apr 16, 2012 at 16:23, Jae Gangemi <jgangemi@xxxxxxxxx>
>> >>> >> wrote:
>> >>> >>>
>> >>> >>>
>> >>> >>> Â do you have this patched in a sub-class or are you just using a
>> >>> >>> local
>> >>> >>> patched version of the core?
>> >>> >>>
>> >>> >>> Â i'm just trying to figure out where i'd drop this in (although
>> >>> >>> it
>> >>> >>> does
>> >>> >>> seem like it's just a potential bug in general).
>> >>> >>>
>> >>> >>>
>> >>> >>> On Mon, Apr 16, 2012 at 10:18 AM, Johan Compagner
>> >>> >>> <jcompagner@xxxxxxxxx>
>> >>> >>> wrote:
>> >>> >>>>
>> >>> >>>> Not sure if this is related but this is my patch that i have in
>> >>> >>>> SourceModule
>> >>> >>>>
>> >>> >>>> ÂÂÂ public void makeConsistent(IProgressMonitor monitor) throws
>> >>> >>>> ModelException {
>> >>> >>>> ÂÂÂ ÂÂÂ // if (isConsistent())
>> >>> >>>> ÂÂÂ ÂÂÂ // return;
>> >>> >>>> ÂÂÂ ÂÂÂ // makeConsistent(false/*don't create AST*/, 0, monitor);
>> >>> >>>>
>> >>> >>>> ÂÂÂ ÂÂÂ HashSet<Openable> elementsOutOfSynchWithBuffers =
>> >>> >>>> ModelManager
>> >>> >>>>
>> >>> >>>> .getModelManager().getElementsOutOfSynchWithBuffers();
>> >>> >>>> ÂÂÂ ÂÂÂ synchronized (elementsOutOfSynchWithBuffers) {
>> >>> >>>> ÂÂÂ ÂÂÂ ÂÂÂ if (!elementsOutOfSynchWithBuffers.contains(this))
>> >>> >>>> ÂÂÂ ÂÂÂ ÂÂÂ ÂÂÂ return;
>> >>> >>>> ÂÂÂ ÂÂÂ ÂÂÂ elementsOutOfSynchWithBuffers.remove(this);
>> >>> >>>> ÂÂÂ ÂÂÂ }
>> >>> >>>>
>> >>> >>>> ÂÂÂ ÂÂÂ openWhenClosed(createElementInfo(), monitor);
>> >>> >>>> ÂÂÂ }
>> >>> >>>>
>> >>> >>>> the above method is what i use, because i also did see double
>> >>> >>>> parsing
>> >>> >>>> and so on.
>> >>> >>>>
>> >>> >>>> And we also use very large files so it was noticeable..
>> >>> >>>>
>> >>> >>>>
>> >>> >>>> On Sun, Apr 15, 2012 at 22:22, Jae Gangemi <jgangemi@xxxxxxxxx>
>> >>> >>>> wrote:
>> >>> >>>>>
>> >>> >>>>> hello all -
>> >>> >>>>>
>> >>> >>>>> Â i'm noticing some kind of ISourceModuleInfo cache miss
>> >>> >>>>> occuring
>> >>> >>>>> in
>> >>> >>>>> the SourceParserUtil.
>> >>> >>>>>
>> >>> >>>>> Â when i double click on a document to open in the editor, the
>> >>> >>>>> ISourceModuleInfo is requested (line 48 of SourceParserUtil) is
>> >>> >>>>> NOT
>> >>> >>>>> the same
>> >>> >>>>> object that is returned when SourceParserUtil gets invoked by
>> >>> >>>>> the
>> >>> >>>>> folding
>> >>> >>>>> structure code, but it is the same object when the Reconcile
>> >>> >>>>> thread
>> >>> >>>>> fires
>> >>> >>>>> next.
>> >>> >>>>>
>> >>> >>>>> Â the double parse is causing some slow downs on more complex
>> >>> >>>>> objects.
>> >>> >>>>>
>> >>> >>>>> Â i've tried stepping through the code numerous times trying to
>> >>> >>>>> track
>> >>> >>>>> down the issue, but i haven't been able to figure out why the
>> >>> >>>>> objects aren't
>> >>> >>>>> the same. hopefully someone on the list can help. :)
>> >>> >>>>>
>> >>> >>>>> --
>> >>> >>>>> -jae
>> >>> >>>>>
>> >>> >>>>> _______________________________________________
>> >>> >>>>> dltk-dev mailing list
>> >>> >>>>> dltk-dev@xxxxxxxxxxx
>> >>> >>>>> https://dev.eclipse.org/mailman/listinfo/dltk-dev
>> >>> >>>>>
>> >>> >>>>
>> >>> >>>>
>> >>> >>>> _______________________________________________
>> >>> >>>> dltk-dev mailing list
>> >>> >>>> dltk-dev@xxxxxxxxxxx
>> >>> >>>> https://dev.eclipse.org/mailman/listinfo/dltk-dev
>> >>> >>>>
>> >>> >>>
>> >>> >>>
>> >>> >>>
>> >>> >>> --
>> >>> >>> -jae
>> >>> >>>
>> >>> >>> _______________________________________________
>> >>> >>> dltk-dev mailing list
>> >>> >>> dltk-dev@xxxxxxxxxxx
>> >>> >>> https://dev.eclipse.org/mailman/listinfo/dltk-dev
>> >>> >>>
>> >>> >>
>> >>> >>
>> >>> >> _______________________________________________
>> >>> >> dltk-dev mailing list
>> >>> >> dltk-dev@xxxxxxxxxxx
>> >>> >> https://dev.eclipse.org/mailman/listinfo/dltk-dev
>> >>> >>
>> >>> >
>> >>> >
>> >>> >
>> >>> > --
>> >>> > -jae
>> >>> >
>> >>> > _______________________________________________
>> >>> > dltk-dev mailing list
>> >>> > dltk-dev@xxxxxxxxxxx
>> >>> > https://dev.eclipse.org/mailman/listinfo/dltk-dev
>> >>> >
>> >>> _______________________________________________
>> >>> dltk-dev mailing list
>> >>> dltk-dev@xxxxxxxxxxx
>> >>> https://dev.eclipse.org/mailman/listinfo/dltk-dev
>> >>
>> >>
>> >>
>> >>
>> >> --
>> >> -jae
>> >>
>> >> _______________________________________________
>> >> dltk-dev mailing list
>> >> dltk-dev@xxxxxxxxxxx
>> >> https://dev.eclipse.org/mailman/listinfo/dltk-dev
>> >>
>> >
>> >
>> > _______________________________________________
>> > dltk-dev mailing list
>> > dltk-dev@xxxxxxxxxxx
>> > https://dev.eclipse.org/mailman/listinfo/dltk-dev
>> >
>> _______________________________________________
>> dltk-dev mailing list
>> dltk-dev@xxxxxxxxxxx
>> https://dev.eclipse.org/mailman/listinfo/dltk-dev
>
>
> _______________________________________________
> dltk-dev mailing list
> dltk-dev@xxxxxxxxxxx
> https://dev.eclipse.org/mailman/listinfo/dltk-dev
>