Generate less relocations in the native typemap code #10225
Conversation
grendello
changed the title
grendello
force-pushed
the
dev/grendel/clr-host-typemap-less-relocs
branch
from
e3628bd to
77ed61f
Compare
grendello
force-pushed
the
dev/grendel/clr-host-typemap-less-relocs
branch
from
77ed61f to
fa405a6
Compare
There was a problem hiding this comment.
Pull Request Overview
This pull request refactors the native typemap logic by eliminating redundant pointer fields and using index-based lookups for type maps, ultimately reducing relocations in the libxamarin-app.so library. Key changes include replacing per-module pointer fields with index fields in the C++ structs, updating logging levels to reduce noise, and extending the LLVM IR generation code to support sectioned arrays for module maps.
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/native/clr/xamarin-app-stub/application_dso_stub.cc | Introduced new const arrays for module mapping data |
| src/native/clr/runtime-base/util.cc | Replaced mkdir with create_directory for directory creation |
| src/native/clr/include/xamarin-app.hh | Modified TypeMapModule structure to use index fields |
| src/native/clr/host/typemap.cc | Updated map lookup logic to use index values and added clarifying comments |
| src/native/clr/host/host.cc, host-jni.cc | Removed redundant log messages from JNI_OnLoad flows |
| src/native/clr/host/assembly-store.cc | Changed log level from warn to debug for assembly lookup failures |
| src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingReleaseNativeAssemblyGeneratorCLR.cs | Adjusted native assembly generator to use index-based mapping; removed unused symbol name methods |
| src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/* | Added support for sectioned arrays to output module map data cleanly |
| src/Mono.Android/Java.Interop/TypeManager.cs | Removed extraneous logging after type load |
Comments suppressed due to low confidence (2)
src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingReleaseNativeAssemblyGeneratorCLR.cs:29
- Ensure that the removal of the GetPointedToSymbolName method and its associated symbol name fields does not affect any downstream native assembly generation logic.
return String.Empty;
src/native/clr/runtime-base/util.cc:55
- Please confirm that create_directory behaves identically to mkdir in terms of error handling and permission settings to maintain consistent behavior across platforms.
int ret = create_directory (dir.data (), 0777);
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| log_warn (LOG_ASSEMBLY, "Assembly '{}' (hash 0x{:x}) not found"sv, optional_string (name.data ()), name_hash); | ||
| // This message should really be `log_warn`, but since CoreCLR attempts to load `AssemblyName.ni.dll` for each | ||
| // `AssemblyName.dll`, it creates a lot of non-actionable noise. | ||
| // TODO: generate hashes for the .ni.dll names and ignore them at the top of the function. Then restore | ||
| // `log_warn` here. | ||
| log_debug (LOG_ASSEMBLY, "Assembly '{}' (hash 0x{:x}) not found"sv, optional_string (name.data ()), name_hash); |
There was a problem hiding this comment.
Could this log_debug() if the name ends with .ni.dll and otherwise log_warn()? Then we probably wouldn't need a TODO anymore.
There was a problem hiding this comment.
I'll have a separate PR to handle this.
| @@ -247,7 +247,6 @@ static Type monovm_typemap_java_to_managed (string java_type_name) | |||
| } | |||
| } | |||
|
|
|||
| Logger.Log (LogLevel.Info, "monodroid", $"Loaded type: {ret}"); | |||
There was a problem hiding this comment.
I don't know why this was here, but maybe we could leave the message and just check if (Logger.LogAssembly).
There was a problem hiding this comment.
It looks like it was there just for testing, but I'm not opposed to hiding it behind the category check
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
NOTE: Needs #10247 to be merged first
Context: 96a73e6
Context: f37158e
This is another PR in the series that remove native relocations from
the
libxamarin-app.solibrary.Modify the typemap data so that no pointers to per-module data for
the primary and duplicate maps need to be stored. Module here refers
to a single assembly used by the application. Each assembly/module is
described by an instance of a C++
structwhich used to contain up totwo pointers to tables containing actual type mapping data - one for the
primary map (all modules have this one) and an optional map of duplicate
types, which was stored in another pointer.
The two pointers pointed to separate arrays of map data structures. Instead,
this PR now puts all of the primary map data for all modules in one array,
and all of the duplicate map data in another array. The assembly/module descriptor
structures now merely contain indices into those arrays.
This removes up to X*2 relocations (with X being the number of assemblies).
The change gives us another modest startup performance boost of 0.67% on a Pixel 8
device, using the
dotnet new maui -scsample. It also decreases the number ofrelocations in
libxamarin-app.sofrom 394 to 70 entries for this sample app.Modest size decrease of
libxamarin-app.sois another outcome: