Use stackalloc in TypeMapHelper to reduce allocations#11133
Use stackalloc in TypeMapHelper to reduce allocations#11133jonathanpeppers merged 6 commits intomainfrom
Conversation
Replace Encoding.GetBytes() allocations with ArrayPool<byte>.Shared Rent/Return in a try-finally block. Extracted a shared HashString() helper that handles the pool lifecycle. HashBytes now takes ReadOnlySpan<byte> to properly slice the rented buffer to the actual byte count (since Rent may return a larger array). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR reduces build-time allocations in the Java type-map hashing path by replacing per-call Encoding.GetBytes(string) allocations with ArrayPool<byte>.Shared.Rent/Return in TypeMapHelper, which is invoked for every Java type name during type-map generation.
Changes:
- Replaced
Encoding.*.GetBytes(name)allocations with anArrayPool<byte>-backed buffer in a sharedHashString()helper. - Updated hashing to operate on
ReadOnlySpan<byte>so only the actual encoded byte range is hashed (excluding any extra capacity from pooled arrays).
Java type names are always ASCII and typically 20-100 characters, so the encoded byte count is well within stackalloc limits. Use the unsafe Encoding.GetBytes(char*, int, byte*, int) overload since the Span-based overload requires netstandard2.1+. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use stackalloc for buffers <= 512 bytes and fall back to ArrayPool for larger ones to avoid potential stack overflow with unusually long type names. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The tmt tool includes TypeMapHelper.cs which now uses unsafe code for the stackalloc + Encoding.GetBytes pointer overload. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/review |
|
✅ Android PR Reviewer completed successfully! |
There was a problem hiding this comment.
🤖 AI Review Summary
Verdict: ✅ LGTM
Clean, well-motivated allocation reduction. The unsafe approach is justified: TypeMapHelper.cs is compiled under netstandard2.0 (Xamarin.Android.Build.Tasks) where Encoding.GetBytes(ReadOnlySpan<char>, Span<byte>) isn't available, and the file is shared directly with tmt (a modern .NET TFM tool), so the unsafe overload is the right way to get zero-allocation encoding without conditional compilation noise.
Found 1 issue:
- 💡 Code organization: Magic number
256for the stackalloc threshold (TypeMapHelper.cs:42)
👍 Good call changing HashBytes to accept ReadOnlySpan<byte> — it works seamlessly with both the stackalloc and heap-allocated fallback. The 256-byte threshold is well-chosen: it comfortably handles real-world Java names in both Unicode (2 bytes/char → 128 char limit) and UTF-8 (1 byte/char → 256 char limit) encodings.
Note: CI is still in progress (macOS build running, main check queued). Confirm all checks pass before merging.
Review generated by android-reviewer from review guidelines.
Generated by Android PR Reviewer for issue #11133 · ● 523.5K
Each call to
Encoding.GetBytes(string)allocates a newbyte[]that is immediately discarded after hashing. SinceTypeMapHelperis called for every Java type name during the build, these short-lived allocations add up.This PR replaces those allocations with
stackalloc+ the unsafeEncoding.GetBytes(char*, int, byte*, int)overload (since the Span-based overload requires netstandard2.1+). For the rare case where encoded bytes exceed 256, it falls back to a plainnew byte[]allocation.Key details:
HashString()helper that bothHashJavaName(Unicode) andHashJavaNameForCLR(UTF8) delegate toHashBytesnow takesReadOnlySpan<byte>so it works with both stackalloc and heap-allocated buffersandroid/widget/TextView), typically 20-100 chars, so the stackalloc path handles virtually all real-world usage