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.
Summary: | Fast replacement for EditorCookie.getOpenedPanes()[0] | ||
---|---|---|---|
Product: | platform | Reporter: | Michal Mocnak <mmocnak> |
Component: | Text | Assignee: | mslama <mslama> |
Status: | VERIFIED FIXED | ||
Severity: | blocker | CC: | anba, apireviews, dartme18, dima_s_d_s, dstrupl, mentlicher, mgoe, misterm, mmocnak, mslama, ovk, robertwalton, swachter1, tpavek, uipoet, vv159170 |
Priority: | P2 | Keywords: | API_REVIEW_FAST |
Version: | 6.x | ||
Hardware: | All | ||
OS: | All | ||
URL: | http://statistics.netbeans.org/exceptions/detail.do?id=153548 | ||
Issue Type: | DEFECT | Exception Reporter: | 153548 |
Bug Depends on: | |||
Bug Blocks: | 172864 | ||
Attachments: |
nps snapshot
nps snapshot 800ms can be saved by querying AuxiliaryConfiguration outside of AWT 995ms would be saved if sidebars were preloaded outside of AWT Proposed patch New patch. It fixes all notes Added @since |
Description
Michal Mocnak
2009-07-10 20:01:39 UTC
Created attachment 84613 [details]
nps snapshot
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 Created attachment 84624 [details]
nps snapshot
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. Created attachment 85016 [details]
800ms can be saved by querying AuxiliaryConfiguration outside of AWT
Created attachment 85017 [details]
995ms would be saved if sidebars were preloaded outside of AWT
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(). 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. I prepare patch and test build. How to test that it fixes this problem? *** Issue 169109 has been marked as a duplicate of this issue. *** *** Issue 169773 has been marked as a duplicate of this issue. *** Created attachment 85770 [details]
Proposed patch
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. 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; This issue already has 13 duplicates see http://statistics.netbeans.org/exceptions/detail.do?id=153548 This issue already has 14 duplicates see http://statistics.netbeans.org/exceptions/detail.do?id=153548 *** Issue 169939 has been marked as a duplicate of this issue. *** Thanks to Max the SideBarFactories are now initialized outside of AWT. http://hg.netbeans.org/main-silver/rev/c959d8aa2e9c Created attachment 86159 [details]
New patch. It fixes all notes
Created attachment 86160 [details]
Added @since
Excellent. This issue already has 20 duplicates see http://statistics.netbeans.org/exceptions/detail.do?id=153548 jet-main #218c176d8a97 jet-main #212a93c8e507 Fix condition and remove assert. jet-main #2cc969ba8a31 Fix apichanges. 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]. 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. 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. 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. 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 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. *** Issue 171561 has been marked as a duplicate of this issue. *** This fixed has caused a regression again. Please see issue #172889, NbDocument.findRecentEditorPane(org.netbeans.modules.vmd.io.javame.MEDesignEditorSupport) = null. Martin, why did you reopen this? Coudn't you track the regression as a separate issue? Added evaluation to issue #172889. Problem seems to be in mobility. Closing this issue. 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. 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. FYI: this fix has caused another P1 regression - issue #174460. 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. 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. 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? 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. Verified |