-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[LLD][MachO] Option to emit separate cstring sections #158720
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-lld-macho @llvm/pr-subscribers-lld Author: Ellis Hoag (ellishg) ChangesAdd the The reason this is useful is because strings in different sections might have different access patterns at runtime. By splitting these strings into separate sections, we may reduce the number of page faults during startup. For example, the ObjC runtime accesses all strings in Full diff: https://github.com/llvm/llvm-project/pull/158720.diff 9 Files Affected:
|
For reference, code emitting the |
lld/MachO/MapFile.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think performance is a concern here, since the number of entries in this map is not large.
Although I don't believe using this map impacts the determinism of the output, could you double-check that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for pointing out the iteration order. Instead I decided to store cstring sections in a map whose order is deterministic and use a map to point to the indices in the vector.
947444d
to
20e7d1a
Compare
firstTLVDataSection = nullptr; | ||
tar = nullptr; | ||
memset(&in, 0, sizeof(in)); | ||
in = InStruct(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think adding a vector and map to this struct made it no longer trivially copyable. This seems like a better way to clear it anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Maybe a little bit off-topic, but could we consider installing string tail merging for |
I wasn't aware of string tail merging. Thanks for sharing! I think it's definitely worth pursuing if it gives a size win. Do ObjC methods tend to have shares suffixes? I would assume they might share prefixes if two methods are the same but with different number of arguments. |
I just check a large binary and it seems |
I realized this broke the builds. I'll try to land a fix shortly. https://lab.llvm.org/buildbot/#/builders/190/builds/27994
|
Fix a test added in #158720. I had accidentally required arm64 when the test was using x86_64.
Fix a test added in llvm/llvm-project#158720. I had accidentally required arm64 when the test was using x86_64.
@nocchijiang I just published #161262 to tail merge cstring sections and it gives a nice win! Thanks again for pointing this out! |
I forgot to add a release note for llvm/llvm-project#158720 so I'll add it here.
I forgot to add a release note for llvm#158720 so I'll add it here.
Add the flag `--tail-merge-strings` to enable tail merging of cstrings. For example, if we have strings `mystring\0` and `ring\0`, we could place `mystring\0` at address `0x1000` and `ring\0` at address `0x1004` and have them share the same underlying data. It turns out that many ObjC method names can be tail merged. For example, `error:` and `doFoo:error:`. On a large iOS binary, we saw nearly a 15% size improvement in the `__TEXT__objc_methname` section and negligible impact on link time. ``` $ bloaty --domain=vm merged.o.stripped -- base.o.stripped VM SIZE -------------- +95% +5.85Ki [__TEXT] -2.4% -239Ki __TEXT,__cstring -14.5% -710Ki __TEXT,__objc_methname -1.0% -944Ki TOTAL ``` Tail merging for MachO was originally removed in 7c269db. The previous implementation used `StringTableBuilder`, but that was removed in 4308f03 to ensure deduplicated strings are aligned correctly. This implementation ensures that tail merged strings are also aligned correctly. Special thanks to nocchijiang for pointing this out in #158720 (comment). Depends on #161253.
Add the flag `--tail-merge-strings` to enable tail merging of cstrings. For example, if we have strings `mystring\0` and `ring\0`, we could place `mystring\0` at address `0x1000` and `ring\0` at address `0x1004` and have them share the same underlying data. It turns out that many ObjC method names can be tail merged. For example, `error:` and `doFoo:error:`. On a large iOS binary, we saw nearly a 15% size improvement in the `__TEXT__objc_methname` section and negligible impact on link time. ``` $ bloaty --domain=vm merged.o.stripped -- base.o.stripped VM SIZE -------------- +95% +5.85Ki [__TEXT] -2.4% -239Ki __TEXT,__cstring -14.5% -710Ki __TEXT,__objc_methname -1.0% -944Ki TOTAL ``` Tail merging for MachO was originally removed in llvm/llvm-project@7c269db. The previous implementation used `StringTableBuilder`, but that was removed in llvm/llvm-project@4308f03 to ensure deduplicated strings are aligned correctly. This implementation ensures that tail merged strings are also aligned correctly. Special thanks to nocchijiang for pointing this out in llvm/llvm-project#158720 (comment). Depends on llvm/llvm-project#161253.
Add the
--{no-}separate-cstring-literal-sections
option to emit cstring literals into sections defined by their section name. This allows for changes like swiftlang/swift#84300 and swiftlang/swift#84236 to actually have an affect. The default behavior has not changed.The reason this is useful is because strings in different sections might have different access patterns at runtime. By splitting these strings into separate sections, we may reduce the number of page faults during startup. For example, the ObjC runtime accesses all strings in
__objc_classname
before main.