Re: [PATCH v2] perf evlist: Remove group option.

[Ian Rogers]

On Wed, Sep 22, 2021 at 2:17 PM Jiri Olsa <jolsa@xxxxxxxxxx> wrote:
>
> On Tue, Sep 21, 2021 at 11:13:49AM -0700, Ian Rogers wrote:
>
> SNIP
>
> > diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> > index 8feef3a05af7..9cd79513eebb 100644
> > --- a/tools/perf/util/python.c
> > +++ b/tools/perf/util/python.c
> > @@ -1109,14 +1109,6 @@ static PyObject *pyrf_evlist__open(struct pyrf_evlist *pevlist,
> >                                  PyObject *args, PyObject *kwargs)
> >  {
> >       struct evlist *evlist = &pevlist->evlist;
> > -     int group = 0;
> > -     static char *kwlist[] = { "group", NULL };
> > -
> > -     if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|OOii", kwlist, &group))
> > -             return NULL;
> > -
> > -     if (group)
> > -             evlist__set_leader(evlist);
> >
> >       if (evlist__open(evlist) < 0) {
> >               PyErr_SetFromErrno(PyExc_OSError);
> > diff --git a/tools/perf/util/record.c b/tools/perf/util/record.c
> > index bff669b615ee..9e694db7c7ee 100644
> > --- a/tools/perf/util/record.c
> > +++ b/tools/perf/util/record.c
> > @@ -99,13 +99,6 @@ void evlist__config(struct evlist *evlist, struct record_opts *opts, struct call
> >       bool use_comm_exec;
> >       bool sample_id = opts->sample_id;
> >
> > -     /*
> > -      * Set the evsel leader links before we configure attributes,
> > -      * since some might depend on this info.
> > -      */
> > -     if (opts->group)
> > -             evlist__set_leader(evlist);
> > -
>
> I don't mind erasing that, but just curious if you're going
> to add something which would clash with this? it does not
> look too complex for the code.. but still, let's remove it
> if it's in 'legacy mode' for this long ;-)

I've nothing conflicting but I came across old tutorials describing it
and I suspect usage of it is likely broken - in particular mixing
--group with -e {}.

Thanks,
Ian

> thanks,
> jirka
>
>
> >       if (evlist->core.cpus->map[0] < 0)
> >               opts->no_inherit = true;
> >
> > diff --git a/tools/perf/util/record.h b/tools/perf/util/record.h
> > index 68f471d9a88b..d71dee9ce41c 100644
> > --- a/tools/perf/util/record.h
> > +++ b/tools/perf/util/record.h
> > @@ -13,7 +13,6 @@ struct option;
> >
> >  struct record_opts {
> >       struct target target;
> > -     bool          group;
> >       bool          inherit_stat;
> >       bool          no_buffering;
> >       bool          no_inherit;
> > --
> > 2.33.0.464.g1972c5931b-goog
> >
>