Skip to content

[Xamarin.Android.Build.Tasks] memoize LlvmIrGenerator basic type writes - #12286

Closed
jonathanpeppers wants to merge 1 commit into
jonathanpeppers-build-perf-coreclr-round2from
jonathanpeppers-build-perf-resolveaars
Closed

[Xamarin.Android.Build.Tasks] memoize LlvmIrGenerator basic type writes#12286
jonathanpeppers wants to merge 1 commit into
jonathanpeppers-build-perf-coreclr-round2from
jonathanpeppers-build-perf-resolveaars

Conversation

@jonathanpeppers

Copy link
Copy Markdown
Member

Memoizes the basic-scalar fast path in LlvmIrGenerator.WriteType. The method's own comment says it "runs millions of times for a typical application" — a MAUI app's typemaps.arm64-v8a.ll is 7.6 MB / 232k lines with 16,567 typemap entries, and every scalar member of every array entry lands here.

Each call did 3 dictionary lookups (basicTypeMap.ContainsKeyMapToIRTypeIsNativePointer + basicTypeMap.TryGetValue) plus an LlvmTypeInfo allocation. All of it is a pure function of the Type: MapToIRType is documented as target-independent, and a generator instance has a fixed target, so the (IRType, LlvmTypeInfo) pair is cached per generator instance. Steady state: 1 lookup, 0 allocations.

Structural evidence — dotnet-trace, MAUI source-change rebuild, matched pair

Inclusive ms inside GenerateTypeMappings:

frame before after
GetActualType 303.9 frame absent
MapToIRType 307.9 3.8
WriteType (context, Type, …) 305.9 58.2
WriteStructureValue 313.6 183.3
LlvmIrGenerator.Generate 379.9 286.8
TypeMapGenerator.GenerateNativeAssembly 429.8 330.5

GetActualType no longer appears in the trace's frame table at all — proof the call was eliminated, independent of timing.

Wall clock — honest read: below the noise floor

Interleaved A/B, 6 pairs, source-change rebuild of a dotnet new maui -sc app, -nodeReuse:false. GenerateTypeMappings task, raw ms:

pair before after delta
1 743.4 725.1 −18.3
2 736.6 787.3 +50.7
3 783.2 749.3 −33.9
4 746.8 686.3 −60.5
5 737.6 1301.2 +563.6
6 1488.0 1302.7 −185.3

Pairs 5–6 were hit by unrelated machine load. The ranges fully overlap; I am not claiming a wall-clock win. The trace numbers are inclusive time under a profiler and overstate the real saving. The defensible claim is the structural one: ~200k dictionary lookups and ~200k allocations removed per build.

Correctness

446 generated outputs hashed (SHA256), sorted, diffed against a matched baseline captured immediately before the binary swap: all identical.

  • 368 .java Java Callable Wrappers
  • 10 .ll (both typemaps.*, environment.*, jni_remap.*, marshal_methods.*, compressed_assemblies.*)
  • AndroidManifest.xml ×2, acw-map.txt

Tests: Microsoft.Android.Build.BaseTasks-Tests 129/129 pass; Xamarin.Android.Build.Tests --filter LlvmIr 9/9 pass.

Squash on merge.

`LlvmIrGenerator.WriteType (context, Type, ...)` has a fast path for basic
scalar types.  Its own comment notes it "runs millions of times for a typical
application" -- a MAUI app's `typemaps.arm64-v8a.ll` is 7.6MB / 232k lines with
16,567 typemap entries, and every scalar member of every array entry goes
through it.

Each call did three dictionary lookups (`basicTypeMap.ContainsKey`, then
`MapToIRType` -> `IsNativePointer` + `basicTypeMap.TryGetValue`) plus a
`LlvmTypeInfo` allocation, all of which produce the same result for a given
`Type`.  `MapToIRType` is documented as target-independent, and a generator
instance has a fixed `target`, so the pair can be memoized per generator
instance.  Steady state is now one dictionary lookup and no allocation.

Structural evidence, `dotnet-trace` of a MAUI source-change rebuild
(inclusive ms, same machine, matched pair):

| frame                              | before | after |
| ---------------------------------- | ------ | ----- |
| `GetActualType`                    |  303.9 | (gone)|
| `MapToIRType`                      |  307.9 |   3.8 |
| `WriteType (context, Type, ...)`   |  305.9 |  58.2 |
| `LlvmIrGenerator.Generate`         |  379.9 | 286.8 |
| `TypeMapGenerator.GenerateNativeAssembly` | 429.8 | 330.5 |

All 446 generated outputs (368 `.java`, 10 `.ll` incl. both typemaps,
`AndroidManifest.xml` x2, `acw-map.txt`) are byte-identical before and after.

Co-authored-by: Copilot App <[email protected]>
Copilot AI review requested due to automatic review settings July 31, 2026 19:46

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 optimizes the LLVM IR generation hot path in LlvmIrGenerator.WriteType by memoizing the IR type string and associated LlvmTypeInfo for basic scalar managed types, reducing repeated lookups and per-value allocations during typemap generation.

Changes:

  • Introduces a per-generator cache for basic scalar type write metadata to avoid repeated type mapping work in WriteType.
  • Updates WriteType’s fast path to reuse cached (IRType, LlvmTypeInfo) when emitting basic scalar types.
Suppressed comments (1)

src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrGenerator.cs:526

  • 💡 suggestion: After the cache miss, this branch still does a ContainsKey lookup and then calls GetIRType, which redoes basicTypeMap.TryGetValue internally. You can collapse this into a single TryGetValue and avoid the extra lookup + GetActualType work for the first encounter of each basic scalar type.
			if (basicTypeMap.ContainsKey (type)) {
				string basicIRType = GetIRType (context, type, out ulong basicSize, out bool basicIsPointer);
				typeInfo = new LlvmTypeInfo (
					isPointer: basicIsPointer,
					isAggregate: false,

Comment on lines +102 to +112
sealed class BasicTypeWriteInfo
{
public readonly string IRType;
public readonly LlvmTypeInfo TypeInfo;

public BasicTypeWriteInfo (string irType, LlvmTypeInfo typeInfo)
{
IRType = irType;
TypeInfo = typeInfo;
}
}
@jonathanpeppers

Copy link
Copy Markdown
Member Author

Closing and removing this from the stack. It was the top of the chain (main -> #12279 -> #12280 -> #12285 -> #12286) and nothing was based on it, so no rebase is needed for the PRs below.

@jonathanpeppers

Copy link
Copy Markdown
Member Author

Recording a known redundancy found while profiling _GenerateJavaStubs. Not proposing a fix — filing it so it isn't lost.

The same .jlo.xml files are fully parsed twice per build, back to back, inside the same target:

  • GenerateJavaCallableWrappers.cs:72JavaObjectsXmlFile.Import (path, JavaObjectsXmlFileReadType.JavaCallableWrappers)
  • GenerateACWMap.cs:48JavaObjectsXmlFile.Import (path, JavaObjectsXmlFileReadType.AndroidResourceFixups)

Import does an unconditional XDocument.Load (filename) and then selects a different child element (jcw-types vs acw-types). So the readType flag avoids the deserialization of the unwanted half, but not the parse — the whole document is loaded both times.

Scale on a dotnet new maui -sc app:

count
.jlo.xml files 340
zero-length (skipped by the existing FileInfo.Length check) 256
actually parsed 84
bytes parsed, per pass 1.58 MB

So ~1.58 MB of XML is parsed twice.

The obvious fix — a process-wide static cache keyed on path + mtime — is not safe here. MSBuild node reuse is on by default for customers, so a static would persist across builds in a reused node and would have to get the invalidation exactly right; that class of cache is a known source of stale-output bugs. Sharing the parse would otherwise require merging or coupling two separate public tasks.

Worth knowing about; not worth fixing speculatively.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants