1654182 - check_binary is slow on Linux builds

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Mike Hommey [:glandium]]

I found this out while building like automation locally for bug 1654123.

Comment 1

[Mike Hommey [:glandium]]

Attachment: Bug 1654182 - Reduce the amount of work done for binary compatibility check.

Iterating symbols is actually expensive because of all the line
splitting. So iterating over all symbols 4 times is actually a huge
waste of time.

Instead, iterate over them once, doing all the version checks at once
for each of them.

This brings down the time spent in check_binary from 2:25 to 0:45 on my
machine.

Comment 2

[Mike Hommey [:glandium]]

Attachment: Bug 1654182 - Remove support for iterating symbols on non-Linux platforms.

The way check_binary currently works is the result of many years of
changes, but some of the more recent ones actually make some of the
earliest ones unnecessary.

For instance, bug 1541792 removed the need to use iter_symbols on
non-Linux platforms. So we remove support for non-Linux (non-ELF,
really) platforms in iter_symbols (and rename the function to reflect
that).

Comment 3

[Mike Hommey [:glandium]]

Attachment: Bug 1654182 - Only look at the dynamic symbols table for the version checks.

The main reason we look at the complete symbols table is that before bug
1541792, we needed to look at that table for _NSModule symbols.

In bug 1516228, we also made everything llvm-objdump to limit the
differences cross-platform, but that's not necessary anymore per the
previous change.

llvm-objdump doesn't support getting only the dynamic symbols table, so
we go back to what we were using before bug 1516228, namely readelf,
while preserving a code path to use the complete symbols table for the
networking test on libgkrust.a, which doesn't have a dynamic symbols
table.

With this change, check_binary goes from 45s to 0.2s on my machine.

Comment 4

[Mike Hommey [:glandium]]

Strictly speaking, with the last patch, the first patch is essentially useless, but that's the first thing I tried and it still seemed like a nice thing to do.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6100f0d3c0ad
Reduce the amount of work done for binary compatibility check. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/13ab8f6d24bf
Remove support for iterating symbols on non-Linux platforms. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/205329cc5f30
Only look at the dynamic symbols table for the version checks. r=froydnj

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8ecbb1da91cf
Fix spidermonkey bustages. r=glandium

Comment 7

[Dorel Luca [:dluca]]

Backed out 4 changesets (bug 1654182) for Spidermonkey failure and Android build bustage. CLOSED TREE

Log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=310615836&repo=autoland&lineNumber=39885

Log:
https://treeherder.mozilla.org/logviewer.html#?job_id=310615842&repo=autoland

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception&revision=205329cc5f3073075ad768606fa092ed81a3673c

Backout:
https://hg.mozilla.org/integration/autoland/rev/8b96fe79f8ba64d5f89b4b501d03a2ae4fd6753e

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d2787ae2b210
Reduce the amount of work done for binary compatibility check. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/baa743f59bcc
Remove support for iterating symbols on non-Linux platforms. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/b26f94c6bfde
Only look at the dynamic symbols table for the version checks. r=froydnj

Comment 9

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/d2787ae2b210
https://hg.mozilla.org/mozilla-central/rev/baa743f59bcc
https://hg.mozilla.org/mozilla-central/rev/b26f94c6bfde