Bug 182238 - PDE source lookup director creates source container for each request
Summary: PDE source lookup director creates source container for each request
Status: RESOLVED FIXED
Alias: None
Product: PDE
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.3   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.4 M2   Edit
Assignee: Darin Wright CLA
QA Contact:
URL:
Whiteboard:
Keywords: bugday, performance
Depends on:
Blocks:
 
Reported: 2007-04-12 17:54 EDT by Darin Wright CLA
Modified: 2007-09-08 17:19 EDT (History)
6 users (show)

See Also:
baumanbr: review+


Attachments
patch (16.51 KB, patch)
2007-09-07 17:00 EDT, Darin Wright CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Darin Wright CLA 2007-04-12 17:54:31 EDT
While investigating bug 181994, we noticed that the PDE source lookup director creates a source container each time it looks up source. If this approach is to be used, it would be best to "dispose()" of each container once done with it. However, it would likely be even better to cache/re-use source containers per source lookup director instance. It would create less gargbage, and should be event faster (than creating them on demand for each request).
Comment 1 Brian Bauman CLA 2007-09-07 15:50:37 EDT
This could be a good bug to get some new feet wet with :)
Comment 2 Chris Aniszczyk CLA 2007-09-07 15:54:13 EDT
ok, sweet. first one to get the patch wins the race.

Your picture will go on our famous PDE committer/contributor webpage.

http://www.eclipse.org/pde/pde-ui/committers/committers.php

gogogogogo
Comment 3 Darin Wright CLA 2007-09-07 17:00:11 EDT
Created attachment 77924 [details]
patch

Adds a cache of source containers to the PDE source lookup director. The source lookup query retrieves source containers from the director rather than building them on each request. Containers are disposed when the director is disposed (when the launch is removed). Containers are added to the cache lazily - the first time a container is required for a location/bundle.

Makes moving between stack frames (source lookup) in different bundles more snappy.
Comment 4 Michael Rennie CLA 2007-09-07 17:07:33 EDT
just had to beat me to it :)

not axesome
Comment 5 Adam Archer CLA 2007-09-07 17:10:20 EDT
That was quick!

Debug doesn't mess around.
Comment 6 Chris Aniszczyk CLA 2007-09-07 17:26:07 EDT
That's what I'm talking about, bug reporters who actually solve the bug they report ;)
Comment 7 Brian Bauman CLA 2007-09-07 17:51:21 EDT
Darin, looks very good, not to mention the quick turn around.  I like the custom key object :)  

Patch applied as-is to HEAD.  Will be available in 3.4M2.
Comment 8 Darin Wright CLA 2007-09-07 22:55:45 EDT
Admitedly - I had an unfair advantage here since it was an implementation of an interface that debug defines. However, it's good practice to eat your own dog food.

The PDE source lookup director is interesting in that it performs a dynamic source lookup based on bundles, rather than searching along a path for source (i.e. like a typical classpath).
Comment 9 Chris Aniszczyk CLA 2007-09-08 15:01:02 EDT
Now we need a picture of Darin featuring a "leafy background" to put him on PDE's hall of fame webpage.
Comment 10 Adam Archer CLA 2007-09-08 16:12:50 EDT
(In reply to comment #9)
> Now we need a picture of Darin featuring a "leafy background" to put him on
> PDE's hall of fame webpage.

Actually, if you look at his sametime picture, you will notice that it promotes the PDE trademarked "leafy background". He was clearly born ready.
Comment 11 Wassim Melhem CLA 2007-09-08 17:19:18 EDT
I am not sure this bug challenged Darin to step out of his comfort zone.  It's an optimization of a source lookup director that is based on the debug framework.

Can we have him write a form-based editor or something? :)