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 168415 - Fast replacement for EditorCookie.getOpenedPanes()[0]
Summary: Fast replacement for EditorCookie.getOpenedPanes()[0]
Status: VERIFIED FIXED
Alias: None
Product: platform
Classification: Unclassified
Component: Text (show other bugs)
Version: 6.x
Hardware: All All
: P2 blocker (vote)
Assignee: mslama
URL: http://statistics.netbeans.org/except...
Keywords: API_REVIEW_FAST
: 169109 169773 169939 171561 (view as bug list)
Depends on:
Blocks: 172864
  Show dependency tree
 
Reported: 2009-07-10 20:01 UTC by Michal Mocnak
Modified: 2009-11-26 03:07 UTC (History)
16 users (show)

See Also:
Issue Type: DEFECT
Exception Reporter: 153548


Attachments
nps snapshot (63.27 KB, bin/nps)
2009-07-10 20:01 UTC, Michal Mocnak
Details
nps snapshot (77.24 KB, bin/nps)
2009-07-11 12:38 UTC, Michal Mocnak
Details
800ms can be saved by querying AuxiliaryConfiguration outside of AWT (51.67 KB, application/octet-stream)
2009-07-21 15:15 UTC, Jaroslav Tulach
Details
995ms would be saved if sidebars were preloaded outside of AWT (28.11 KB, image/png)
2009-07-21 15:24 UTC, Jaroslav Tulach
Details
Proposed patch (8.81 KB, text/plain)
2009-08-04 14:32 UTC, mslama
Details
New patch. It fixes all notes (19.59 KB, text/plain)
2009-08-12 15:51 UTC, mslama
Details
Added @since (19.62 KB, text/plain)
2009-08-12 15:53 UTC, mslama
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Michal Mocnak 2009-07-10 20:01:39 UTC
Build: NetBeans IDE Dev (Build 200907100200)
VM: Java HotSpot(TM) 64-Bit Server VM, 11.3-b02-83, Java(TM) SE Runtime Environment, 1.6.0_13-b03-211
OS: Mac OS X, 10.5.7, x86_64

User Comments:
jglick: Started up IDE, editor window open.

mmocnak: Starting the IDE


Maximal alredy reported slowness was 11171 ms, average is 7754
Comment 1 Michal Mocnak 2009-07-10 20:01:48 UTC
Created attachment 84613 [details]
nps snapshot
Comment 2 Michal Mocnak 2009-07-11 12:37:57 UTC
Build: NetBeans IDE Dev (Build 200907100200)
VM: Java HotSpot(TM) 64-Bit Server VM, 11.3-b02-83, Java(TM) SE Runtime Environment, 1.6.0_13-b03-211
OS: Mac OS X, 10.5.7, x86_64

User Comments: 

Maximal alredy reported slowness was 11171 ms, average is 8372
Comment 3 Michal Mocnak 2009-07-11 12:38:05 UTC
Created attachment 84624 [details]
nps snapshot
Comment 4 mslama 2009-07-13 14:17:26 UTC
I do not see any possibility how to fix this. Visual part of editor is created in AWT thread. All possible work (eg.
document loading) is already done out of AWT thread.
Comment 5 Jaroslav Tulach 2009-07-21 15:15:45 UTC
Created attachment 85016 [details]
800ms can be saved by querying AuxiliaryConfiguration outside of AWT
Comment 6 Jaroslav Tulach 2009-07-21 15:24:15 UTC
Created attachment 85017 [details]
995ms would be saved if sidebars were preloaded outside of AWT
Comment 7 Jaroslav Tulach 2009-07-21 16:25:03 UTC
The whole call is initiated from debugger. Possible solution is to create new API between openide.text and 
spi.debugger.ui to find out that the editor is not ready yet. I suggest:

public class org.openide.text.NbDocument {
  public static JEditorPane[] findInitializedPanes(EditorCookie ec) {
    if (ec instanceof CloneableEditorSupport) {
      // find if initialized or return null immediatelly
    } else {
      return ec.getOpenedPanes();
    }
  }
}

And use this method in spi.debugger.ui instead of plain call to getOpenedPanes().
Comment 8 Jaroslav Tulach 2009-07-26 09:32:24 UTC
Please address this problem soon. It hides other issues related to initialization of editor and it shall not be left 
up until the end of the release. Thanks.
Comment 9 mslama 2009-07-29 13:53:47 UTC
I prepare patch and test build. How to test that it fixes this problem?
Comment 10 mslama 2009-07-31 13:40:25 UTC
*** Issue 169109 has been marked as a duplicate of this issue. ***
Comment 11 Jindrich Sedek 2009-08-04 07:59:53 UTC
*** Issue 169773 has been marked as a duplicate of this issue. ***
Comment 12 mslama 2009-08-04 14:32:48 UTC
Created attachment 85770 [details]
Proposed patch
Comment 13 mslama 2009-08-04 14:41:51 UTC
Proposed patch avoids blocking AWT thread when CloneableEditor.getEditorPane() is called before
CloneableEditor.DoInitialize.initNonVisual is finished. 

CloneableEditor.DoInitialize.initNonVisual is performed out of AWT thread so its indirect call blocks AWT.
(CloneableEditor.getEditorPane() calls CloneableEditor.DoInitialize.initVisual which waits till
CloneableEditor.DoInitialize.initNonVisual is finished. Code uses CloneableEditorSupport. If EditorCookie is not
intanceof CloneableEditorSupport it works like before using EditorCookie.getOpenedPanes.

CloneableEditor also fires EditorCookie.Observable.PROP_OPENED_PANES when CloneableEditor.DoInitialize.initVisual is
finished so clients are notified that editor pane is ready to use.
Comment 14 Jaroslav Tulach 2009-08-05 08:03:05 UTC
Y01 Write some tests to show that the method returns sooner than plain getOpenedPanes()[0] while initializing.
Y02 Reminder: right now there are apichanges.xml, spec. version increment and @since tag
Y03 I do not like two parameters for the static method. Can there be either EditorCookie only (preferable imho) or 
TopComponent only
Y04 Name of the method could be findRecentEditorPane(...)
Y05 Firing property change is OK, just test the behaviour, please.

Btw. to getEditorPanes() without blocking I would use following sketch (untested):

Index: CloneableEditorSupport.java
--- CloneableEditorSupport.java Base (BASE)
+++ CloneableEditorSupport.java Locally Modified (Based On LOCAL)
@@ -1051,6 +1051,9 @@
      * @see EditorCookie#getOpenedPanes
      */
     public JEditorPane[] getOpenedPanes() {
+        return getOpenedPanes(true);
+    }
+    JEditorPane[] getOpenedPanes(boolean optimal) {
         // expected in AWT only
         assert SwingUtilities.isEventDispatchThread()
                 : "CloneableEditorSupport.getOpenedPanes() must be called from AWT thread only"; // NOI18N
@@ -1073,7 +1076,12 @@
             if (ed != null) {
                 // #23491: pane could be still null, not yet shown component.
                 // [PENDING] Right solution? TopComponent opened, but pane not.
-                JEditorPane p = ed.getEditorPane();
+                JEditorPane p;
+                if (!optimal && (ed instanceof CloneableEditor)) {
+                    p = ((CloneableEditor)ed).pane;
+                } else {
+                    p = ed.getEditorPane();
+                }
 
                 if (p == null) {
                     continue;
Comment 15 Exceptions Reporter 2009-08-05 18:03:59 UTC
This issue already has 13 duplicates 
see http://statistics.netbeans.org/exceptions/detail.do?id=153548
Comment 16 Exceptions Reporter 2009-08-06 14:22:34 UTC
This issue already has 14 duplicates 
see http://statistics.netbeans.org/exceptions/detail.do?id=153548
Comment 17 mslama 2009-08-07 13:55:24 UTC
*** Issue 169939 has been marked as a duplicate of this issue. ***
Comment 18 Vitezslav Stejskal 2009-08-11 09:40:08 UTC
Thanks to Max the SideBarFactories are now initialized outside of AWT.
http://hg.netbeans.org/main-silver/rev/c959d8aa2e9c
Comment 19 mslama 2009-08-12 15:51:08 UTC
Created attachment 86159 [details]
New patch. It fixes all notes
Comment 20 mslama 2009-08-12 15:53:15 UTC
Created attachment 86160 [details]
Added @since
Comment 21 Jaroslav Tulach 2009-08-13 07:45:05 UTC
Excellent.
Comment 22 Exceptions Reporter 2009-08-13 15:43:23 UTC
This issue already has 20 duplicates 
see http://statistics.netbeans.org/exceptions/detail.do?id=153548
Comment 23 mslama 2009-08-17 12:52:45 UTC
jet-main #218c176d8a97
Comment 24 mslama 2009-08-17 14:08:11 UTC
jet-main #212a93c8e507 Fix condition and remove assert.
Comment 25 mslama 2009-08-19 13:39:37 UTC
jet-main #2cc969ba8a31 Fix apichanges.
Comment 26 Quality Engineering 2009-08-21 06:12:18 UTC
Integrated into 'main-golden', will be available in build *200908210201* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/218c176d8a97
User: Marek Slama <mslama@netbeans.org>
Log: #168415: Add CES.getRecentPane nonblocking replacement for CloneableEditorSupport.getOpenedPanes()[0].
Comment 27 Martin Entlicher 2009-08-24 16:37:12 UTC
This fix has caused regression - issue #170802.

Reopening - NbDocument.findRecentEditorPane(ec) returns null when ec is an instance of
org.netbeans.modules.form.FormEditorSupport.

I'll roll-back the debugger part of this fix, till this is repaired.
Comment 28 Quality Engineering 2009-08-25 02:43:46 UTC
Integrated into 'main-golden', will be available in build *200908242212* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/494c247fd06e
User: mentlicher@netbeans.org
Log: #170802 - Use EditorCookie.getOpenedPanes() again to fix regression. Rollback of fix of issue #168415.
Comment 29 Vitezslav Stejskal 2009-08-25 14:11:24 UTC
Marek (mslama) is no vacation this week and there is nobody else who can work on this at the moment. I'm sorry, but you
will have to wait until he is back.
Comment 30 mslama 2009-09-03 11:22:35 UTC
Reason for this problem is that CloneableOpenSupport.allEditors contains
org.netbeans.core.multiview.MultiViewCloneableTopComponent ie. outer TC whereas CloneableEditorSupport.getLastSelected
returns org.netbeans.modules.form.FormEditorSupport$JavaEditorTopComponent ie. inner TC which is set in
FormEditorSupport$JavaEditorTopComponent.componentActivated -> CloneableEditor.componentActivated.

MultiViewCloneableTopComponent subclass directly CloneableTopComponent so it does not set last selected component in
CloneableEditorSupport.

Fix uses java.awt.Container.isAncestorOf to find out last selected component if == fails.

jet-main #70dfc0e65066
Comment 31 Quality Engineering 2009-09-07 09:57:27 UTC
Integrated into 'main-golden', will be available in build *200909041634* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/70dfc0e65066
User: Marek Slama <mslama@netbeans.org>
Log: #168415: Fix NbDocument.findRecentPane for multiview component.
Comment 32 mslama 2009-09-21 18:52:52 UTC
*** Issue 171561 has been marked as a duplicate of this issue. ***
Comment 33 Martin Entlicher 2009-09-24 15:49:23 UTC
This fixed has caused a regression again. Please see issue #172889,
NbDocument.findRecentEditorPane(org.netbeans.modules.vmd.io.javame.MEDesignEditorSupport) = null.
Comment 34 David Strupl 2009-09-25 08:41:38 UTC
Martin, why did you reopen this? Coudn't you track the regression as a separate issue?
Comment 35 mslama 2009-09-25 12:48:02 UTC
Added evaluation to issue #172889. Problem seems to be in mobility. Closing this issue.
Comment 36 Martin Entlicher 2009-09-25 13:43:08 UTC
I've reopened this as it seemed to cause the P1 regression. I really did not want to spend time filling new defects, as
this was the second time the new API caused problems.
Comment 37 Quality Engineering 2009-10-09 23:04:40 UTC
Integrated into 'main-golden', will be available in build *200910091401* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/cb40ba424f1d
User: mentlicher@netbeans.org
Log: #173269 - Rollback of http://hg.netbeans.org/main/rev/dc7b7eaa1878 after issue #172889 was fixed. This reapplies the original fix of issue #168415.
Comment 38 Martin Entlicher 2009-10-14 17:29:48 UTC
FYI: this fix has caused another P1 regression - issue #174460.
Comment 39 Jaroslav Tulach 2009-10-14 18:11:14 UTC
Small correction. This fix causes no compatibility problems, neither regressions. The regressions are caused by issue 
172864.

Originally I did a mistake and tracked both, separate problems under issue 168415. I want to apologize for that and 
gently ask to deal with any future regressions as part of issue 172864 while keeping this issue only for the 
compatible API extension in NbDocument.
Comment 40 Martin Entlicher 2009-10-14 18:39:44 UTC
Well, yes, technically you're right that regressions are caused by issue #172864.
But debugger has no choice here. It must not call EditorCookie.getOpenedPanes(), therefore it calls
NbDocument.findRecentEditorPane().

NbDocument.findRecentEditorPane() does not work when the EditorCookie does not manage correctly the selected panes.
From this point of view replacement of EditorCookie.getOpenedPanes() with NbDocument.findRecentEditorPane(EditorCookie)
is not a compatible change.

From the discussion I had with Yarda a moment ago evolved that:
It would be cool if the implementation could detect that CloneableEditorSupport.getLastSelected() returns null and in
case that the opening was finished, call EditorCookie.getOpenedPanes(), which should return immediately in this case.
This would assure the compatibility in behavior.
Comment 41 Vladimir Voskresensky 2009-10-14 20:38:04 UTC
I'm reopening because of strange NPE http://www.netbeans.org/issues/show_bug.cgi?id=174573
Could someone clarify, please:
EditorCookie ec;
panes = ec.getOpenedPanes();
if (panes != null && panes[0] != null) => does it guarantee that NbDocument.findRecentEditorPane(ec) != null? 
(files are not closed between ec.getOpenedPanes() && NbDocument.findRecentEditorPane(ec) calls)

Are these two impl in sync? I expected - yes. But... NPE

May be side effect of Java Memory Model? I.e. visibility of "smth" is not exposed to other threads after finished editor
initialization thread?
Comment 42 mslama 2009-11-02 11:14:55 UTC
Closing again. Yes if CloneableEditor.componentActivated is called as expected then
CloneableEditorSupport.setLastSelected is called then:
EditorCookie ec;
panes = ec.getOpenedPanes();
if (panes != null && panes[0] != null) => does it guarantee that NbDocument.findRecentEditorPane(ec) != null? 
(files are not closed between ec.getOpenedPanes() && NbDocument.findRecentEditorPane(ec) calls)
as you said above. If you have problem with file issue and assign it to me (platform/text). I will add logging so that
when next time NPE happens we can get more info.
Comment 43 Michal Mocnak 2009-11-26 03:07:44 UTC
Verified