This is an archive of the discontinued LLVM Phabricator instance.

[LLD][ELF] Fix performance of MarkLive::scanEhFrameSection
ClosedPublic

Authored by andrewng on Sep 7 2020, 10:09 AM.

Details

Summary

MarkLive::scanEhFrameSection is used to retain personality/LSDA
functions when --gc-sections is enabled.

Improve its performance by only iterating over the .eh_frame relocations
that need to be resolved for an EhSectionPiece. This optimization makes
the same assumption as elsewhere in LLD that the .eh_frame relocations
are sorted by r_offset.

This appears to be a performance regression introduced in commit
e6c24299d237 (https://reviews.llvm.org/D59800).

This change has been seen to reduce link time by up to ~50%.

Diff Detail

Event Timeline

andrewng created this revision.Sep 7 2020, 10:09 AM
andrewng requested review of this revision.Sep 7 2020, 10:09 AM
MaskRay added a comment.EditedSep 7 2020, 3:44 PM

Thanks for noting this issue! Were you profiling ld.lld?

Here we are making a little assumption that a FDE's relocations are contiguous. The same assumption is made by InputSections getReloc (called by EhInputSection::split), so I think it is fine.
(A more robust approach is to sort relocations by r_offset, but it is cumbersome/slow to do that here as rels are raw Elf*_Rel[a] records.)

Worth mentioning that this function is used to retain personality/LSDA functions.

lld/ELF/MarkLive.cpp
156

parentheses beside && are not needed

andrewng updated this revision to Diff 290432.Sep 8 2020, 1:51 AM
andrewng edited the summary of this revision. (Show Details)

Address review comments.

andrewng marked an inline comment as done.Sep 8 2020, 1:59 AM

Thanks for noting this issue! Were you profiling ld.lld?

We have a game link that was taking an unusually long time to link with our downstream linker when --gc-sections was enabled. So I ran this link through the profiler and it identified this function as the primary hot spot which led me to find this issue.

MaskRay accepted this revision.Sep 8 2020, 8:36 AM

Looks great!

This revision is now accepted and ready to land.Sep 8 2020, 8:36 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 8 2020, 11:36 AM