Skip to content

[LlvmIrGenerator] emit c"..." literals for string blobs - #12280

Merged
jonathanpeppers merged 6 commits into
mainfrom
jonathanpeppers-llvm-ir-cstring-blobs
Aug 3, 2026
Merged

[LlvmIrGenerator] emit c"..." literals for string blobs#12280
jonathanpeppers merged 6 commits into
mainfrom
jonathanpeppers-llvm-ir-cstring-blobs

Conversation

@jonathanpeppers

Copy link
Copy Markdown
Member

Stacked on #12279 — review that first; this PR's diff is one file, LlvmIrGenerator.cs.

WriteStringBlobArray() emitted one LLVM IR element per byte of the type map string blobs:

@type_map_managed_type_names = ... constant [3035518 x i8] [
	; 'Android.OS.AsyncTask' @ 0
	i8 u0x41, i8 u0x6e, ...
]

For a hello world MAUI app that's ~3.6 million elements and a 52.6MB typemaps.arm64-v8a.ll, which is slow to write and much slower for llc to consume.

Emit a single constant byte string instead — the same form the generator already uses for constant string literals:

@type_map_managed_type_names = ... constant [3035518 x i8] c"Android.OS.AsyncTask\00..."

Bytes stream through a pooled char [] rather than millions of single-character TextWriter writes.

Results

dotnet new maui -sc, Debug, clean build, -nodeReuse:false. The machine is shared and noisy, so raw per-run values are shown rather than a single median:

Metric Before After
typemaps.arm64-v8a.ll 52.6MB 15.2MB
CompileNativeAssembly (llc) 2239, 2287, 2177ms 224, 168, 203, 201, 221, 203ms
GenerateTypeMappings 1489, 1649, 1414ms 645, 1187, 1206, 543, 576, 643ms

CompileNativeAssembly is the headline: ~10x faster, with no overlap between the before and after ranges. That directly addresses the CompileNativeAssembly regression seen in the CoreCLR-vs-Mono binlog comparison that motivated this work.

GenerateTypeMappings is genuinely faster — every "after" run beats every "before" run — but it's noisy enough (543-1206ms across identical binaries) that I won't quote a percentage.

LinkApplicationSharedLibraries and total build time are unchanged. The linker consumes a byte-identical .o, so there is nothing there to win.

Correctness

typemaps.arm64-v8a.o is byte-for-byte identical before and after (SHA256 73E4A982E8C8098FB7909EA550CDEE392E19AAB6938E145A4C362BE0F0417DCF, 4161824 bytes).

One caveat for anyone reproducing this: the -ci.<branch>.<n> SDK version string is embedded in the assembly names blob and changes on every SDK rebuild, so .o hashes only compare within a matched before/after pair.

Additional checks:

  • All three globals decode from the .ll to exactly their declared [N x i8] length — type_map_assembly_names 2707, type_map_managed_type_names 3035518, type_map_java_type_names 592060 — and the decoded bytes appear verbatim in the .o.
  • All 28,225 i32 offsets in the file (16,567 java + 11,658 managed) resolve correctly against the decoded blobs, zero unresolved. This matters because those offsets index into the blobs.
  • Microsoft.Android.Build.BaseTasks-Tests 129/129, LlvmIrGeneratorTests 9/9, GenerateNativeApplicationConfigSourcesTests 4/4. The last of those runs llc on a generated .ll and parses the resulting .s, so it round-trips a string blob end to end.

Nothing parses blob contents out of the .ll, so no test changes were needed. TrimmableTypeMapBuildTests only matches the array type ([N x), which is unchanged.

Tradeoffs

  • Per-string ; 'Foo' @ 123 comments are gone. A constant string literal has to fit on a single line and ; comments extend to the end of the line, so they can't be interleaved. They were also a large fraction of the file size.
  • The managed type names line is ~3MB wide. llc handles it fine; text editors and diff tools will not enjoy it.
  • 15.2MB, not ~5MB. The remaining bulk is the i32 offset/index arrays, which this change doesn't touch — the obvious next target.
  • This removes the last caller of HexUtilities.WriteHex (TextWriter, ...) that [LlvmIrGenerator] ~4x faster type map generation #12279 added for the old hot path. The overload is public and unit-tested, so it's left in place.

Squash on merge — the 4 commits are kept separate for readability.

Copilot AI review requested due to automatic review settings July 31, 2026 17:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates LlvmIrGenerator to emit type-map string blobs as LLVM IR constant byte strings (c"...") instead of per-byte i8 u0xNN array elements, dramatically reducing .ll size and improving llc compile time for large type maps.

Changes:

  • Reworked WriteStringBlobArray() to stream a single c"..." literal for the blob, escaping bytes as needed.
  • Switched the hot output path to use a pooled char[] chunk buffer to avoid millions of tiny TextWriter writes.
  • Removed per-string inline comments in blob emission (no longer representable within a single-line constant string literal).

Comment thread src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrGenerator.cs Outdated
@jonathanpeppers

Copy link
Copy Markdown
Member Author

/review

@github-actions

github-actions Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Android PR Reviewer completed successfully!

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM

Solid, well-scoped performance change to WriteStringBlobArray. Emitting a single c"..." byte-string constant instead of millions of i8 u0xNN array elements is the right approach, and the reasoning is clearly documented in the method comment.

Correctness review (independent):

  • LLVM IR escaping is correct: ", \, and everything outside printable ASCII [32,127) are hex-escaped; 127 (DEL) and 0 (NUL) fall through to the escape path as expected.
  • Buffer bounds are safe: the literal path flushes at chunkUsed == chunkCapacity (needs 1 char) and the escape path at chunkUsed + 3 > chunkCapacity (needs \ + 2 hex). After Flush() the 4096-char rented buffer always has room, so no overflow of chunk.AsSpan(chunkUsed, 2).
  • Per-string terminating NUL is preserved (WriteByte(0)), so total byte count still matches the declared [N x i8] length.
  • ArrayPool<char> rent/return is correctly wrapped in try/finally, and using chunk.Length as the capacity handles Rent over-allocation.
  • No test impact: EnvironmentHelper.ReadStringBlob parses the llc-produced .asciz output, which is downstream of the .ll and byte-identical per the PR's .o hash verification.

Verification: The PR body's byte-identical .o (SHA256 match) plus offset-resolution and unit-test checks are exactly the right evidence for a change like this — thank you for the thoroughness.

Two non-blocking 💡 suggestions inline (magic-number naming; uppercase-hex cosmetic note). Neither blocks merge.

CI note: at review time the pipeline shows pending with no completed checks — confirm the dotnet-android build/tests go green before merging.

Generated by Android PR Reviewer for #12280 · 91.4 AIC · ⌖ 18.7 AIC · ⊞ 6.9K
Comment /review to run again

Comment thread src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrGenerator.cs Outdated
Comment thread src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrGenerator.cs Outdated
Base automatically changed from jonathanpeppers-silver-spork to main August 3, 2026 11:06
jonathanpeppers and others added 5 commits August 3, 2026 06:08
`WriteStringBlobArray()` emitted one LLVM IR element per byte of the type map
string blobs:

    @type_map_managed_type_names = ... constant [3035518 x i8] [
    	; 'Android.OS.AsyncTask' @ 0
    	i8 u0x41, i8 u0x6e, ...
    ]

For a hello world MAUI app that is ~3.6 million elements, producing a 52.6MB
`typemaps.arm64-v8a.ll`, which is slow both to write and for `llc` to consume.

Emit a single constant byte string instead, which is the same form the
generator already uses for constant string literals:

    @type_map_managed_type_names = ... constant [3035518 x i8] c"Android.OS.AsyncTask\00..."

Bytes are streamed through a pooled `char[]` chunk buffer to avoid millions of
single-character `TextWriter` writes.

A constant string literal has to fit on a single line, and `;` comments extend
to the end of the line, so the per-string `; 'Foo' @ 123` comments are gone.

Results for a hello world MAUI app (`dotnet new maui -sc`, Debug, median of 3):

  * `typemaps.arm64-v8a.ll`: 52.6MB -> 15.2MB
  * `GenerateTypeMappings`: 1489ms -> 1187ms
  * `CompileNativeAssembly`: 2239ms -> 203ms

The resulting `typemaps.arm64-v8a.o` is byte-for-byte identical to the one
produced before this change.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: f6a038cc-e09a-455d-8f08-1ea17ecfd849
ArrayPool<T>.Rent() may hand back an array larger than requested.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: f6a038cc-e09a-455d-8f08-1ea17ecfd849
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: f6a038cc-e09a-455d-8f08-1ea17ecfd849
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: f6a038cc-e09a-455d-8f08-1ea17ecfd849
Makes the `exactly two hex digits` contract obvious at the call site.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: f6a038cc-e09a-455d-8f08-1ea17ecfd849
@jonathanpeppers
jonathanpeppers force-pushed the jonathanpeppers-llvm-ir-cstring-blobs branch from e11fcc3 to 6a1bce2 Compare August 3, 2026 11:10
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: f6a038cc-e09a-455d-8f08-1ea17ecfd849
@jonathanpeppers jonathanpeppers added the ready-to-review This PR is ready to review/merge, I think any CI failures are just flaky (ignorable). label Aug 3, 2026
@jonathanpeppers
jonathanpeppers merged commit a51c46a into main Aug 3, 2026
44 checks passed
@jonathanpeppers
jonathanpeppers deleted the jonathanpeppers-llvm-ir-cstring-blobs branch August 3, 2026 19:04
jonathanpeppers added a commit that referenced this pull request Aug 3, 2026
Stacked on #12280.

Two more LLVM IR generator wins found by profiling a `dotnet new maui -sc` app (Debug, CoreCLR, `arm64-v8a`) with `dotnet-trace` against the MSBuild *worker* node.

## 1. Cache `StructureMemberInfo.IsIRStruct`

`IsIRStruct()` was an extension method that walked the member's type on every call, and it is called once per member per array element. In the trace it was **345 ms of the 550 ms** spent in `TypeMapGenerator.GenerateNativeAssembly`:

```
TypeMapGenerator.GenerateNativeAssembly                       549.9
└─ LlvmIrGenerator.Generate                                   517.8
   └─ WriteGlobalVariables → WriteArrayEntries                438.6
      ├─ TypeUtilities.IsIRStruct                             345.1   <-- 63%
      │  └─ TypeUtilities.IsStructure                         282.0
      └─ WriteType                                            298.2
```

The answer depends only on the member, which is immutable, so it's now computed once in the `StructureMemberInfo` constructor.

`TypeUtilities.IsIRStruct` and `TypeUtilities.IsStructure` are **entirely absent** from the frame table of a trace captured after the change. `.ll` and `.o` are byte-identical.

## 2. `$(_AndroidEmitLlvmIrComments)`, off by default

Comments in the generated IR are purely descriptive — `llc` ignores them — but they are more than half the file:

| File | comments on | comments off (new default) |
| --- | --- | --- |
| `typemaps.arm64-v8a.ll` | 15,181,829 | **7,631,959** (−49.7%) |
| `typemaps.arm64-v8a.o` | 4,161,824 | 4,161,824 |

New private property `$(_AndroidEmitLlvmIrComments)`, blank (off) by default, flows into `<GenerateTypeMappings/>`, `<GenerateNativeApplicationConfigSources/>` and `<GenerateCompressedAssembliesNativeSourceFiles/>`. Set it to `true` to get the old output back when debugging the generators.

Where a comment used to terminate a line of real content, an explicit newline is written instead, and the hot paths skip *building* the comment string rather than just skipping the write.

`GenerateTypeMappings`, 12 interleaved on/off runs on the same app, raw ms:

```
on   733, 793, 807, 839, 841, 848
off  679, 689, 689, 696, 721, 746
```

Roughly 100 ms, same direction in every pair. It is **not** visible in wall clock (~6.1 s either way) — this machine can't resolve 100 ms end to end, so please don't read more into it than the paired ordering.

## Correctness

Matched pair, same SDK binary, only the property flipped:

* `typemaps.arm64-v8a.o` byte-identical — SHA256 `230894AF63A583A8D2FCAC18C9AA962131A815EFCC307704B6D1E96887A837A0`
* all three string blobs byte-identical
* 132,537 `i32` values in an identical sequence
* every non-sentinel offset still resolves to the start of a nul-terminated string in its blob; the 4,909 that don't are all the `0xFFFFFFFF` sentinel

Worth calling out: the `; from:` / `; to:` annotations that go away by default are exactly what that offset check reads. It was run *before* flipping the default, and `$(_AndroidEmitLlvmIrComments)=true` exists so it can be run again.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@github-actions github-actions Bot locked and limited conversation to collaborators Sep 3, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

ready-to-review This PR is ready to review/merge, I think any CI failures are just flaky (ignorable).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants