Bug 153923 - avoid loading project instance only to obtain icon in project chooser.
avoid loading project instance only to obtain icon in project chooser.
Status: RESOLVED FIXED
Product: projects
Classification: Unclassified
Component: Generic Infrastructure
6.x
All All
: P2 (vote)
: 6.x
Assigned To: Milos Kleint
issues@projects
: API, API_REVIEW_FAST
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-11-26 10:19 UTC by Milos Kleint
Modified: 2009-02-19 22:56 UTC (History)
0 users

See Also:
Issue Type: ENHANCEMENT
:


Attachments
basic diff (10.15 KB, patch)
2008-11-26 10:28 UTC, Milos Kleint
Details | Diff
complete list of changes associated with the issue (41.09 KB, text/plain)
2009-01-06 07:36 UTC, Milos Kleint
Details
new patch (34.93 KB, patch)
2009-01-13 13:27 UTC, Milos Kleint
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Milos Kleint 2008-11-26 10:19:30 UTC
see subject.
in current codebase the project needs to be loaded, it's lookup constructed and the ProjectInformation looked up in
order to annotate the folder icon in the project chooser. That's memory and CPU inefficient
Comment 1 Milos Kleint 2008-11-26 10:28:21 UTC
Created attachment 74172 [details]
basic diff
Comment 2 Milos Kleint 2008-11-26 10:33:55 UTC
a simple sketch that works,
todos:
dummy names ending with 2 need probably better wording
no apichanges dialog
only maven project type rewritten to use  the new api.

Comment 3 Jesse Glick 2008-11-26 17:48:27 UTC
[JG01] ImageUtilities.loadImage should be called with a 2nd arg 'true'.


[JG02] I don't see ProjectFactory2 here - maybe forgot to 'hg add'?


[JG03] If you are speeding up icon loading, you can surely retune ProjectChooserAccessory's badging tactic. It might be
feasible to make it badge synchronously. (Try on a cold disk.) If not, you can at least reduce or eliminate the schedule
delay in the badger task.


[JG04] Might be handy for Result.getIcon to have a 'boolean forceLoad' param, which if true would do the logic PCA is
doing now in 'if (icon == null) {...}'. Just test carefully on projects that e.g. have invalid project.xml files or are
otherwise unloadable.


[JG05] OpenProject.java change is unnecessary.


[JG06] Perhaps Result should be abstract, and an icon should be produced only on demand. Consider Ant-based projects. It
is quick to tell if Result should be non-null: whether project.xml exists or not. But parsing project.xml to find the
specific type is more expensive, and some code may only be interested in whether the folder is a project, but not
interested in the icon, so it should not have to pay for the parsing. Of course the primary code calling isProject is
PCA, which wants the icon, but we would be penalizing any other code. If PM.isProject (not iP2) still called old PF.iP
then this would not be an issue, but your current patch makes PM.iP delegate to iP2.

A related question is how you avoid parsing project.xml a second time if and when the Project is actually to be loaded.
Currently ABPFS parses project.xml only once, when PF.loadProject is called. But if it needs to parse project.xml just
to get an icon, and then later lP is called, how could it avoid reparsing it? Maybe this is a minor enough issue that it
would suffice to rely on warm disk caches, and assume that the icon parse (with no validation enabled) is fast enough.
If it seems important, a WeakHashMap<FileObject,Document> cache (as mentioned in a separate issue) would be the obvious
approach, but then you need to deal with the file being modified on disk after the cache is created but before the
project is loaded. (Once the project is loaded, APH tracks file changes on disk.)
Comment 4 Milos Kleint 2008-11-27 11:29:02 UTC
JG01 done
JG02 forgot to add indeed, has just method     ProjectManager.Result isProject2(FileObject projectDirectory);
JG03 definitely TBD, maybe at the time most frequent project types use ProjectFactory2
JG04 I was thinking of providing special impl of Icon in case someone needs such a lazy loading. If the class turns into
abstract (or non final only) we can remove the Result(Icon) constructor
JG05 just a sideeffect or the repository wide commit
Comment 5 Milos Kleint 2009-01-06 07:36:44 UTC
Created attachment 75471 [details]
complete list of changes associated with the issue
Comment 6 Milos Kleint 2009-01-06 07:40:44 UTC
please review. The changes in APIs for projectapi and project.ant are intended to prevent project loading in the open
project dialog.

JG01-06 shall be addressed now, I upgraded the spec versions, implemented the new apis in j2se, web, apisupport and
maven modules, added javadoc and apichanges.


the attached diff is produced by hg out -p, I haven't figured any way of showing the complete diff of local changes.
Comment 7 Jesse Glick 2009-01-06 14:05:05 UTC
Regarding a diff: call the base rev (parent of c682e4092760 in this case) $base. Then run

hg di -r $base -r tip

If you do any merges from trunk, then you would use the trunk parent of the merge as your new base rev. A bit of work
but the result is correct; see also

http://www.selenic.com/mercurial/bts/issue28

You can also try

http://wiki.netbeans.org/HgHowTos#section-HgHowTos-DevelopAPIReviewPatchesUsingMQ

if you prefer that style.

BTW it is a good idea to set

[diff]
git=1
Comment 8 Milos Kleint 2009-01-12 07:03:16 UTC
re diff: Unfortunately I have 2 large scale merges in between the relevant local changes. Diff produced  wrong results.

I'd like to integrate in the coming days, please speak up if you strongly oppose the move.
Comment 9 Jesse Glick 2009-01-12 18:26:58 UTC
As I say, having merges in between the local changes should not be a problem so long as you set the base revision for
the diff correctly, to the remote changeset from the last such merge. If you can't figure it out quickly, just attach a
bundle (hg bundle /tmp/153923.hg) and I can try to produce the diff for you.
Comment 10 Jesse Glick 2009-01-13 00:38:52 UTC
[JG07] "{@link projectManager.Result}" is not going to work.


[JG08] Why is Exceptions.printStackTrace commented out in AntBasedProjectFactorySingleton?


[JG09] I guess ABPFS should return something non-null for an old ABPT provider. I think there needs to be some way for a
Result to be constructed with an unknown icon.


[JG11] Why the Result(String) constructor? Result(Icon) seems enough to me.
Comment 11 Milos Kleint 2009-01-13 13:27:45 UTC
Created attachment 75753 [details]
new patch
Comment 12 Milos Kleint 2009-01-13 13:41:19 UTC
final diff attached.

JG07+JG11 done.
JG08+JG09 - good catch, thanks. i've reworked the way Ant factory works, as the ProjectManager.isProject() javadoc
states, the method shall be "optimistic" in the sense that it can report false positives, but shall not report false
negatives. That was not the case with the previous ant implementation, shall be better now.

I've pushes the bits to main.
Comment 13 Quality Engineering 2009-01-14 07:43:17 UTC
Integrated into 'main-golden', will be available in build *200901140201* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/c682e4092760
User: Milos Kleint <mkleint@netbeans.org>
Log: #153923 extend ProjectFactory to provide a project icon and rewrite the project open dialog to utilize it


By use of this website, you agree to the NetBeans Policies and Terms of Use. © 2012, Oracle Corporation and/or its affiliates. Sponsored by Oracle logo