This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 112464 - Editor is redrawing more than it has to - SemanticHighlightsLayer fires change of whole document
Summary: Editor is redrawing more than it has to - SemanticHighlightsLayer fires chang...
Status: RESOLVED FIXED
Alias: None
Product: editor
Classification: Unclassified
Component: Painting & Printing (show other bugs)
Version: 6.x
Hardware: All Windows XP
: P3 blocker (vote)
Assignee: Jan Jancura
URL:
Keywords: PERFORMANCE
Depends on:
Blocks: 110302 111047
  Show dependency tree
 
Reported: 2007-08-09 15:04 UTC by Jan Jancura
Modified: 2008-08-20 08:03 UTC (History)
4 users (show)

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments
JS file (162.38 KB, text/plain)
2007-08-20 22:23 UTC, Jan Jancura
Details
The log of events from SemanticHighlightsLayer (3.83 KB, text/plain)
2007-08-22 16:48 UTC, Vitezslav Stejskal
Details
The stacktrace when AWT was busy (3.67 KB, text/plain)
2007-08-29 16:59 UTC, Vitezslav Stejskal
Details
The patch for measuring performance of LanguagesNavigator.refresh() (1.17 KB, text/plain)
2007-08-29 17:01 UTC, Vitezslav Stejskal
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Jancura 2007-08-09 15:04:55 UTC
1) add following sout to org.netbeans.modules.languages.features.SemanticHighlightsLayer:
    public HighlightsSequence getHighlights (int startOffset, int endOffset) {
        System.out.println("SemanticHighlightsLayer.getHighlights " + startOffset + " : " + endOffset);
        return new Highlights (document, highlighting, parserManager, startOffset, endOffset);
    }

2) try to open some big JavaScript file, wait on parser finish

3) change the file - 
SemanticHighlightsLayer fires change of whole document. AbstractHighlightsContainer should asks for highlights for
visible area only, but it asks for highlights for whole document.
Comment 1 Miloslav Metelka 2007-08-10 12:28:13 UTC
I will ask Vita since he was working on some performance tuning of the highlight layers already. Basically I agree with
your after invalidation the highlight layers cache in the infrastructure should fetch highlights from the HLs for the
visible area only.
Comment 2 Vitezslav Stejskal 2007-08-20 17:16:00 UTC
I'll have a closer look, but generally the highlighting infrastructure computes highlights for areas it is asked to by
DrawEngine/swing/whoever. So, I think the question here is why somebody tries to draw more then it needs to. Anyway,
Hanzi could you please attach the JavaScript file you were using to test this? Thanks.
Comment 3 Jan Jancura 2007-08-20 22:23:59 UTC
Created attachment 46916 [details]
JS file
Comment 4 Vitezslav Stejskal 2007-08-22 16:48:15 UTC
Created attachment 47088 [details]
The log of events from SemanticHighlightsLayer
Comment 5 Vitezslav Stejskal 2007-08-22 17:10:15 UTC
I added two logging statements in SemanticHighlightsLayer - one (###) in the getHighlights method that shows what
highlights is the layer asked to provide and another one (@@@) in SemanticHighlightsLayer.Listener that shows what
changes the layer reports to the highlighting infrastructure.

The attached log file shows what happened when typing a single character right at the beginning of the BigJavaScript.js
document. The document was open in the IDE, the IDE was restarted and all the initial startup activity was done.

The events fired from the layer always invalidate the whole layer (from 0 to doc.getLenght()), no surprise here. The
layer is usually asked for highlights in not so long region at the beginning of the document and at the very end of the
document. However, after the layer fires several (!) change events the text component finally decides to redraw the
whole document and asks for highlights from the whole document. This is shown on the stacktrace.

I'm still not sure where the problem is, but it seems to me that the highlighting infrastructure is doing what it is
supposed to do and asks layers only for highlights from areas that need to be repainted (this is determined by swing).
One question to ask is why the layer fires 6 change events when typing a single character? Although I am not sure if
fixing this would actually help.
Comment 6 Vitezslav Stejskal 2007-08-29 16:58:18 UTC
I think the problem is not the highlighting infrastructure or at least not the biggest problem. When typing a char the
editor response is instantaneous, but after a short while the whole IDE freezes for several seconds. The thread dump
shows that the AWT thread is busy in LanguagesNavigator task that was spawned from LanguagesNavigator.refresh(). I tried
to measure the time spent in this task and for the BigJavaScript.js file it took around 13-14 seconds to finish! For
that whole time the AWT and whole UI was blocked of course. I am going to attach the exact stacktrace and the patch I
used for measuring.
Comment 7 Vitezslav Stejskal 2007-08-29 16:59:49 UTC
Created attachment 47727 [details]
The stacktrace when AWT was busy
Comment 8 Vitezslav Stejskal 2007-08-29 17:01:47 UTC
Created attachment 47728 [details]
The patch for measuring performance of LanguagesNavigator.refresh()
Comment 9 Vitezslav Stejskal 2007-08-29 17:10:06 UTC
I also tried commenting out the code in LanguagesNavigator.refresh() and the editor started to perform normally, ie no
more freezes, editing and scrolling reasonably fast. IMO LanguagesNavigator definitely needs fixing.
Comment 10 Jan Jancura 2007-08-29 18:20:45 UTC
Yes, you are right. But we have separate issue for this bug already fired. This issue is about:

"SemanticHighlightsLayer fires change of whole document. AbstractHighlightsContainer should asks for highlights for
visible area only, but it asks for highlights for whole document."

So, feel free to fix it or wave it.
We can discuss priority of this issue too.
Comment 11 Vitezslav Stejskal 2007-08-30 09:41:03 UTC
Ok, my impression was that without LanguagesNavigator the performance of the editor was acceptable. If you can confirm
that then I'll make this P3. I agree that the editor's draw engine does not always work efficiently. It should have been
improved by the new view hierarchy, which unfortunately we didn't manage to implement in nb6.
Comment 12 Petr Hrebejk 2007-08-30 10:06:31 UTC
OK changing the description and lowering to P3
Comment 13 Marek Fukala 2007-08-31 12:30:43 UTC
As for the HL being called multiple times, there are two issues in Schlieman:
1) SemanticHighlightLayer.parsed method fires the HL change even when the parsing just started. The method should IMHO
fire only when parsed, or at least when the parser state != PARSING:

        public void parsed (State state, ASTNode root) {
+            if(state != State.PARSING) {
                fireHighlightsChange (0, document.getLength ());
+            }
        }

2) there are multiple instancies of SemanticHighlighLayer (I am seeing three) and each of them fires the events to the
HL which seems to be completely unnecessary.

+) when debugging the whole stuff I also found that there are more instancies of EditorParser (4) and each of them hold
different instance of j.t.s.Document !!! Two instancies are empty and two contains the real data.

I know that these issues are not related to HL, but since there was some discussion about it I am passing it here.
Hanzi, can you please comment on it?

Anyway, during my profiler session (I was testing what is going on in the IDE after a keystroke in editor) the AWT
thread was occupied for about 50% of time in this HL stuff. I belive that there are some bugs in S., but  anyway the HL
infrastructure should not ask all HL to get highlights for the whole document in ANY CASE. I consider this issue to be
of higher priority that P3, I'll try to profile it once again and if it confirms the last results, I'll increase it to P2.

Comment 14 Marek Fukala 2007-08-31 12:37:41 UTC
BTW, I filled the problem describe in previous comment as issue #114359 to languages/engine module.
Comment 15 Vitezslav Stejskal 2007-09-05 14:14:14 UTC
Please see also issue #114016. The optimization done in DrawEngineLineView should help to improve behavior describe in
this issue as well. There still may be situations when the infrastructure asks from more highlights than it should, but
this should be happening way to less frequently, because of the optimized modelToView and viewToModel translations.
Comment 16 Jiri Prox 2008-04-11 00:42:19 UTC
moving opened issues from TM <= 6.1 to TM=Dev
Comment 17 Jan Becicka 2008-08-15 14:20:05 UTC
Is this issue still valid?
Comment 18 Vitezslav Stejskal 2008-08-20 08:03:39 UTC
Well, strictly speaking it is and will be until we have a new view hierarchy (and replace DrawEngine), which will only
draw editor's visible area. On the other hand with all the other performance improvements I think painting in editor is
reasonably fast. At least I haven't heard any complaints recently. The new view hierarchy is requested in issue #121357,
so I think we can close this issue as fixed. If there are objections please feel free to reopen it.