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 185582 - Remove expensive calls from critical editor EDT loop
Summary: Remove expensive calls from critical editor EDT loop
Status: RESOLVED FIXED
Alias: None
Product: editor
Classification: Unclassified
Component: Navigation (show other bugs)
Version: 6.x
Hardware: PC Solaris
: P2 normal (vote)
Assignee: Vitezslav Stejskal
URL:
Keywords: PERFORMANCE
Depends on:
Blocks: 185466
  Show dependency tree
 
Reported: 2010-05-06 07:18 UTC by Alexander Simon
Modified: 2010-05-14 04:08 UTC (History)
1 user (show)

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments
Simplest caching patch (but the best patch is initializing fields in constructor or component shown methods) (3.96 KB, patch)
2010-05-06 07:18 UTC, Alexander Simon
Details | Diff
cumulative patch with caching + improved modelToView (6.79 KB, patch)
2010-05-06 13:13 UTC, Alexander Simon
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexander Simon 2010-05-06 07:18:32 UTC
Created attachment 98537 [details]
Simplest caching patch (but the best patch is initializing fields in constructor or component shown methods)

Test case:
- open large C/C++ file in editor.
- wait finished painting  of content and annotations.
- profile moving cursor by up/down arrows
Profiler show that editor eat 78% of CPU time in two expensive methods:
- 54% of CPU time consumes GapDocumentView in calls of SwingUtilities.getAncestorOfClass()
- 24% of CPU time consumes AnnotationView in calls UIManager.getInsets()
So editor performance can be unproved in 4 times if editor avoids that calls.
Priority P2 because navigation in 12Mb file has a visible cursor jam.
Comment 1 Alexander Simon 2010-05-06 07:39:47 UTC
sorry typo: unproved->improved
Comment 2 Alexander Simon 2010-05-06 12:55:45 UTC
next hotspot is:
- AnnotationView. modelToView() that called from AnnotationView.getLinesSpan()
Why modelToView() calls every time:
- AnnotationView.getComponentHeight()
- AnnotationView.getUsableHeight()
- ErrorManager..isLoggable()
?
IMHO it is enough 3 calls on each AnnotationView.getLinesSpan() (or on each AnnotationView.paintComponent()).
Comment 3 Alexander Simon 2010-05-06 13:13:48 UTC
Created attachment 98562 [details]
cumulative patch with caching + improved modelToView
Comment 4 Jan Lahoda 2010-05-06 13:30:30 UTC
Thanks for your investigations and patch. The AnnotationView part seems fine to me. Minor comments:
-the UIManager.getInsets("Nb.Editor.ErrorStripe.ScrollBar.Insets") could be computed in constructor and cached in a final field, which would remove the need to have any getter/local variable.
-getLinesSpan is called repeatedly from paintComponent method, might be possible to cache the component and usable height also in paintComponent method - not sure if that would give big enough improvement.
Comment 5 Jan Lahoda 2010-05-10 07:33:03 UTC
I have applied the AnnotationView patch:
http://hg.netbeans.org/jet-main/rev/32630997d761
(the only change I did is that UIManager.getInsets is kept in a final field and filled in from the constructor). Thanks again for it.
Comment 6 Vitezslav Stejskal 2010-05-10 13:35:42 UTC
With the patch in can this be closed now?
Comment 7 Jan Lahoda 2010-05-10 13:42:40 UTC
I have put in only the AnnotationView part, not the GapDocumentView. That part should be reviewed probably by Mila (I have sent him an e-mail in the morning).
Comment 8 Vitezslav Stejskal 2010-05-11 09:11:48 UTC
The GapDocumentView part looks ok to me. I'm going to apply this part unless Mila objects.
Comment 9 Miloslav Metelka 2010-05-11 10:52:57 UTC
I've applied the patch. BTW I don't think 54% of CPU time consumed is relevant. The SwingUtilities.gAOC() just does Component.getParent() which is getting a field's value and it only iterates once so the only culprit can be Class.isInstance() mostly because it's a native call but as I've seen somewhere
    Class.isInstance and Class.isAssignableFrom are as cheap as instanceof bytecodes when the operands are constants, and otherwise no more expensive than aastore type checks.
so IMHO it's the measurement what is not very accurate in this case.
http://hg.netbeans.org/jet-main/rev/96e1b1b53609
Comment 10 Vitezslav Stejskal 2010-05-11 13:36:56 UTC
(In reply to comment #9)
> so IMHO it's the measurement what is not very accurate in this case.

'Profile Me!' snapshots are almost useless for profiling performance of the rendering code. There are usually zillions of invocations of quite simple methods and the fixed rate sampling of 'Profile Me!' snapshots just misses too much. Using the real profiler with methods instrumentation gives much more accurate results.
Comment 11 Quality Engineering 2010-05-12 04:14:16 UTC
Integrated into 'main-golden', will be available in build *201005112200* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/
User: 
Log:
Comment 12 Quality Engineering 2010-05-14 04:08:47 UTC
Integrated into 'main-golden', will be available in build *201005132200* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/
User: 
Log: