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 191844 - 26s in setting preferredProject
Summary: 26s in setting preferredProject
Status: RESOLVED FIXED
Alias: None
Product: projects
Classification: Unclassified
Component: Generic Infrastructure (show other bugs)
Version: 7.0
Hardware: All All
: P2 normal (vote)
Assignee: Jesse Glick
URL:
Keywords: PERFORMANCE, REGRESSION
Depends on:
Blocks: 187657
  Show dependency tree
 
Reported: 2010-11-11 13:17 UTC by Tomas Danek
Modified: 2011-02-27 23:55 UTC (History)
2 users (show)

See Also:
Issue Type: DEFECT
Exception Reporter: 173816


Attachments
nps snapshot (18.47 KB, application/nps)
2010-11-11 13:17 UTC, Tomas Danek
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Tomas Danek 2010-11-11 13:17:37 UTC
This bug was originally marked as duplicate of bug 177675, that is already resolved. This bug is still valid, so this seems to be another bug, but it might be related.

Build: NetBeans IDE 7.0 Beta (Build 201011082300)
VM: Java HotSpot(TM) 64-Bit Server VM, 17.1-b03-307, Java(TM) SE Runtime Environment, 1.6.0_22-b04-307-10M3261
OS: Mac OS X

User Comments:
musilt2: via favorites view invoked popup on project folder node



Maximum slowness yet reported was 26783 ms, average is 26783
Comment 1 Tomas Danek 2010-11-11 13:17:42 UTC
Created attachment 102905 [details]
nps snapshot
Comment 2 Jaroslav Tulach 2010-11-29 20:12:20 UTC
Cause by YAGL created by "simplification" when trying to fix bug #187657.
Comment 3 Jesse Glick 2010-11-29 22:37:49 UTC
core-main #53f82a9b175d
Comment 4 Quality Engineering 2010-12-01 06:41:19 UTC
Integrated into 'main-golden', will be available in build *201012010001* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/53f82a9b175d
User: Jesse Glick <jglick@netbeans.org>
Log: #191844: LowPerformance took 26783 ms.
Comment 5 Jaroslav Tulach 2010-12-02 14:40:26 UTC
Jesse, I don't think that "doing something later" is proper fix of this problem. The goal of the preferredProject method is to tell the project opening task what project the user is interested in, so it can be opened preferrably. As such the method preferredProject needs to finish immediatelly, not after 26s or 40s.

I guess this goes back to bug 187657. The "Simplification" of threading is probably over simplification. Consider backout of 2160fbaa2114 and try different fix.
Comment 6 Jesse Glick 2010-12-02 17:29:47 UTC
(In reply to comment #5)
> The goal of the preferredProject method is to tell the project opening
> task what project the user is interested in, so it can be opened [first].

Only affects lazy projects during startup AFAIK. Not a big deal IMHO. Can open a separate bug for it but it's not P2. The IDE is typically going to be useless until real projects are opened anyway.

> Consider backout of 2160fbaa2114 and try different fix.

I don't have any other ideas for a fix; the synchronization in the previous code was terribly complicated and could not be seen to be correct with incremental fixes.
Comment 7 Jaroslav Tulach 2010-12-02 20:23:41 UTC
I can imagine that fans of single global locks would be disappointed by the original code.

> The IDE is typically going to be useless until real projects are opened anyway.

If you remove all code that makes it usable and replace it with YAGLs or delayed processing, then yes, it will indeed be unusable.
Comment 8 Jesse Glick 2010-12-02 21:35:05 UTC
When projects are still being opened, typically the disk and CPU are saturated and the UI will not be responsive; this phase is much shorter than scanning and is effectively just the tail end of startup. Whatever task reorderings might be made here do not seem worth the added complexity and risk of deadlocks and race conditions which have historically accompanied them.

Anyway this bug was filed for a particular EQ freeze which I think is solved: LoadOpenProjects.resultChanged should not acquire a mutex.
Comment 9 Jaroslav Tulach 2010-12-03 09:47:31 UTC
(In reply to comment #8)
> Whatever task reorderings might be
> made here do not seem worth the added complexity and risk of deadlocks and race
> conditions which have historically accompanied them.

We have successfully dealt with all deadlocks before 7.0. No reason to believe  #187657 is unfixable with minor change. I don't see a single reason to replace 99% correct code with complete rewrite full of performance problems which does not even do what it should:

> Anyway this bug was filed for a particular EQ freeze which I think is solved:
> LoadOpenProjects.resultChanged should not acquire a mutex.

postponing reaction to result change effectively disables the whole system. (I think that) As soon as the ProjectManager.mutex() lock is free, all the projects are open anyway. With attitude like this you can easily delete the code.

> > Consider backout of 2160fbaa2114 and try different fix.
> 
> I don't have any other ideas for a fix; 

It is easy: write a test and then make small change. Give me permission and I can do it (reopen #187657, backout 2160fbaa2114, write the test, do the fix).

> the synchronization in the previous
> code was terribly complicated and could not be seen to be correct with
> incremental fixes.

Every change in that code used to be backed up by unit tests. Should new code changes continue to follow that practice the correctness would be assured even with incremental fixes.
Comment 10 Jesse Glick 2010-12-03 18:00:19 UTC
(In reply to comment #9)
> postponing reaction to result change effectively disables the whole system.

Could write to some kind of lockless set of project paths that should be loaded first, rather than manipulating the to-be-loaded list directly. This would bypass PM.mutex without compromising the integrity of the locked data structures.

> With attitude like this you can easily delete the code.

I have always advocated removing the lazy project system.

> Give me permission and I can do it

If you want to maintain OpenProjectList and related code in perpetuity, go ahead.

> Every change in that code used to be backed up by unit tests. Should new code
> changes continue to follow that practice the correctness would be assured even
> with incremental fixes.

Unit tests do a lousy job of protecting against threading-related bugs. Deleting problematic locks and idioms is more permanent.
Comment 11 Jaroslav Tulach 2010-12-03 20:07:16 UTC
> I have always advocated removing the lazy project system.

I see.So 2160fbaa2114 is the first step towards this great vision of yours.

> > Give me permission and I can do it
> 
> If you want to maintain OpenProjectList and related code in perpetuity, go
> ahead.

I'll think about that offer.
Comment 12 Jesse Glick 2010-12-06 21:33:24 UTC
(In reply to comment #10)
> Could write to some kind of lockless set of project paths that should be loaded
> first, rather than manipulating the to-be-loaded list directly.

Looked at this but I don't think I follow what the original logic was doing. toOpenProjects is a collection of Project, so they must have been loaded already. What would be the purpose of changing the order in which they are _opened_? AFAIK all newly opened projects are displayed in the Projects tab at once, and of course Java functionality will not work until everything has been both opened and scanned. I can't think of any visible effect of reordering this list.
Comment 13 _ tboudreau 2011-02-27 23:55:13 UTC
Yes, please remove lazy projects.  It's a perfect example of the what you get when you hyperfocus on startup-time, and is mostly just annoying.

If you want startup to be faster, then, do less work to reach a fully initialized state.  Do some actual optimization.