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

Bug 318412

Summary: Change inheritance of AbstractOprofileLaunchConfigurationDelegate to ProfileLaunchConfigurationDelegate instead of AbstractCLaunchDelegate
Product: z_Archived Reporter: Thavidu Ranatunga <thavidur>
Component: LinuxToolsAssignee: Roland Grunberg <rgrunber>
Status: RESOLVED FIXED QA Contact:
Severity: enhancement    
Priority: P3 CC: charley.wang, kksebasti, linux.oprofile-inbox
Version: unspecified   
Target Milestone: ---   
Hardware: PC   
OS: Linux   
Whiteboard:
Attachments:
Description Flags
patch attempt
none
mylyn/context/zip none

Description Thavidu Ranatunga CLA 2010-06-30 02:02:01 EDT 
Build Identifier: 

While looking what functions I used in common in the Perf plugin that should be moved to the profiling framework I realised the respective common functions had already been moved/implemented there- the OProfile plugin just hadn't changed to be using it yet. Changing this inheritance covers most of it- the exec() function in AbstractOprofile.. should be removed and references swapped to execute() in ProfileLaunch.. =)

Reproducible: Always
Comment 1 Andrew Overholt CLA 2010-07-08 15:28:30 EDT 
Created attachment 173805 [details]
patch attempt

Did you envision something like this?
Comment 2 Andrew Overholt CLA 2010-07-08 15:28:32 EDT 
Created attachment 173806 [details]
mylyn/context/zip
Comment 3 Andrew Overholt CLA 2010-07-08 15:31:51 EDT 
Charley, do you know whatthe generateCommand implementations should be?
Comment 4 Charley Wang CLA 2010-07-08 15:40:21 EDT 
(In reply to comment #3)
> Charley, do you know whatthe generateCommand implementations should be?

The Profile Framework's launch process works by creating and launching a script in /tmp:

#!/bin/sh
exec __COMMAND__

__COMMAND__ is filled by the return value of generateCommand, and should contain the command as it would appear on the commandline.

e.g. for SystemTap, we return:

stap -o /output/path -c '/binary/path' /script/path.stp /binary/path
Comment 5 Charley Wang CLA 2010-07-09 12:20:48 EDT 
Further detail: I can take a look at having OProfile use ProfileLaunchConfiguraitonDelegate#createProcess, which would make use of the generateCommand function. 

From a quick glance, I think the difference is that createProcess launches a system process and returns an IProcess with streams that can be monitored, while execute just launches a system process. For OProfile this does not appear to be necessary.

I will investigate further.
Comment 6 Charley Wang CLA 2010-07-09 14:26:54 EDT 
Tested Andrew's patch, it works :)

No real benefits to using createProcess here, but this section of API could perhaps be better documented
Comment 7 Thavidu Ranatunga CLA 2010-07-14 01:41:31 EDT 
Personally when I swapped my Perf implementation (which was originally based on OProfile) to the one in the profiling framework, I didn't make use of generateCommand or createProcess here, the previous method is better suited I think =)
Comment 8 Roland Grunberg CLA 2010-08-03 13:41:07 EDT 
The patch looks fine to me. As long as createProcess isn't used, there's no reason to implement generateCommand.
Comment 9 Andrew Overholt CLA 2010-08-04 09:39:20 EDT 
Please apply as you see fit, Roland.
Comment 10 Roland Grunberg CLA 2010-08-04 10:12:03 EDT 
Revision #25325
Comment 11 Andrew Overholt CLA 2010-08-04 10:52:29 EDT 
Resolving.