Bug 180061

The PHP regression appeared on Jan 28th (still present), much less visible in Ruby, there are also regressions in CSS and JS (expanding folder with 100 files) - but started later (since Feb 15th).

I guess I know where the regression comes from. If I look at locally generated snapshot I see that bunch of time is spend in 
javax.swing.RepaintManager.paintDirtyRegions()
which then end up in 
org.openide.explorer.view.TreeView$ExplorerTree.guardedPaint(). Something must be redrawing content of explorer.

The regression appeared on Jan 28, 2010. The previous good build (which I can find) is from Jan 12, 2010. There has been one interesting integration meanwhile: http://hg.netbeans.org/main-golden/rev/96f5c992fe4c

Prior to this change the error badges were enabled only in java projects, not in php, ruby, etc. The fact that this regression is visible in php, ruby and not that much in Java, is another indicator, that this is the reason for the big variance in test results.

I am CCing Jan Lahoda. If the above hypothesis is true, Jan will be able to help us. Either by speeding the badges or by disabling them during test run.

Created attachment 95553 [details]
Stacktraces that cause the ErrorAnnotator to stack background thread and request repaint

The processing of error badges seems to be always done on background. This is not really fast, as that always requires redrawing of already drawn explorer views.

My expectation is that (for files in open projects for example), the values will be preloaded and returned immediately. Only if a file not in cache is found, then the processing starts on background, fills the cache, delivers file change event.

Moreover file status events shall not be delivered for files which were not in the cache, but are still without errors (it seems to me that events are fired all the time after the worker thread finishes computation).

(In reply to comment #4)
> The processing of error badges seems to be always done on background. This is
> not really fast, as that always requires redrawing of already drawn explorer
> views.
> 
> My expectation is that (for files in open projects for example), the values
> will be preloaded and returned immediately. Only if a file not in cache is
> found, then the processing starts on background, fills the cache, delivers file
> change event.

Sorry, I do not understand this proposal. Could you please described in more detail what you expect?

Currently, there are two caches: a persistent cache and in-memory cache. The persistent cache keeps all errors for all known files, as filled by the indexers. The in-memory cache holds the last known state for files for which the status was requested at least once. For each file, the persistent cache should be queried only a few times (normally only once), the state is then cached in the in-memory cache until the state is known to change or until the corresponding FileObject is garbage collected. What exactly do you propose?

> 
> Moreover file status events shall not be delivered for files which were not in
> the cache, but are still without errors (it seems to me that events are fired
> all the time after the worker thread finishes computation).

Ok, I will try to change this.

(In reply to comment #5)
> (In reply to comment #4)
> > The processing of error badges seems to be always done on background. This is
> > not really fast, as that always requires redrawing of already drawn explorer
> > views.
> > 
> > My expectation is that (for files in open projects for example), the values
> > will be preloaded and returned immediately. Only if a file not in cache is
> > found, then the processing starts on background, fills the cache, delivers file
> > change event.
> 
> Sorry, I do not understand this proposal. Could you please described in more
> detail what you expect?
> 
> Currently, there are two caches: a persistent cache and in-memory cache. The
> persistent cache keeps all errors for all known files, as filled by the
> indexers. The in-memory cache holds the last known state for files for which
> the status was requested at least once. For each file, the persistent cache
> should be queried only a few times (normally only once), the state is then

Only once now.

> cached in the in-memory cache until the state is known to change or until the
> corresponding FileObject is garbage collected. What exactly do you propose?
> 
> > 
> > Moreover file status events shall not be delivered for files which were not in
> > the cache, but are still without errors (it seems to me that events are fired
> > all the time after the worker thread finishes computation).
> 
> Ok, I will try to change this.

http://hg.netbeans.org/jet-main/rev/898e2f6cfa3d

Thanks for the quick fix. Delivering the status event only if there is a change in the state is definitely improvement and we will make sure our tests use it (e.g. the files in test are without errors).

Additionally I was proposing (in the new terminology) to prepopulate the in-memory cache on suitable event. Indeed, it remains to be determined what is suitable event.

My original thought was that opening a project could be such event. When a project is opened preload status for all its files (up to certain limit). The other idea would be to load the information for whole source root, as soon as one of its files is queried.

(In reply to comment #7)
> Thanks for the quick fix. Delivering the status event only if there is a change
> in the state is definitely improvement and we will make sure our tests use it
> (e.g. the files in test are without errors).
> 
> Additionally I was proposing (in the new terminology) to prepopulate the
> in-memory cache on suitable event. Indeed, it remains to be determined what is
> suitable event.

I still fail to see why the prepopulation would be useful to the users, but I assume you have some hard data about that (could you please share that data with me).

> 
> My original thought was that opening a project could be such event. When a
> project is opened preload status for all its files (up to certain limit). The
> other idea would be to load the information for whole source root, as soon as
> one of its files is queried.

I am attaching a patch that prepopulates the data when the project is opened (takes ~150ms for java.hints). Jarda, please verify whether this is what performance team wants. Not sure what "certain limit" is (neither why it should be defined?) would be - performance team should provide exact number (otherwise, I will choose 0). Unfortunately, it is not possible to just ignore files which are not mentioned in the error cache (otherwise files opened not belonging to any open project would not be correctly marked), and so it is necessary to always traverse all files in the given source root using FileObjects.

Created attachment 95591 [details]
Patch for "prepopulating" the in-memory cache.

Integrated into 'main-golden', will be available in build *201003240200* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/898e2f6cfa3d
User: Jan Lahoda <jlahoda@netbeans.org>
Log: #180061: do not fire file status changes when there is no change.

It is probably too early to evaluate trends, but recent test results

http://aventurine.russia.sun.com:8080/PerformanceDashboard/atomicResult.jsp?suiteName=Actions&resultUnit=ms&resultName=testExpandFolderWith100PhpFiles&resultThreshold=1000&resultClassname=org.netbeans.performance.languages.actions.ScriptingExpandFolderTest&history=30

seem to indicate that Jan's change reduced the repaints and as such it improved the test results. That means the priority of this problem can be decreased.

However we still considered an improvement to the error badge visualization, so it does not require repaints in most cases. Instead of relying on OpenProjects notifications the system would have some kind of in memory hierarchical cache. When queried for a status of a not yet known folder, the system would behave the same, a background thread would handle the operation. However it would not load just information about the single folder, but for (reasonably sized) subtree and kept this in memory. This information would contain list of errors in the subtree up to certain limit and information whether there is more errors or not. If another query for a file in subtree was made, it would consult this in-memory cache first in AWT thread and only if it was not ultimate (e.g. there could be more errors and this file was not among the known ones), it would switch to background and operate as it currently does.

The parsing.api could preload this in-memory cache for each known source root. Under the assumption that classes are mostly compilable (e.g. amount of error files in a root is less then ~50), this would eliminate the background computation and subsequent repaints all together.

To create such hierarchical cache one needs effective fileObject.getParent() which I reported as bug 182741. If Jan can implement the cache I am sure I can solve bug 182741 for 6.9.