Messages in this thread | | | Date | Tue, 27 Oct 2020 13:01:30 +0100 | From | Jiri Olsa <> | Subject | Re: [PATCH v2 12/15] perf record: introduce thread local variable for trace streaming |
| |
On Mon, Oct 26, 2020 at 05:11:30PM +0300, Alexei Budankov wrote: > > On 26.10.2020 13:34, Jiri Olsa wrote: > > On Mon, Oct 26, 2020 at 11:21:28AM +0300, Alexei Budankov wrote: > >> > >> On 24.10.2020 18:43, Jiri Olsa wrote: > >>> On Wed, Oct 21, 2020 at 07:07:00PM +0300, Alexey Budankov wrote: > >>>> > >>>> Introduce thread local variable and use it for threaded trace streaming. > >>>> > >>>> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com> > >>>> --- > >>>> tools/perf/builtin-record.c | 71 ++++++++++++++++++++++++++++++++----- > >>>> 1 file changed, 62 insertions(+), 9 deletions(-) > >>>> > >>>> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c > >>>> index 89cb8e913fb3..3b7e9026f25b 100644 > >>>> --- a/tools/perf/builtin-record.c > >>>> +++ b/tools/perf/builtin-record.c > >>>> @@ -101,6 +101,8 @@ struct thread_data { > >>>> u64 bytes_written; > >>>> }; > >>>> > >>>> +static __thread struct thread_data *thread; > >>>> + > >>>> struct record { > >>>> struct perf_tool tool; > >>>> struct record_opts opts; > >>>> @@ -587,7 +589,11 @@ static int record__pushfn(struct mmap *map, void *to, void *bf, size_t size) > >>>> } > >>>> } > >>>> > >>>> - rec->samples++; > >>>> + if (thread) > >>>> + thread->samples++; > >>>> + else > >>>> + rec->samples++; > >>> > >>> this is really wrong, let's keep just single samples counter > >>> ditto for all the other places in this patch > >> > >> This does look like data parallelism [1] which is very true for > >> threaded trace streaming so your prototype design looks optimal. > >> > >> For this specific place incrementing global counter in memory is > >> less performant and faces scalability limitations as a number of > >> cores grow. > >> > >> Not sure why you have changed your mind. > > > > I'm not sure I follow.. what I'm complaining about is to have > > 'samples' stat variable in separate locations for --threads > > and --no-threads mode > > It is optimal to have samples variable as per thread one > and then sum up the total in the end of data collection. > > Single global variable design has scalability and performance > drawbacks. > > Why do you complain about per thread variable in this case? > It looks like ideally fits these specific needs.
I think there's misunderstanding.. I think we should move samples to per thread 'thread' object and have just one copy of that.. and do not increase separate variables for thread and non-thread cases
jirka
| |