This is an archive of the discontinued LLVM Phabricator instance.

Read section headers upfront
ClosedPublic

Authored by rafael on Oct 25 2016, 12:52 PM.

Details

Reviewers
evgeny777
ruiu
Summary

Instead of storing a pointer, store the members we need.

The reason for doing this is that it makes it far easier to create synthetic sections. It also avoids reading data from files multiple times., which might help with cross endian linking and host architectures with slow unaligned access.

There are obvious compacting opportunities, but this already has mixed results even on native x86_64 linking.

There is also the possibility of better refactoring the code for handling common symbols, but this already shows that a custom class is not necessary.

In summary, I would like to commit this an iterate if you guys are OK with it.

The perf number I got on native x86_64 are

firefox

master 7.309622414
patch  7.285605127 1.00329653976x faster

firefox-gc

master 7.510372903
patch  7.485952007 1.00326222984x faster

chromium

master 5.310620915
patch  5.269296067 1.0078425747x faster

chromium fast

master 2.062932807
patch  2.080311297 1.00842416677x slower

the gold plugin

master 0.356284971
patch  0.356714755 1.00120629281x slower

clang

master 0.602845631
patch  0.604443573 1.00265066531x slower

llvm-as

master 0.034642356
patch  0.034852631 1.00606988162x slower

the gold plugin fsds

master 0.3861195
patch  0.388452457 1.00604205952x slower

clang fsds

master 0.688210298
patch  0.689806352 1.00231913705x slower

llvm-as fsds

master 0.0320734
patch  0.032240692 1.005215911x slower

scylla

master 3.249120693
patch  3.246897164 1.00068481658x faster

I am building lld on a big endian power8 and will try to bencmark a cross link of an x86_64 binary.

Diff Detail

Event Timeline

rafael retitled this revision from to Read section headers upfront.
rafael updated this object.
rafael added reviewers: ruiu, evgeny777.
rafael added a subscriber: llvm-commits.

added llvm-commits

I just benchmarked on ppc64. Unfortunately I don't have root in the machine, so I am getting over 1% variation in each run and so this patch is in the noise there.

davide added a subscriber: davide.Oct 25 2016, 2:04 PM

If you plan to iterate on this, probably it's fine, but my reaction when I see your number is that the performance effect of the change is pretty much neutral. Sure, you have some slowdowns and some improvements, but they vary roughly between 0.1% and 0.5% , which, no matter how controlled the enviroment on which we're running tests, could be caused by other factors.

ruiu edited edge metadata.Oct 25 2016, 3:20 PM

Thank you for doing this. I agree that it should make things easier to create linker-synthesized input sections.

ELF/InputSection.cpp
35–36

What is this change for?

846

Is it a common practice to create an ArrayRef with non-zero size and a nullptr? It seems odd to me.

ELF/InputSection.h
93

Please add a comment to note that they correspond to Elf_Shdr.

124–129

Do you still need these accessors?

285–286

Please update the comment becasue "this class" no longer exists.

rafael updated this revision to Diff 75822.Oct 25 2016, 5:42 PM
rafael edited edge metadata.

Address review comments

ruiu accepted this revision.Oct 25 2016, 5:52 PM
ruiu edited edge metadata.

LGTM.

So now that I'm not sure if I liked the separation of data and size. If you want to revert it, please do it.

This revision is now accepted and ready to land.Oct 25 2016, 5:52 PM
rafael updated this revision to Diff 75823.Oct 25 2016, 5:58 PM
rafael edited edge metadata.

Avoid creating an ArrayRef with nullptr.

espindola closed this revision.Mar 14 2018, 4:25 PM
espindola added a subscriber: espindola.

285148