Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Switch 64-bit hashes to xxhash3 #8721

Merged
merged 19 commits into from
Apr 3, 2024
Merged

Switch 64-bit hashes to xxhash3 #8721

merged 19 commits into from
Apr 3, 2024

Conversation

grendello
Copy link
Contributor

@grendello grendello commented Feb 14, 2024 •
edited
Loading

Performance improvements compared to main, tests with Android 14 on Pixel 6 Pro:

  • Displayed time: 2.17% faster
  • Native-to-managed init: 1.4% faster
  • Total native init: 1.15% faster

@grendello
Add xxHash submodule, v0.8.2
0e0f064
@grendello
Add as submodule for C++ compile-time xxh3 implementation
8d9ca39
@grendello
The beginnings
9a3a2ec
@grendello
Merge branch 'main' into dev/grendel/xxhash3
b7e4d3b 
* main: (116 commits)
  [tmt] Update to work with current `libxamarin-app.so` (#8694)
  [Xamarin.Android.Build.Tasks] remove `$(AndroidSupportedAbis)` from `build.props` (#8717)
  [Xamarin.Android.Build.Tasks] BannedApiAnalyzers for Resolve() (#8715)
  Bump to xamarin/Java.Interop/main@dfcbd670 (#8714)
  [monodroid] C++ tweaks and legacy code cleanup (#8638)
  Bump to xamarin/xamarin-android-tools/main@a698a33 (#8710)
  [readme] Add `d17-8` download links. (#8709)
  Bump external/Java.Interop from `07c7300` to `7f08b77` (#8702)
  Bump to xamarin/monodroid@848d1277b7 (#8691)
  [Xamarin.Android.Build.Tasks] `FixAbstractMethodsStep` performance (#8650)
  Bump to dotnet/installer@fb7b9a4b9e 9.0.100-preview.2.24106.6 (#8700)
  [Mono.Android] Cache `Profiles/api-34.xml` contents (#8679)
  [monodroid] typemaps may need to load assemblies (#8625)
  Bump $(AndroidNetPreviousVersion) to 34.0.79 (#8693)
  Bump to xamarin/java.interop/main@07c73009 (#8681)
  Bump to dotnet/installer@1c496970b7 9.0.100-preview.2.24078.1 (#8685)
  [GetAndroidDependencies] Add Jdk dependency info (#8651)
  [xaprepare] Add support for newer SparkyLinux (#8684)
  Bump to dotnet/installer@5680e93cb2 9.0.100-preview.2.24073.12 (#8666)
  $(AndroidPackVersionSuffix)=preview.2; net9 is 34.99.0.preview.2 (#8678)
  ...
@grendello
Fix the build
4f1cb16
@grendello
Merge branch 'main' into dev/grendel/xxhash3
d3b8879 
* main:
  Bump to xamarin/Java.Interop/main@b8f6f88 (#8718)
  [Microsoft.Android.Sdk.ILLink] remove Java.Interop.Export reference (#8719)
@grendello
Update after merge
876b884
@grendello
Merge branch 'main' into dev/grendel/xxhash3
89a462c 
* main:
  [xabuild] Remove xabuild, unused pipelines (#8720)
  Bump to xamarin/LibZipSharp/3.1.1@7abbbf4 (#8723)
  Bump to xamarin/java.interop/main@7d1e705 (#8722)
@grendello
Fix p/invoke tables check script
593bb94
@grendello
Avoid a warning
b86a1c5
@grendello grendello marked this pull request as ready for review February 16, 2024 18:06
@grendello grendello requested a review from jonpryor as a code owner February 16, 2024 18:06
@jpobst
Copy link
Contributor

jpobst commented Feb 27, 2024

I don't know if we only use this from managed code, but I recently saw that the .NET Team maintains a NuGet with these algorithms:
https://www.nuget.org/packages/System.IO.Hashing#readme-body-tab

@grendello
Copy link
Contributor Author

@jpobst we use the System.IO.Hashing nuget, that's where the xxHash3 APIs are from. We also use the algorithm at runtime in native code, but we have native implementation(s) there instead.

@grendello
Merge branch 'main' into dev/grendel/xxhash3
ab8db9b 
* main:
  Bump to xamarin/xamarin-android-tools/main@37d79c9 (#8752)
  Bump to dotnet/installer@d070660282 9.0.100-preview.3.24126.2 (#8763)
  Bump to xamarin/java.interop/main@14a9470 (#8766)
  $(AndroidPackVersionSuffix)=preview.3; net9 is 34.99.0.preview.3 (#8765)
  [Mono.Android] Do not dispose request content stream in AndroidMessageHandler (#8764)
  Bump com.android.tools:r8 from 8.2.42 to 8.2.47 (#8761)
  [Mono.Android] fix a set of the "easiest" trimmer warnings (#8731)
  Bump to dotnet/installer@0a73f814e1 9.0.100-preview.2.24122.3 (#8716)
  [ci] Always run the MAUI test job (#8750)
  Add a property required by #8478 (#8749)
  [xamarin-android-tools] import $(LibZipSharpVersion) value (#8738)
  Bump to xamarin/Java.Interop/main@c825dcad (#8701)
  Bump to xamarin/monodroid@cb01503327 (#8742)
  Bump to xamarin/Java.Interop/main@ae65609 (#8744)
  Bring in changes from PR #8478 (#8727)
  [xaprepare] Make 7zip work with "dangerous" symlinks in ZIPs (#8737)
  Bump NDK to r26c (#8732)
  Debugging MSBuild Tasks (#8730)
@grendello
Merge branch 'main' into dev/grendel/xxhash3
1a8e48c 
* main:
  [Mono.Android] Fix race condition in AndroidMessageHandler (#8753)
  [ci] Fix SDL Sources Analysis for PRs from forks (#8785)
  [ci] Add 1ESPT override to MSBuild test stages (#8784)
  [ci] Do not use @self annotation for templates (#8783)
  [ci] Migrate to the 1ES template (#8747)
  [Mono.Android] fix trimming warnings, part 2 (#8758)
  [Xamarin.Android.Build.Tasks] set `%(DefineConstantsOnly)` for older API levels (#8777)
  [tests] fix duplicate sources in `NuGet.config` (#8772)
  Bump to xamarin/monodroid@e13723e701 (#8771)
@grendello
Merge branch 'main' into dev/grendel/xxhash3
f85a6ae 
* main:
  [templates] Remove redundant "template" from display name. (#8773)
  Bump to xamarin/Java.Interop/main@a7e09b7 (#8793)
  [build] Include MIT license in most NuGet packages (#8787)
  Bump to dotnet/installer@893b762b6e 9.0.100-preview.3.24153.2 (#8782)
  [docs] update notes about `dotnet-trace` and `dotnet-gcdump` (#8713)
@grendello
Merge branch 'main' into dev/grendel/xxhash3
9281385 
* main:
  [Mono.Android] Fix missing enum issues that cause BG8800 warnings. (#8707)
  Bump external/Java.Interop from `3436a30` to `5bca8ad` (#8803)
  Bump to xamarin/monodroid@77124dc1 (#8804)
  Bump to dotnet/installer@e911f5c82c 9.0.100-preview.3.24161.2 (#8802)
  Bump to xamarin/Java.Interop/main@3436a30 (#8799)
@grendello
Merge branch 'main' into dev/grendel/xxhash3
c7c3610 
* main:
  [Mono.Android] is now "trimming safe" (#8778)
@grendello
This one too
fae658b
jonathanpeppers
jonathanpeppers approved these changes Mar 13, 2024
View reviewed changes
Copy link
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.

The managed side looks ok to me, we already ship System.IO.Hashing.dll so should be fine to use:

(Probably other review needed for the native code)

@grendello
Merge branch 'main' into dev/grendel/xxhash3
7c03882 
* main:
  [runtime] Remove the last vestiges of desktop builds (#8810)
  [ci] Don't auto-retry APK test suites. (#8811)
  [Microsoft.Android.Templates] Update EN l10n template strings (#8808)
  Bump to xamarin/Java.Interop/main@651de42 (#8809)
@grendello
Merge branch 'main' into dev/grendel/xxhash3
4e2d8d3 
* main:
  LEGO: Merge pull request 8818
  Bump to dotnet/installer@b40c44502d 9.0.100-preview.3.24165.20 (#8817)
  Bump com.android.tools:r8 from 8.2.47 to 8.3.37 (#8816)
  [Mono.Android] Prevent NullPointerException in TranslateStackTrace (#8795)
  Localized file check-in by OneLocBuild Task (#8815)
  [Xamarin.Android.Build.Tasks] Make all assemblies RID-specific (#8478)
  Localized file check-in by OneLocBuild Task (#8813)
  [Xamarin.Android.Build.Tasks] %(AndroidAsset.AssetPack) Support (#8631)
@grendello
Copy link
Contributor Author

/azp run

@grendello
Copy link
Contributor Author

@dellis1972 it seems GH is confused, the diff I see doesn't mention asset packs at all

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jonpryor
Copy link
Member

@grendello: please also update the THIRD-PARTY-NOTICES.TXT generation code to include these repositories and licenses.

@grendello grendello merged commit 6d2a4bb into main Apr 3, 2024
48 checks passed
@grendello grendello deleted the dev/grendel/xxhash3 branch April 3, 2024 09:32
grendello added a commit that referenced this pull request Apr 3, 2024
@grendello
Merge branch 'main' into dev/grendel/extended-timing
418279d 
* main:
  Switch 64-bit hashes to xxhash3 (#8721)
  Bump to xamarin/monodroid@fefc158ef3 (#8850)
  Bump binutils to 17.0.6-7.2.1 (#8806)
grendello added a commit that referenced this pull request Apr 3, 2024
@grendello
Merge branch 'main' into dev/grendel/blazor-hang
35bffea 
* main:
  Switch 64-bit hashes to xxhash3 (#8721)
  Bump to xamarin/monodroid@fefc158ef3 (#8850)
  Bump binutils to 17.0.6-7.2.1 (#8806)
@github-actions github-actions bot locked and limited conversation to collaborators May 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@dellis1972 dellis1972 dellis1972 left review comments

@jonathanpeppers jonathanpeppers jonathanpeppers approved these changes

@jonpryor jonpryor Awaiting requested review from jonpryor jonpryor is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@grendello @jpobst @jonpryor @dellis1972 @jonathanpeppers