[PATCH] D113073: [lld-macho] Cache library paths from findLibrary

[Keith Smiley via Phabricator via llvm-commits]

keith added a comment.

In D113073#3118944 <https://reviews.llvm.org/D113073#3118944>, @oontvoo wrote:

> In D113073#3118817 <https://reviews.llvm.org/D113073#3118817>, @keith wrote:
>
>> 
>
> [...]
>
>>>>>>> By the way, thanks for contributing all these optimizations! I was quite surprised to hear that ld64 was faster, given that LLD is typically much faster for our own workloads, but I guess you have rather different inputs. Hopefully we can make LLD the fastest Mach-O linker for all builds :)
>>>>>>
>>>>>> Thanks for all the reviews! For context our project is a huge iOS application with on the order of thousands of static libraries, and many iOS system framework + system library dependencies. Is this a case you've benchmarked? If so I'd be interested to dig a bit deeper into the differences to try and understand why it has been slower for us.
>>>>>
>>>>> For one of our largest ios apps that I've measured(different from int3's ):
>>>>>
>>>>> - ~7100 archives
>>>>> - 56 frameworks  (including system ones)
>>>>> - 12 weak frameworks
>>>>
>>>> Thanks for the info! What's your bottleneck at this point? After all my changes here our biggest area is we now spend 20+ seconds in `lld::macho::readFile`
>>>
>>> *With* all of the patches improving load input applied, linking the app above is only slightly faster than our optimised LD64:
>>> (cross-linking, on Linux - Broadwell (yeah, kind of old) RAM 32GB)
>>>
>>>   x ./LLD_with_cache_patches.txt
>>>   + ./LD64_local_imprv.txt
>>>   +-----------------------------------------------------------------------------------------------------------------------------------------------------------------------+
>>>   |x     *           x  *x  x  * *          +      +   +                         +                                                                +                      +|
>>>   |        |_|________A__M______|__________________M____________A____________________________________________________|                                                    |
>>>   +-----------------------------------------------------------------------------------------------------------------------------------------------------------------------+
>>>       N           Min           Max        Median           Avg        Stddev
>>>   x  10         23.64         23.89         23.82        23.799   0.077667382
>>>   +  10         23.69         25.01         24.04        24.146     0.4379041
>>>   Difference at 95.0% confidence
>>>   	0.347 +/- 0.295482
>>>   	1.45804% +/- 1.24157%
>>>   	(Student's t, pooled s = 0.314478)
>>>
>>> Looking at the trace, the bottleneck is still in loading input and writing output. 
>>> It seems it could be improved further ...
>>>
>>> F20184452: Screen Shot 2021-11-09 at 10.20.13 AM.png <https://reviews.llvm.org/F20184452>
>>
>> Thanks for the data point! You might have mentioned before but does this mean these changes _were_ an improvement for you as well? Do you have a time for this benchmark before these changes?
>
> Yes - these changes improved it a bit.
>
> without these improvement, LLD vs  LD64 (our fork) :
>
>   x LLD_no_cache.txt
>   + LD64_local_imprv.txt
>   +-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
>   |   +  x                *         +   +    x    x  x +         + x  +  x    x                          +                                                                                          +                               +                        x|
>   ||_____|___________________________________________M___________M____A____________A______________________________________________________|_________________|                                                                                                 |
>   +-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
>       N           Min           Max        Median           Avg        Stddev
>   x  10         23.71         25.16         23.97        24.073    0.40136019
>   +  10         23.69         25.01         24.04        24.146     0.4379041
>   No difference proven at 95.0% confidence

That's too bad, I guess our use cases are just different enough. But good to know it didn't regress, thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113073/new/

https://reviews.llvm.org/D113073