Re: [PATCH] perf maps: Remove check_invariants() from maps__lock()

[Arnaldo Carvalho de Melo]

On Mon, Apr 29, 2024 at 07:09:54PM -0700, Ian Rogers wrote:
> On Mon, Apr 29, 2024 at 3:57 PM Namhyung Kim <namhyung@xxxxxxxxxx> wrote:
> >
> > I found that the debug build was a lot slowed down by the maps lock code
> > since it checks the invariants whenever it gets the pointer to the lock.
> > This means it checks twice the invariants before and after the access.
> >
> > Instead, let's move the checking code within the lock area but after any
> > modification and remove it from the read paths.  This would remove (more
> > than) the half of the maps lock overhead.
> >
> > The time for perf report with a huge data file (200k+ of MMAP2 events).
> >
> >   Non-debug     Before      After
> >   ---------   --------   --------
> >      2m 43s     6m 45s     4m 21s

Yeah, I had a debug build to check the size of 'struct hist_entry' with
pahole and noticed that invariant checking in 'perf top'.

> Thanks Namhyung, I think the change makes sense. There is an issue
> that a user of a map may mutate it in a way that breaks the maps
> sorting, symbol code would be a likely culprit of such a thing. A fix
> for such a breakage would be to just set the unsorted flag on maps.
> We'd be less likely to spot such an issue after this change, but it is
> simple enough to add the function call when needed. Another option
> would be to add another make flag like PARSER_DEBUG for these kind of,
> "be paranoid," type things. Anyway, let's try to fix those problems
> when they exist.
> 
> Reviewed-by: Ian Rogers <irogers@xxxxxxxxxx>

Thanks, applied to perf-tools-next,

- Arnaldo