llvm dev - Mar 2016 - [RFC] Lazy-loading of debug info metadata

On Thu, Mar 24, 2016 at 6:35 AM, Duncan Exon Smith <dexonsmith at
apple.com>
wrote:
>
>
> On Mar 24, 2016, at 6:22 AM, Teresa Johnson <tejohnson at google.com>
wrote:
>
>
>
> On Wed, Mar 23, 2016 at 11:06 PM, Duncan P. N. Exon Smith <
> dexonsmith at apple.com> wrote:
>
>>
>> > On 2016-Mar-22, at 19:28, Duncan P. N. Exon Smith via llvm-dev
<
>> llvm-dev at lists.llvm.org> wrote:
>> >
>> > 1. If a piece of metadata is referenced from only a single
`Function`,
>> >    serialize that metadata in the function's metadata block
instead of
>> >    the global metadata block.
>> >
>> >    This addresses problems (a) and (b), primarily targeting
>> >    `DILocation`s.  It should pick up lots of other stuff,
depending on
>> >    how much inlining has happened.
>> >
>> >    (I have a draft of the writer side, still working on the
reader.)
>>
>> Teresa: I had trouble make this work with the delayed metadata parsing
>> (the logic around "SeenModuleValuesRecord").  My WIP patch
rips that
>> out rather unceremoniously.
>>
>> (Attached the patch for reference, but it needs to be cleaned up, split
>> up, etc.; not ready for review (although comments always welcome)).
>>
>> My understanding from Mehdi is that maybe ThinLTO isn't currently
>> relying on the delayed parsing.  I.e., the original scheme was:
>> - cherry-pick functions one at a time (never reading metadata), then
>> - come back at the end to read the metadata all at once.
>> But the scheme has evolved to:
>> - calculate the desired functions from each module, then
>> - load the module and link all the functions in one go.
>>
>
> Right, that is what the FunctionImporter logic has changed to. I was
> keeping that support alive on the concern that for very large modules the
> overhead of keeping all the source modules materialized while importing
> decisions are made would be too high. But since I added a full
> reference/call graph to the summary and Mehdi has sent a patch to change
> the importer to do summary-based importing, that concern should be moot.
>
>>
>> Is that accurate?  If so, I don't see remaining benefit in delaying
the
>> global metadata parsing, just an extra code path to maintain.  Do you
>> agree?
>>
>>
> I think we can take this support out at this point now.
>
>
> Great.
>
> Note however that llvm-link (used for testing) is still doing function at
> a time importing and therefore relying on this support. However it
> shouldn't be hard to rework that to collect all the imports from a
given
> module for batch importing.
>
> Let me take a stab at moving llvm-link over to using batch importing
> hopefully today or tomorrow, then at least no tests will rely on the
> post-pass metadata importing and I can pull that out independently.
>
>
> Thank you!
>
Removed from llvm-link in  r264326.

Teresa

> Teresa
>
> --
> Teresa Johnson |  Software Engineer |  tejohnson at google.com |
> 408-460-2413
>
>

-- 
Teresa Johnson |  Software Engineer |  tejohnson at google.com |  408-460-2413
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160324/299a7ae5/attachment.html>

> On 2016-Mar-24, at 12:58, Teresa Johnson <tejohnson at google.com>
wrote:
> 
> 
> 
> On Thu, Mar 24, 2016 at 6:35 AM, Duncan Exon Smith <dexonsmith at
apple.com> wrote:
> 
> 
> On Mar 24, 2016, at 6:22 AM, Teresa Johnson <tejohnson at google.com>
wrote:
> 
>> 
>> 
>> On Wed, Mar 23, 2016 at 11:06 PM, Duncan P. N. Exon Smith
<dexonsmith at apple.com> wrote:
>> 
>> > On 2016-Mar-22, at 19:28, Duncan P. N. Exon Smith via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
>> >
>> > 1. If a piece of metadata is referenced from only a single
`Function`,
>> >    serialize that metadata in the function's metadata block
instead of
>> >    the global metadata block.
>> >
>> >    This addresses problems (a) and (b), primarily targeting
>> >    `DILocation`s.  It should pick up lots of other stuff,
depending on
>> >    how much inlining has happened.
>> >
>> >    (I have a draft of the writer side, still working on the
reader.)
>> 
>> Teresa: I had trouble make this work with the delayed metadata parsing
>> (the logic around "SeenModuleValuesRecord").  My WIP patch
rips that
>> out rather unceremoniously.
>> 
>> (Attached the patch for reference, but it needs to be cleaned up, split
>> up, etc.; not ready for review (although comments always welcome)).
>> 
>> My understanding from Mehdi is that maybe ThinLTO isn't currently
>> relying on the delayed parsing.  I.e., the original scheme was:
>> - cherry-pick functions one at a time (never reading metadata), then
>> - come back at the end to read the metadata all at once.
>> But the scheme has evolved to:
>> - calculate the desired functions from each module, then
>> - load the module and link all the functions in one go.
>> 
>> Right, that is what the FunctionImporter logic has changed to. I was
keeping that support alive on the concern that for very large modules the
overhead of keeping all the source modules materialized while importing
decisions are made would be too high. But since I added a full reference/call
graph to the summary and Mehdi has sent a patch to change the importer to do
summary-based importing, that concern should be moot.
>> 
>> Is that accurate?  If so, I don't see remaining benefit in delaying
the
>> global metadata parsing, just an extra code path to maintain.  Do you
>> agree?
>> 
>> 
>> I think we can take this support out at this point now. 
> 
> Great.
> 
>> Note however that llvm-link (used for testing) is still doing function
at a time importing and therefore relying on this support. However it
shouldn't be hard to rework that to collect all the imports from a given
module for batch importing.
>> 
>> Let me take a stab at moving llvm-link over to using batch importing
hopefully today or tomorrow, then at least no tests will rely on the post-pass
metadata importing and I can pull that out independently.
> 
> Thank you!
> 
> Removed from llvm-link in  r264326.
I removed the use of it in r264378.
> 
> Teresa
> 
> 
>> Teresa
>> 
>> -- 
>> Teresa Johnson |	 Software Engineer |	 tejohnson at google.com |	
408-460-2413
> 
> 
> 
> -- 
> Teresa Johnson |	 Software Engineer |	 tejohnson at google.com |	
408-460-2413

On Thu, Mar 24, 2016 at 6:35 PM, Duncan P. N. Exon Smith <
dexonsmith at apple.com> wrote:
>
> > On 2016-Mar-24, at 12:58, Teresa Johnson <tejohnson at
google.com> wrote:
> >
> >
> >
> > On Thu, Mar 24, 2016 at 6:35 AM, Duncan Exon Smith <dexonsmith at
apple.com>
> wrote:
> >
> >
> > On Mar 24, 2016, at 6:22 AM, Teresa Johnson <tejohnson at
google.com>
> wrote:
> >
> >>
> >>
> >> On Wed, Mar 23, 2016 at 11:06 PM, Duncan P. N. Exon Smith <
> dexonsmith at apple.com> wrote:
> >>
> >> > On 2016-Mar-22, at 19:28, Duncan P. N. Exon Smith via
llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
> >> >
> >> > 1. If a piece of metadata is referenced from only a single
`Function`,
> >> >    serialize that metadata in the function's metadata
block instead of
> >> >    the global metadata block.
> >> >
> >> >    This addresses problems (a) and (b), primarily targeting
> >> >    `DILocation`s.  It should pick up lots of other stuff,
depending on
> >> >    how much inlining has happened.
> >> >
> >> >    (I have a draft of the writer side, still working on the
reader.)
> >>
> >> Teresa: I had trouble make this work with the delayed metadata
parsing
> >> (the logic around "SeenModuleValuesRecord").  My WIP
patch rips that
> >> out rather unceremoniously.
> >>
> >> (Attached the patch for reference, but it needs to be cleaned up,
split
> >> up, etc.; not ready for review (although comments always
welcome)).
> >>
> >> My understanding from Mehdi is that maybe ThinLTO isn't
currently
> >> relying on the delayed parsing.  I.e., the original scheme was:
> >> - cherry-pick functions one at a time (never reading metadata),
then
> >> - come back at the end to read the metadata all at once.
> >> But the scheme has evolved to:
> >> - calculate the desired functions from each module, then
> >> - load the module and link all the functions in one go.
> >>
> >> Right, that is what the FunctionImporter logic has changed to. I
was
> keeping that support alive on the concern that for very large modules the
> overhead of keeping all the source modules materialized while importing
> decisions are made would be too high. But since I added a full
> reference/call graph to the summary and Mehdi has sent a patch to change
> the importer to do summary-based importing, that concern should be moot.
> >>
> >> Is that accurate?  If so, I don't see remaining benefit in
delaying the
> >> global metadata parsing, just an extra code path to maintain.  Do
you
> >> agree?
> >>
> >>
> >> I think we can take this support out at this point now.
> >
> > Great.
> >
> >> Note however that llvm-link (used for testing) is still doing
function
> at a time importing and therefore relying on this support. However it
> shouldn't be hard to rework that to collect all the imports from a
given
> module for batch importing.
> >>
> >> Let me take a stab at moving llvm-link over to using batch
importing
> hopefully today or tomorrow, then at least no tests will rely on the
> post-pass metadata importing and I can pull that out independently.
> >
> > Thank you!
> >
> > Removed from llvm-link in  r264326.
>
> I removed the use of it in r264378.
>
Thanks. That is only part of the post-pass metadata linking, I'll remove
the rest.

Teresa
>
> >
> > Teresa
> >
> >
> >> Teresa
> >>
> >> --
> >> Teresa Johnson |      Software Engineer |     tejohnson at
google.com |
> 408-460-2413
> >
> >
> >
> > --
> > Teresa Johnson |       Software Engineer |     tejohnson at google.com
|
> 408-460-2413
>
>

-- 
Teresa Johnson |  Software Engineer |  tejohnson at google.com |  408-460-2413
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160324/bd164ef6/attachment-0001.html>