[all-commits] [llvm/llvm-project] 637a33: [MachO] Fix struct size assertion

Shoaib Meenai via All-commits all-commits at lists.llvm.org
Tue Nov 16 16:31:11 PST 2021


  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 637a3396b3f8292d2118973b90eff314c553f6d2
      https://github.com/llvm/llvm-project/commit/637a3396b3f8292d2118973b90eff314c553f6d2
  Author: Shoaib Meenai <smeenai at fb.com>
  Date:   2021-11-16 (Tue, 16 Nov 2021)

  Changed paths:
    M lld/MachO/InputSection.h

  Log Message:
  -----------
  [MachO] Fix struct size assertion

It was checking for 64-bit builds incorrectly. Unfortunately,
ConcatInputSection has grown a bit in the meantime, and I don't see any
obvious way to shrink it. Perhaps icfEqClass could use 32-bit hashes
instead of 64-bit ones, but xxHash64 is supposed to be much faster than
xxHash32 (https://github.com/Cyan4973/xxHash#benchmarks), so that sounds
like a loss. (Unrelatedly, we should really look at using XXH3 instead
of xxHash64 now.)

Reviewed By: #lld-macho, int3

Differential Revision: https://reviews.llvm.org/D113809


  Commit: 31952978970dcc3a2b91a0ac240d9ab06c06c856
      https://github.com/llvm/llvm-project/commit/31952978970dcc3a2b91a0ac240d9ab06c06c856
  Author: Shoaib Meenai <smeenai at fb.com>
  Date:   2021-11-16 (Tue, 16 Nov 2021)

  Changed paths:
    M lld/MachO/Symbols.cpp
    M lld/MachO/Symbols.h

  Log Message:
  -----------
  [MachO] Reduce size of Symbol and Defined

We can lay out Symbol more optimally to reduce its size from 56 bytes to
48 bytes by eliminating unnecessary padding, and we can lay out Defined
such that its bitfield members are placed in the tail padding of Symbol
(on ABIs which support this), to reduce it from 96 bytes to 80 bytes (8
bytes from the Symbol reduction, and 8 bytes from the tail padding
reuse).

This is perf-neutral for an internal app (results from two different
machines):

```
           smol-syms       baseline        difference (95% CI)
sys_time   7.430 ± 0.202   7.440 ± 0.193   [  -2.6% ..   +2.9%]
user_time  21.443 ± 0.513  21.206 ± 0.396  [  -3.3% ..   +1.1%]
wall_time  20.453 ± 0.534  20.222 ± 0.488  [  -3.7% ..   +1.5%]
samples    9               8

           smol-syms       baseline        difference (95% CI)
sys_time   3.011 ± 0.050   3.040 ± 0.052   [  -0.4% ..   +2.3%]
user_time  10.416 ± 0.075  10.496 ± 0.091  [  +0.1% ..   +1.4%]
wall_time  12.229 ± 0.144  12.354 ± 0.192  [  -0.1% ..   +2.1%]
samples    14              13
```

However, on the first machine, it reduces maximum resident set size by
65.9 MB (0.8%, from 7.92 GB to 7.85 GB). On the second machine, it
reduces it by 92 MB (1.4%, from 6.40 GB to 6.31 GB).

Reviewed By: #lld-macho, int3

Differential Revision: https://reviews.llvm.org/D113813


  Commit: 93bf271f27439d133616815266b50db4a294e118
      https://github.com/llvm/llvm-project/commit/93bf271f27439d133616815266b50db4a294e118
  Author: Shoaib Meenai <smeenai at fb.com>
  Date:   2021-11-16 (Tue, 16 Nov 2021)

  Changed paths:
    M lld/MachO/InputFiles.cpp
    M lld/MachO/Relocations.h

  Log Message:
  -----------
  [MachO] Shrink reloc from 32 bytes to 24 bytes

The `r_address` field of `relocation_info` is only 4 bytes, so our
offset field (which is the `r_address` field adjusted for subsection
splitting) also only needs to be 4 bytes. This reduces the structure
size from 32 bytes to 24 bytes.

Combined with https://reviews.llvm.org/D113813, this is a minor perf
improvement for linking an internal app, tested on two machines:

```
           smol-relocs     baseline        difference (95% CI)
sys_time   7.367 ± 0.138   7.543 ± 0.157   [  +0.9% ..   +3.8%]
user_time  21.843 ± 0.351  21.861 ± 0.450  [  -1.3% ..   +1.4%]
wall_time  20.301 ± 0.307  20.556 ± 0.324  [  +0.1% ..   +2.4%]
samples    16              16

           smol-relocs     baseline        difference (95% CI)
sys_time   2.923 ± 0.050   2.992 ± 0.018   [  +1.4% ..   +3.4%]
user_time  10.345 ± 0.039  10.448 ± 0.023  [  +0.8% ..   +1.2%]
wall_time  12.068 ± 0.071  12.229 ± 0.021  [  +1.0% ..   +1.7%]
samples    15              12
```

More importantly though, this change by itself reduces our maximum
resident set size by 220 MB (2.75%, from 7.85 GB to 7.64 GB) on the
first machine. On the second machine, it reduces it by 125 MB (1.94%,
from 6.31 GB to 6.19 GB).

Reviewed By: #lld-macho, int3

Differential Revision: https://reviews.llvm.org/D113818


Compare: https://github.com/llvm/llvm-project/compare/b715b79d54d5...93bf271f2743


More information about the All-commits mailing list