Re: [PATCH v2 3/4] perf script: Add --lazy-load-symbols for lazy symbol loading

[Alireza Haghdoost]

> > +
> > +     if (!symbol_conf.allow_aliases) {
> > +             u32 out = 0;
>
> I think we agreed to factor out this block of code.
>

You’re right. I interpreted the earlier request too narrowly and only
factored out the pairwise selection policy into symbol__choose_best().
I’ll factor out the complete block in v3.


> > +
> > +     /*
> > +      * Keep an exact-path data DSO for the symbol source.  This may differ
> > +      * from the runtime image (for example, split debuginfo), so using the
> > +      * primary DSO's data cache could read an unrelated string-table offset.
> > +      * The standard DSO data cache manages descriptor eviction and reopening.
> > +      */
>
> Yes, it's a known problem and I hope to address it soon.  Can you please
> make the path handling a separate commit?  I think it's an independent
> fix for split debuginfo.
>

Ack. I’ll split the exact-path DSO data-cache support into a separate
split-debuginfo fix and have the lazy-loading patch build on top
of it in v3.

> > +static int choose_best_symbol(struct symbol *syma, struct symbol *symb)
> > +{
> > +     return symbol__choose_best(syma->end - syma->start,
> > +                                symbol__type(syma), symbol__binding(syma), syma->name,
> > +                                symb->end - symb->start,
> > +                                symbol__type(symb), symbol__binding(symb), symb->name);
> >  }
>
> To reduce the size of the patch, I think it's better to split this as a
> separate commit.  Please consider minimize the patch size in general to
> help reviewers. :)
>

Ack. I’ll move the shared duplicate-selection refactoring into a
separate preparatory commit to keep the lazy-loading patch smaller in v3.