Re: [RESEND PATCH] reader context: do not reuse current corpus and corpus_group

[Matthias Maennich <maennich@google.com>]

Hi Dodji,

On Mon, May 04, 2020 at 04:42:02PM +0200, Dodji Seketeli wrote:
>Hello Matthias, Giuliano,
>
>Matthias Maennich <maennich@google.com> a écrit:
>
>> libabigail's readers (abg-reader and abg-dwarf-reader) currently spare
>> some allocations by reusing the reader context's existing current corpus
>> and current corpus group. When building a corpus_group's vector of
>> corpora, reusing the shared_ptr referring to a corpus means we are
>> modifying the corpus data of a previously read corpus. As a user of the
>> read*corpus functions, that isn't entirely transparent and when storing
>> corpare like in the vector above, we might introduce subtle bugs.
>
>[...]
>
>Giuliano Procida <gprocida@google.com> a écrit:
>
>> Does this change have any impact on incomplete (forward-declared) type
>> differences?
>> Does it significantly impact performance on large inputs?
>
>Right.  That is my concern as well.  The reason why we avoid
>instantiating a new dwarf reader context all the time is for performance
>reasons, especially when analysing a kernel with lots of kernel
>modules.  Example of kernels would be any of the known enterprise
>kernels, I guess.
>
>
>That being said, a lot of things have happened on the front of
>performance for kernels with lots of modules, so maybe this particular
>optimization is not useful anymore, I am not sure.
>
>In any case, we should measure this first before we know if this can get
>in.
>
>I hope this makes sense.

I agree. It makes sense to measure this in a more diverse context. The
Android Kernels were running with this patch for a while, but they were
not at all representative. Since this was a prerequisite patch for a now
obsolete patch, I am also ok to just drop it. I will also drop it from
mm-next then.

Cheers,
Matthias

>
>Cheers,
>
>-- 
>		Dodji