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 225449 - Lookup queries are blocked due to wait on monitor while doing cleanup
Summary: Lookup queries are blocked due to wait on monitor while doing cleanup
Status: RESOLVED FIXED
Alias: None
Product: platform
Classification: Unclassified
Component: Lookup (show other bugs)
Version: 7.3
Hardware: All All
: P3 normal (vote)
Assignee: Jaroslav Tulach
URL:
Keywords: PERFORMANCE
Depends on:
Blocks:
 
Reported: 2013-01-29 08:11 UTC by Vladimir Voskresensky
Modified: 2013-05-06 12:25 UTC (History)
5 users (show)

See Also:
Issue Type: DEFECT
Exception Reporter: 198122


Attachments
nps snapshot (21.36 KB, application/nps)
2013-01-29 08:11 UTC, Vladimir Voskresensky
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Vladimir Voskresensky 2013-01-29 08:11:25 UTC
This issue was reported manually by vv159170.
It already has 1 duplicates 


Build: NetBeans IDE Dev (Build 20130128-1d53acd6cd83)
VM: Java HotSpot(TM) 64-Bit Server VM, 20.10-b01, Java(TM) SE Runtime Environment, 1.6.0_35-b10
OS: Linux

User Comments:
vv159170: clicking in navigator



Maximum slowness yet reported was 3701 ms, average is 3701
Comment 1 Vladimir Voskresensky 2013-01-29 08:11:28 UTC
Created attachment 130762 [details]
nps snapshot
Comment 2 Egor Ushakov 2013-01-29 08:19:46 UTC
see also bug 189036, I saw the same blocked AbstractLookup.enterStorage() there
Comment 3 Jaroslav Tulach 2013-01-30 06:55:56 UTC
The fact that the monitor is static, is not important (the code is not waiting to enter shared monitor, but in Object.wait) the behavior would be the same even if the lock was per lookup instance.

It seems that the problem is proliferation of AbstractLookup.Result instances. With every click in CND qnavigator new instances of Result are generated (for each queried class). There can be thousands of such instances. When GCed, they then hold the lock for a significant amount of time.

I am changing the known lookups to return existing instances, if possible.
Comment 4 Jaroslav Tulach 2013-01-30 06:59:22 UTC
ergonomics#ab41f4584b22
Comment 5 Vladimir Voskresensky 2013-01-30 07:22:22 UTC
thanks, Jarda!
Comment 6 Quality Engineering 2013-02-01 03:05:31 UTC
Integrated into 'main-golden', will be available in build *201302010001* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/ab41f4584b22
User: Jaroslav Tulach <jtulach@netbeans.org>
Log: #225449: Lookup queries are blocked due to wait on monitor while doing cleanup
AbstractLookup and ExcludingLookup modified to keep track of existing Results and search them before creating a new one. This increased size of ExcludingLookup by 8 bytes and assertSize as some assertGC tests needed to be modified.
Comment 7 Vladimir Voskresensky 2013-02-26 12:58:12 UTC
verified
Comment 8 Vladimir Voskresensky 2013-03-06 17:05:18 UTC
Looks like fix has serious side effect causing OOM.
Although Lookup.Results are not generated all the time now, but listeners are added to found previous result instance unconditionally in AbstractKookup.modifyListenerList => AbstractLookup.listeners field grows up (I see at least 240'000 in hprof) in several instances.

I found reproducible scenario:
- open new c++ file and change selection in navigator => ~ +200 listeners are added and not removed.
- close file and open new one, ... continue to do the same

Our automatic tests checks big projects => get OOM.

Looks like listener shouldn't be added if it (or equal one) is already in listeners list
Comment 9 Vladimir Voskresensky 2013-03-06 17:06:24 UTC
I can provide hprof, if you need it
Comment 10 Marian Mirilovic 2013-03-11 10:15:59 UTC
deferring to NB 7.3 patch 2
Comment 11 Vladimir Voskresensky 2013-03-11 14:02:32 UTC
temporary backout to prevent OOM:
http://hg.netbeans.org/cnd-main/rev/733514b66c94
Comment 12 Quality Engineering 2013-03-12 02:05:31 UTC
Integrated into 'main-golden', will be available in build *201303112300* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/733514b66c94
User: Egor Ushakov <gorrus@netbeans.org>
Log: Backout of the fix for #225449, revision ab41f4584b22
Comment 13 Vladimir Voskresensky 2013-03-13 06:08:29 UTC
*** Bug 227227 has been marked as a duplicate of this bug. ***
Comment 14 Jaroslav Tulach 2013-03-27 13:37:31 UTC
The problems always happen only with CND navigator component. Can you verify NavigatorComponent.getLookup() implementation and confirm it does not create new lookup instances needlessly?

I am going to reintegrate my fix.
Comment 15 Jaroslav Tulach 2013-03-27 14:26:30 UTC
changeset:   d46128064b43
tag:         tip
user:        Jaroslav Tulach <jtulach@netbeans.org>
date:        Wed Mar 27 15:18:46 2013 +0100
summary:     Re-integrating #225449. The broken behaviour (which initiated the rollback) was due to cnd.qnavigator misbehavior.
Comment 16 Vladimir Voskresensky 2013-03-28 05:17:36 UTC
Jarda, thanks for the fix for cnd.qnavigator.
Btw, at night we've got OOM (navigator was not fixed yet) => don't you think it's worth to fix listeners part anyway?
Comment 17 Jaroslav Tulach 2013-05-02 14:13:59 UTC
I know this fix in lookup caused a memory leak in editor. Mílo, can your crosslink your issue?
Comment 18 Jaroslav Tulach 2013-05-02 14:17:15 UTC
Removing 73patch2-candidate as the fix is not quite safe. It is much safer to backport cnd.qnavigator fix.
Comment 19 Miloslav Metelka 2013-05-06 12:25:00 UTC
It was issue #228361.
Only weak listeners should be attached to Lookup.Result. 
Reference to L.R typically needs to be held as a field in an object instance to prevent the L.R from being GCed. Let's call the object Cls1.
Cls1 instance may be statically reachable e.g. a part of some global cache. So far so good. L.R instance is then also statically reachable.
Now let's assume another class Cls2. If that one would attach a regular non-weak listener on (shared) L.R then Cls2 object instance becomes statically reachable.