Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 317116

Summary: Should only instantiate one ICustomAnalysisView per profile view
Product: z_Archived Reporter: Mike Reid <mikereid>
Component: TPTPAssignee: Mike Reid <mikereid>
Status: CLOSED FIXED QA Contact: Kathy Chan <kathy>
Severity: normal    
Priority: P2 CC: ewchan, jcayne
Version: unspecifiedFlags: jcayne: review+
Target Milestone: ---   
Hardware: PC   
OS: All   
Whiteboard:
Attachments:
Description Flags
Patch
none
New patch
none
Patch v3
none
Patch v4 none

Description Mike Reid CLA 2010-06-16 17:15:55 EDT 
Currently, contributors of the org.eclipse.tptp.trace.jvmti.client.customAnalysisView extension point are instantiated multiple times. Each time an extensible method is called, a new instance of a contributor is created.

I think this violates the principle of least surprise and generally requires contributors to use static class variables in order to save state. Any per-view state needs to be stored in a static map. This is inconvenient, and since the purpose of the extension is to augment the profiling view, it is natural that an instance of ICustomAnalysisView share the lifecycle of the attached profiling view.
Comment 1 Mike Reid CLA 2010-06-16 17:24:41 EDT 
Created attachment 172076 [details]
Patch

This patch implements the suggested behaviour. The list of contributors is fetched when the BaseMemoryStatisticsView is instantiated and used throughout.

When a new view is created, another copy of the custom analysis view will be created with it.
Comment 2 Kathy Chan CLA 2010-08-24 11:16:13 EDT 
Deferring to TPTP 4.7.2.
Comment 3 Mike Reid CLA 2010-11-05 10:58:11 EDT 
Created attachment 182479 [details]
New patch

Attaching a new patch that implements the same but also provides an extended ICustomAnalysisProvider2 interface that allows for implementers to be told when and which view it will be extending.

This gives the implementer a chance to perform per-view setup.
Comment 4 Mike Reid CLA 2010-11-05 11:03:18 EDT 
Joel, can you let me know your thoughts on the proposed patch?
Comment 5 Mike Reid CLA 2010-11-08 09:27:39 EST 
Created attachment 182615 [details]
Patch v3

Update patch to modify existing ICustomAnalysisView interface rather than introduce new one. The interface is in .provisional package and so is open for change.
Comment 6 Joel Cayne CLA 2010-11-08 09:56:22 EST 
Patch looks good.
Comment 7 Mike Reid CLA 2010-12-08 09:54:24 EST 
Created attachment 184792 [details]
Patch v4

After some discussion with joel, attaching a final patch.

The lifecycle model of ICustomAnalysisView is that an implementer will be instantiated once for the whole workbench process. An additional detachView entry point is provided to notify an implementer when a view is finished with the ICustomAnalysisView.
Comment 8 Mike Reid CLA 2010-12-08 09:55:34 EST 
Patch checked into HEAD.
Comment 9 Kathy Chan CLA 2011-02-11 13:45:36 EST 
This defect had been resolved as FIXED for more than 1 month.  Please verify with the latest TPTP 4.7.2 driver.  If this defect is still left unverified by February 25, we'll close it on the originator's behalf.

TPTP 4.7.2 driver can be downloaded from:

http://www.eclipse.org/tptp/home/downloads/?ver=4.7.2
Comment 10 Mike Reid CLA 2011-02-11 14:58:22 EST 
Verified in TPTP-4.7.2-201102102100.