Skip to content

Conversation

grendello
Copy link
Contributor

@grendello grendello commented Jun 25, 2025

Ignore assemblies whose names end in .ni and .ni.dll, when
a load request is made via the CoreCLR host contract callback.

For each CoreCLR will ask to first load Assembly.ni.dll, NI
standing for Native Image. This is something that was used on
Windows long ago and it never was, nor will it be, supported on
Unix.

This is mostly a cosmetic change in that it prevents a false
warning about a missing assembly from being logged in logcat.
This, in itself, is a startup time improvement since those
warnings would always be logged during every application launch,
with logcat logging being very expensive (and unpredictable).

The message about a missing .ni.dll is still logged, but with
the Debug severity, which will cause it to show up only if the
assembly logging category is enabled (it is disabled by default)

@grendello grendello requested a review from Copilot June 25, 2025 20:05
Copy link

@Copilot Copilot AI left a comment

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 improves performance for assembly load requests by early rejecting Native Image assemblies (those ending in .ni or .ni.dll) that are unsupported on Unix. Key changes include:

  • Replacing direct calls to mkdir with create_directory in runtime utility functions.
  • Adjusting logging levels and messages to better reflect the intended behavior when handling ignored assemblies.
  • Extending the assembly-store index and build task logic to incorporate ignored Native Image assemblies.

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/native/clr/runtime-base/util.cc Updated directory creation logic to use create_directory.
src/native/clr/include/runtime-base/android-system.hh Changed log level from warning to debug for directory creation.
src/native/clr/include/host/assembly-store.hh Added constant for identifying ignored index entries.
src/native/clr/host/assembly-store.cc Updated lookup logic to ignore assemblies marked to be skipped.
src/Xamarin.Android.Build.Tasks/Utilities/AssemblyStoreGenerator.cs Refactored index addition and enhanced manifest output for ignored entries.
src/Xamarin.Android.Build.Tasks/Utilities/AssemblyStoreBuilder.cs Modified builder to accept target runtime and add ignored Native Image assemblies.
src/Xamarin.Android.Build.Tasks/Tasks/CreateAssemblyStore.cs Updated task parameters to incorporate the target runtime and improved assembly processing.
src/Mono.Android/Java.Interop/TypeManager.cs Minor logging change to conditionally log loaded types.
Comments suppressed due to low confidence (2)

src/Xamarin.Android.Build.Tasks/Tasks/CreateAssemblyStore.cs:43

  • Ensure that the TargetRuntime MSBuild task input is always provided with a valid, non-empty value so that parsing does not fail at runtime.
		targetRuntime = MonoAndroidHelper.ParseAndroidRuntime (TargetRuntime);

src/native/clr/host/assembly-store.cc:215

  • The new check for ASSEMBLY_STORE_IGNORED_INDEX_ENTRY correctly short-circuits the load of ignored assemblies. Please verify that this change aligns with the overall logging and error reporting practices in the assembly store module.
		if (hash_entry->descriptor_index == ASSEMBLY_STORE_IGNORED_INDEX_ENTRY) {

@grendello grendello force-pushed the dev/grendel/clr-host-ignorable-assemblies branch from e59cb1a to 20ad19f Compare July 1, 2025 07:34
@grendello grendello marked this pull request as ready for review July 1, 2025 09:18
@grendello
Copy link
Contributor Author

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@grendello grendello merged commit aac5912 into main Jul 2, 2025
60 checks passed
@grendello grendello deleted the dev/grendel/clr-host-ignorable-assemblies branch July 2, 2025 07:39
@github-actions github-actions bot locked and limited conversation to collaborators Aug 1, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant