[
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
>
