[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [tracecompass-dev] API change: CallStack Analysis

Hi Genevieve

Weighing the advantages of keeping the changes with the disadvantages of supporting the old code base, I lean towards keeping the changes. The API changes needed to make it work with the new code base are minimal and are well documented in this email. It will help to innovate the call stack/call graph/profiling features by bringing in the incubator enhancements.

Having said that, I'd like to put the Symbol Provider code back to tmf.core/tmf.ui as proposed and discussed in patch [1]. Besides, we'd like to keep the original view ids (see patch [2]) to avoid exception in user's perspectives.

Any objections?

Best Regards
Bernd

[1] https://git.eclipse.org/r/#/c/123106/
[2] https://git.eclipse.org/r/#/c/123123/


-----Original Message-----
From: tracecompass-dev-bounces@xxxxxxxxxxx <tracecompass-dev-bounces@xxxxxxxxxxx> On Behalf Of GeneviÃve Bastien
Sent: May-23-18 10:29 AM
To: tracecompass-dev@xxxxxxxxxxx
Subject: Re: [tracecompass-dev] API change: CallStack Analysis

Hi Bernd,


I'm trying to see if I can have the old extension point still working without undeprecating. The problem as someone said are deprecation warnings. Worst case is to change the deprecation errors to warnings for the profiling plugin for now. I'm working on it, but if you prefer to merge the undeprecate patch, please do so. Where the symbol resides will not be a problem to move incubator's callstack to profiling, so it's fine with me.


Best regards,

GeneviÃve


On 2018-05-22 09:59 PM, Bernd Hufmann wrote:
> Hi Genevieve,
>
> thanks for working on migration the incubator call stack features to Trace Compass mainline.
>
> When rebasing to the latest master in Trace Compass repo that contains your updates, I noticed that an existing extension, that I have to the CallStackAnalysis, doesn't populate the CallStackView, and CallGraph Analysis related views anymore. Looking at the changes, I realized that the "old" source code for these features are deprecated, but not wired up anymore in the plugin.xml. So, the source code is still compiling but not available anymore at runtime. A similar problem arises with the symbol provider change. I noticed a ClassCastException when the ISymbolProviderManager is casting to the ISymbolProviderFactory instance deprecated in tmf.ui, but expects an ISymbolProviderFactory in profiling.ui plug-in. Here also, the previous code still exists and is deprecated, but causes the extension to fail. 
>
> I would be willing to update to the new API in profiling plug-ins and it would not big change. However, there might be (and we know there are) other adopters of the call stack view and symbol provider etc. in community who are affected by this. Any ideas how we can solve this the best way? Reverting the patches could be the last resort. Reminder that tomorrow is API and feature freeze and we need to have the fix merged by lunch so that we have a RC1 build early afternoon.
>
> Please let me know.
>
> Best Regards
> Bernd
>   
> ________________________________________
> From: tracecompass-dev-bounces@xxxxxxxxxxx 
> <tracecompass-dev-bounces@xxxxxxxxxxx> on behalf of GeneviÃve Bastien 
> <gbastien@xxxxxxxxxxxx>
> Sent: May 22, 2018 12:26 PM
> To: tracecompass-dev@xxxxxxxxxxx
> Subject: Re: [tracecompass-dev] API change: CallStack Analysis
>
> So the change has been merged, it's harmless enough ;-) Thanks for the 
> prompt reviews.
>
>
> Additional notes to implementers:
>
>
> * If you extend the CallStackAnalysis or anything related, you'll need 
> to add a dependency to 
> 'org.eclipse.tracecompass.analysis.profiling.[core|ui]' and update all 
> references to the now deprecated classes (they have the same name, 
> package, just change 'tmf' to 'analysis.profiling').
>
>
> * The symbols have also been moved to the profiling plugins and the 
> ones in tmf.core/ui have been deprecated. Just changing the references 
> should be enough. If you extended the symbol provider extension point, 
> you may update the extension point name to 
> 'org.eclipse.tracecompass.analysis.profiling.core.symbolProvider'
>
>
> * In addition to the profiling plugins, you may need to add 
> dependencies to the following plugins if you extended the CallStackAnalysis:
> org.eclipse.tracecompass.analysis.timing.core and 
> org.eclipse.tracecompass.segmentstore.core
>
>
> GeneviÃve
>
>
>
> On 2018-05-16 12:45 PM, GeneviÃve Bastien wrote:
>> Hi all,
>>
>>
>> We've been experimenting with the callstack analysis in the incubator 
>> for a while now. That feature is known as the Generic Callstack and 
>> it has feature parity and much more with the current CallStack 
>> analysis, so we'll slowly move it in the main repo. There's a lot of 
>> code there and API freeze is may 23rd, so here's the [very] short 
>> term plan to migrate the feature:
>>
>>
>> 1- Add a new set of plugins in Trace Compass:
>> org.eclipse.tracecompass.analysis.profiling.[core|ui][.tests[.swtbot]
>> ]
>>
>>
>> 2- Move the IFlameChartProvider and IFlameGraphProvider from the 
>> incubator to those plugins.
>>
>>
>> 3- Move the current CallStackAnalysis and its view to that plugin and 
>> have it extend the previous interfaces. The one in tmf.[core|ui] will 
>> be deprecated.
>>
>>
>> 4- For implementers, all you have to do is have your 
>> CallStackAnalysis and view extend the new ones instead of the 
>> deprecated ones and no other code change should be needed for now. 
>> I'll let you know of any other update required, but I can think of none for now.
>>
>>
>> As another step to productify the generic callstack, we'll be doing 
>> the
>> following:
>>
>>
>> * Move some interfaces from the incubator's analysis.core plugin to 
>> analysis.os.linux.core in the main repo, so that an os-aware 
>> callstack can eventually make use of it, and analysis in the main 
>> repo can implement those interfaces instead of having wrappers in the incubator.
>>
>>
>> That's it for 4.0, then it gives us plenty of time to migrate the 
>> code to main repo and add the generic callstack features in the 4.0 
>> cycle, without requiring major API breakages.
>>
>>
>> Cheers,
>>
>> GeneviÃve
>>
>> _______________________________________________
>> tracecompass-dev mailing list
>> tracecompass-dev@xxxxxxxxxxx
>> To change your delivery options, retrieve your password, or 
>> unsubscribe from this list, visit 
>> https://dev.eclipse.org/mailman/listinfo/tracecompass-dev
> _______________________________________________
> tracecompass-dev mailing list
> tracecompass-dev@xxxxxxxxxxx
> To change your delivery options, retrieve your password, or 
> unsubscribe from this list, visit 
> https://dev.eclipse.org/mailman/listinfo/tracecompass-dev
> _______________________________________________
> tracecompass-dev mailing list
> tracecompass-dev@xxxxxxxxxxx
> To change your delivery options, retrieve your password, or 
> unsubscribe from this list, visit 
> https://dev.eclipse.org/mailman/listinfo/tracecompass-dev

_______________________________________________
tracecompass-dev mailing list
tracecompass-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit https://dev.eclipse.org/mailman/listinfo/tracecompass-dev