Nul terminate rust string literals

[bjorn3]

This allows taking advantage of the C string merging functionality of linkers, reducing code size.

Marked as draft to see if this actually has much of an effect. The disadvantage of this is that people may start to rely on string literals getting nul terminated. A potential solution for that would be to put a byte that is not part of a valid UTF-8 character right before the nul terminator.

Builds on #138503

[rustbot]

r? @estebank

rustbot has assigned @estebank.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[bjorn3]

@bors try @rust-timer queue

[bors]

⌛ Trying commit 7c1cd9b with merge f949c9f...

[bors]

☀️ Try build successful - checks-actions
Build commit: f949c9f (f949c9f96faec0a24e98f55e36cfa88c14c2199c)

[rust-timer]

Finished benchmarking commit (f949c9f): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	1.2%	[0.3%, 1.8%]	5
Improvements ✅ (primary)	-1.2%	[-4.3%, -0.3%]	12
Improvements ✅ (secondary)	-1.5%	[-6.6%, -0.4%]	21
All ❌✅ (primary)	-1.2%	[-4.3%, -0.3%]	12

Max RSS (memory usage)

Results (primary 1.6%, secondary 2.0%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	2.2%	[1.1%, 4.4%]	4
Regressions ❌ (secondary)	3.7%	[1.9%, 4.8%]	3
Improvements ✅ (primary)	-1.2%	[-1.2%, -1.2%]	1
Improvements ✅ (secondary)	-3.3%	[-3.3%, -3.3%]	1
All ❌✅ (primary)	1.6%	[-1.2%, 4.4%]	5

Cycles

Results (primary -2.3%, secondary -3.2%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	2.4%	[2.3%, 2.6%]	2
Improvements ✅ (primary)	-2.3%	[-3.8%, -1.2%]	3
Improvements ✅ (secondary)	-5.5%	[-6.4%, -4.1%]	5
All ❌✅ (primary)	-2.3%	[-3.8%, -1.2%]	3

Binary size

Results (primary -0.5%, secondary -0.3%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.1%	[0.0%, 0.1%]	8
Regressions ❌ (secondary)	0.0%	[0.0%, 0.0%]	32
Improvements ✅ (primary)	-0.6%	[-2.5%, -0.0%]	100
Improvements ✅ (secondary)	-0.5%	[-3.0%, -0.0%]	43
All ❌✅ (primary)	-0.5%	[-2.5%, 0.1%]	108

Bootstrap: 775.364s -> 772.882s (-0.32%)
Artifact size: 365.01 MiB -> 365.44 MiB (0.12%)

[bors]

☔ The latest upstream changes (presumably #138503) made this pull request unmergeable. Please resolve the merge conflicts.

[oxalica]

This allows taking advantage of the C string merging functionality of linkers, reducing code size.

I don't like this. Why Rust is the one to pretend to be C in order to benefit from a (for no reason but) C-only optimization, instead of making that optimization more general to be applicable on Rust strings?

I got here by searching why Rust currently cannot merge "a long string" with any of its substrings like "long" or "a", which is an advantage of no-NUL-terminators and is more widely applicable for tiny strings with a single or two characters. But this PR seems to do the opposite to make it impossible.

[bjorn3]

Why Rust is the one to pretend to be C in order to benefit from a (for no reason but) C-only optimization, instead of making that optimization more general to be applicable on Rust strings?

Because this optimization needs to be done by the linker to be effective and there is no reasonable way for us to modify every linker a user may want to use to support string merging for non-C strings. For Windows and macOS I think it is unrealistic to expect this will be added within the next 10 years. The Windows linker doesn't even support weak symbols despite being a relatively common C feature (it does seem to support weak aliases though). And Apple is betting om Swift rather than Rust. And on Linux we still support systems from before Rust 1.0 was released, with associated ancient linker.

[oxalica]

Because this optimization needs to be done by the linker to be effective and there is no reasonable way for us to modify every linker a user may want to use to support string merging for non-C strings. For Windows and macOS I think it is unrealistic to expect this will be added within the next 10 years. The Windows linker doesn't even support weak symbols despite being a relatively common C feature (it does seem to support weak aliases though). And Apple is betting om Swift rather than Rust. And on Linux we still support systems from before Rust 1.0 was released, with associated ancient linker.

I agree that it's better done in linkers and they are likely to only support C strings. But I think it's too early to just give up before any existing attempt to improve eg. lld. At least I couldn't find discussions under LLVM issues. Note that lld is already shipped with Rust toolchain and is used by default on some platforms (#124129).

Adding NUL for strings without the need for C-interop (unlike #135054) should be more conservative without proven to be required. This PR seems to add NUL for Rust type names, which is not for C-interop and also less applicable for suffix merging. When will you have both foo::Bar and another::foo::Bar? Perf report seems to agree with this.

I also tried to get some numbers on possible size reduction of librustc_driver's all .rodata* sections, by compile it to staticlib, parse all inner ELFs and run manual merging algorithms¹. This should be an upper-bound of size reduction because 1. all .rodata* subsections are subject to merge, not only strings; 2. linker may remove (gc) more unused sections. It seems that a simple dedup (currently being done) is good enough, and suffix and subarray (aka. substring) merging are probably not worth it, considering the total archive weighs 349MiB and the linked .so is still 128MiB.

copy .rodata*: 135428 sections, total bytes: 6638549
dedup simple: 42456 sections, total bytes: 3675613 (previous - 2.83MiB)
merge suffix: 37281 sections, total bytes: 3526647 (previous - 145.47KiB)
merge subarray: 28527 sections, total bytes: 3353123 (previous - 169.46KiB)

Footnotes

1. Code: https://gist.github.com/oxalica/c3a67f56486ad77772f9df72f703eaa4 I'm using online suffix tree for substring search on an increasing haystack. It's not performance-wise good, but is good enough in complexity (O(N)) so it can at least complete in seconds. ↩

[bors]

☔ The latest upstream changes (presumably #139766) made this pull request unmergeable. Please resolve the merge conflicts.