Re: [linuxtools-dev] Tmf: a model to support new trace analysis types

[Alexandre Montplaisir <alexmonthy@xxxxxxxxxxxx>]

On 13-07-31 11:52 AM, Geneviève Bastien wrote:
> Hi there,
>
> Here is a first patch for review of this feature:
> https://git.eclipse.org/r/#/c/14935/
>
> My main issue is to hook the views to the analysis.
>
>>> Initially I proposed one extension point
>>> org.eclipse.linuxtools.tmf.core.analysis having attribute "provided
>>> views", but that should be really two extension points, as some quick
>>> prototyping clearly pointed out:
>>>
>>> * one for the analysis itself, which logically goes in to tmf.core,
>>> describing the type and class of the analysis.
>>> * one for the views it provides, used by tmf.ui to populate the list
>>> of analysis and available TmfView (maybe all outputs in general?)
>> Do you need the views in the extension points? Couldn't the view simply
>> have a getter that returns the list of views? A bit like we do for state
>> systems and statistics provider for traces, so we don't need to put
>> those in the extension point.
>>
>>
> Alexandre, I know you're not hot at the idea of adding extension
> points.  I'd like to know why because I don't see how else to do it
> than with those 2 extension points (I tried...).

Hehe, it's not that I don't like extension points per se. But more often
than not, extension points are used where plain polymorphism could do
the same job, in a simpler way.

There's still some discussion to do around this, before we dive head-on
into refactoring the whole code base... Personally, I find the whole
"generic pluggable analysis modules with inter-dependencies and such" to
be waaaay over-engineered, at this stage. We should start with something
small, and as we discover new use cases we can iterate on it.


Here's what I had in mind, which is smaller in scope, but I think can
cover the use cases you and the people at Poly are trying to do. Of
course, this is just an idea among others, perhaps the end result will
be something in between ;)

- Have the "analysis" become the new "view". So replace the View
extension point with an Analysis one.
  - An analysis specifies which trace type it supports (so the framework
knows, from a given trace, which analyzes are available).
  - The analysis determines which outputs it has (views, command line
printouts, files, etc.)
  - The analysis controls all its data-gathering requirements. So stuff
like building state systems, doing requests, etc. If needed it can
dictate the order in which it runs those (wait for a query to be
finished before starting the next one, etc.)

>
> Here's the problematic:
>
> * Analysis are core concepts, they can be executed from command line,
> and have default outputs, like text outputs, .dot files for graph,
> etc.  Therefore analysis need to be in the .core plugins (and .core
> plugins must not depend on UI stuff)

I would still put the Analysis in the .ui plugins. Command-line output
is still a UI. If you have absolutely no output at the end, then why run
an analysis at all?

> * Views are only Eclipse or RCP ui objects.  They are just a friendly
> and cute way of representing whatever the analysis did to the trace data.

Agreed, the view now only becomes a (optional) part of an analysis.

> * Views are associated with an analysis, but analysis couldn't care
> less about the views.  So there shouldn't be a getter/setter for the
> list of views.

I disagree. I prefer having the views "inside" the analysis. By having
generic analyzes and generic views at the same time, it increases the
"degrees of freedom" of the system, and everything ends up being more
confusing. Not sure if the gain is really worth this added complexity.

> * One could implement an analysis and someone else can later add a
> view to it, in a totally different module and that should be
> possible.  Or generic (parameterisable) view could be used to display
> data an analysis happen to provide.

This is where polymorphism could do the job, without the need for an
extension point. If you want to add a view to an existing analysis, you
can just extend it and implement your view as part of your new extended
analysis.


Thoughts?

Cheers,
Alex

> * I tried adding an optional interface IAnalysisViewable that the
> analysis need to implement (it's in the patch), but then analysis
> still need the UI stuff.  Otherwise, they can just return a list of
> string id of views, hardcoded because they don't have access to the UI
> module, with the problems that may cause...
>
> So here's why I think we need the second extension point to associate
> an analysis with a view.  Any other idea how that could be done, given
> the previous constraints?
>
> Cheers,
> Geneviève