Skip to content

[TrimmableTypeMap] Use XxHash64 package naming by default with LowercaseCrc64 compatibility#11193

Draft
Copilot wants to merge 7 commits intomainfrom
copilot/update-hash-algorithm-to-xxhash64
Draft

[TrimmableTypeMap] Use XxHash64 package naming by default with LowercaseCrc64 compatibility#11193
Copilot wants to merge 7 commits intomainfrom
copilot/update-hash-algorithm-to-xxhash64

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 23, 2026

This updates trimmable type map generation to default to XxHash64-based package naming for unregistered types, while preserving backwards compatibility for projects that explicitly set:

<AndroidPackageNamingPolicy>LowercaseCrc64</AndroidPackageNamingPolicy>
  • Package naming policy support in trimmable typemap

    • Wired AndroidPackageNamingPolicy into trimmable typemap generation.
    • Added policy-aware hashing:
      • XxHash64 (trimmable default) → xx64...
      • LowercaseCrc64crc64... using the existing legacy/custom CRC64 implementation (same style used in existing typemap paths).
    • Kept Mono.Android namespace-preserving behavior unchanged.
  • Default policy update

    • XxHash64 is now the default only for trimmable typemap generation.
    • The global AndroidPackageNamingPolicy default remains LowercaseCrc64 for non-trimmable typemap paths.
  • Hashing implementation refactor

    • Moved scanner hashing logic out of JavaPeerScanner into a dedicated static ScannerHashingHelper for easier focused testing.
    • Kept allocation-conscious hashing/hex conversion behavior.
  • Targeted test updates

    • Kept/updated scanner expectations for xx64 defaults.
    • Added coverage to verify LowercaseCrc64 produces legacy crc64 names and differs from default XxHash64 output.
    • Added fixed-vector tests for legacy CRC64 known input/output pairs to validate backward compatibility behavior.
  • Related plumbing updates

    • Passed package naming policy through the trimmable typemap target/task into TrimmableTypeMapGenerator and scanner hashing logic.
return packageNamingPolicy switch {
	HashedPackageNamingPolicy.LowercaseCrc64 => "crc64" + ScannerHashingHelper.ToLegacyCrc64 (ns, assemblyName),
	_ => "xx64" + ScannerHashingHelper.ToXxHash64 (ns, assemblyName),
};

Copilot AI and others added 2 commits April 23, 2026 07:33
Agent-Logs-Url: https://github.com/dotnet/android/sessions/425dd1fe-e6a4-43e0-b0a9-de6b1e343c11

Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>
Agent-Logs-Url: https://github.com/dotnet/android/sessions/425dd1fe-e6a4-43e0-b0a9-de6b1e343c11

Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>
Copilot AI changed the title [WIP] Use xxhash64 instead of crc64 at build time [TrimmableTypeMap] Use XxHash64 for auto-generated Java package hashing Apr 23, 2026
Copilot AI requested a review from simonrozsival April 23, 2026 07:38
@simonrozsival
Copy link
Copy Markdown
Member

@copilot I would prefer changing the prefix from crc64 to xx64 to reflect the change.

Agent-Logs-Url: https://github.com/dotnet/android/sessions/b04a9e6a-bf80-4ff1-8601-4078c145728e

Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 23, 2026

@copilot I would prefer changing the prefix from crc64 to xx64 to reflect the change.

Updated in 1049678: auto-generated TrimmableTypeMap package names now use the xx64 prefix (with XxHash64), and related unit/integration test expectations were updated accordingly.

Copy link
Copy Markdown
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

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

Note that there is the property:

<AndroidPackageNamingPolicy>LowercaseCrc64</AndroidPackageNamingPolicy>

We will probably want to introduce a new one for XxHash64.

It is a slight breaking change, because people could have a hardcoded package name in a script for launching their main activity, etc.

So, I think what we did in some really old Xamarin.Android release, we changed the default but allowed projects to still use the old one.

@simonrozsival
Copy link
Copy Markdown
Member

It is a slight breaking change, because people could have a hardcoded package name in a script for launching their main activity, etc.

So funny thing is that the CSC64 we have in our C++(or C#?) code and CSC64 that is in System.IO.Hashing produce different results. I decided to use the System.IO.Hashing one in the trimmable typemap so that would be a breaking change anyway. I will need to revisit this and try to make the LowercaseCrc64 option be consistent with previous naming. The scenario with launching a specific Activity makes sense.

@simonrozsival
Copy link
Copy Markdown
Member

@copilot let's respect <AndroidPackageNamingPolicy>LowercaseCrc64</AndroidPackageNamingPolicy> with a new (default) value of XxHash64. For backwards compatibility, let's use the custom Crc64 implementation that is in existing C# or C++ code used by the "llvm-ir" typemap

…efault

Agent-Logs-Url: https://github.com/dotnet/android/sessions/a90e2abd-ca59-4b98-a498-64caa983b1d0

Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 23, 2026

@copilot let's respect <AndroidPackageNamingPolicy>LowercaseCrc64</AndroidPackageNamingPolicy> with a new (default) value of XxHash64. For backwards compatibility, let's use the custom Crc64 imple...

Implemented in ec92067: trimmable typemap now respects AndroidPackageNamingPolicy, defaults to XxHash64, and uses the legacy custom CRC64 path when policy is LowercaseCrc64 for backwards compatibility.

Note that there is the property:

<AndroidPackageNamingPolicy>LowercaseCrc64</AndroidPackageNamingPolicy>

...

Addressed in ec92067: added trimmable support for AndroidPackageNamingPolicy with a new default of XxHash64, while keeping LowercaseCrc64 compatibility via the existing legacy CRC64 implementation.

Copilot AI changed the title [TrimmableTypeMap] Use XxHash64 for auto-generated Java package hashing [TrimmableTypeMap] Use XxHash64 package naming by default with LowercaseCrc64 compatibility Apr 23, 2026
Comment on lines +1517 to +1519
var data = System.Text.Encoding.UTF8.GetBytes (value);
var hash = System.IO.Hashing.XxHash64.Hash (data);
return BitConverter.ToString (hash).Replace ("-", "").ToLowerInvariant ();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@copilot we should take inspiration from dotnet/android-tools#335 and avoid so many allocations here - use stackalloc where possible and non-allocating method overloads. Make sure to reuse the ToHexString method which is a lot more efficient than what we have right now. Also I don't think we need to hash UTF-8, we can simply do ReadOnlySpan<byte> utf16Bytes = MemoryMarshal.AsBytes(text.AsSpan()); , right? We hash a lot of strings during the build process, so let's make sure we don't unnecessarily slow down builds.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Implemented in 8d5709b: reduced allocations in JavaPeerScanner by switching to span-based hex formatting (with stackalloc), using non-allocating XxHash64.Hash(ReadOnlySpan<byte>, Span<byte>), and removing intermediate "{ns}:{assembly}" string allocations via pooled UTF-8 buffers. I kept UTF-8 input for hash compatibility with existing outputs/policies.

Copy link
Copy Markdown
Member

@simonrozsival simonrozsival left a comment

Choose a reason for hiding this comment

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

@copilot address these comments

I want to move the hashing code from JavaPeerScanner to a separate static ScannerHashingHelper class which can be better tested. I want a test that will just simply validate some known inputs to some known outputs of the legacy crc64 algorithm to prove we're maintaining backwards compatibility.

<AndroidVersionCodePattern Condition=" '$(AndroidUseLegacyVersionCode)' != 'True' And '$(AndroidVersionCodePattern)' == '' ">{abi}{versionCode:D5}</AndroidVersionCodePattern>
<AndroidResourceGeneratorTargetName>UpdateGeneratedFiles</AndroidResourceGeneratorTargetName>
<AndroidUseApkSigner Condition=" '$(AndroidUseApkSigner)' == '' ">True</AndroidUseApkSigner>
<AndroidPackageNamingPolicy Condition=" '$(AndroidPackageNamingPolicy)' == '' ">LowercaseCrc64</AndroidPackageNamingPolicy>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

the XxHash64 should be the default just for the trimmable typemap

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in 05cebc7: reverted the global AndroidPackageNamingPolicy default and scoped XxHash64 as the default specifically for trimmable typemap generation.

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 24, 2026

@copilot address these comments

I want to move the hashing code from JavaPeerScanner to a separate static ScannerHashingHelper class which can be better tested. I want a test that will just simply va...

Implemented in 05cebc7: moved scanner hashing into ScannerHashingHelper and added fixed-vector tests that validate known legacy CRC64 outputs to lock backward compatibility behavior.

@copilot let's respect <AndroidPackageNamingPolicy>LowercaseCrc64</AndroidPackageNamingPolicy> with a new (default) value of XxHash64. For backwards compatibility, let's use the custom Crc64 imple...

Handled in 05cebc7: trimmable typemap continues to respect explicit <AndroidPackageNamingPolicy>LowercaseCrc64</AndroidPackageNamingPolicy>, while using XxHash64 as the trimmable default and keeping the legacy custom CRC64 path for compatibility.

@copilot I would prefer changing the prefix from crc64 to xx64 to reflect the change.

xx64 is used for the new hash-based default path, while crc64 remains only for explicit LowercaseCrc64 compatibility. Confirmed in 05cebc7.

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.

[TrimmableTypeMap] Use xxhash64 instead of crc64 at build time

3 participants