New hostfxr hosting API design - discussion #112984
Comments
ivanpovazan
added
area-Host
discussion
os-android
os-browser
|
Tagging subscribers to 'arch-android': @vitek-karas, @simonrozsival, @steveisok, @akoeplinger |
|
Tagging subscribers to 'os-ios': @vitek-karas, @kotlarmilos, @ivanpovazan, @steveisok, @akoeplinger |
What's wrong with the private hosting APIs? From https://github.com/dotnet/runtime/blob/main/docs/design/features/native-hosting.md:
That sounds like a massive overkill for what we need it for. We know which SDK to use (it's embedded in the app), we know where all the native dependencies are (also all in the app), etc. Our startup needs are really rather simple:
Note that for NativeAOT we don't neither of those two points, because we expose both our initialization method + the managed Main method using [UnmanagedCallersOnly] entry points we just call from native code. So I think our needs can be stated rather simply: we can figure out a lot at build time, so we need a hosting API that takes advantage of that fact. |
This assumes that the source of truth for the property values are UTF16 strings. Is it really the case? In general, file formats and the Unix APIs are UTF8 that makes UTF8/UTF16 conversions very common. A few more should not be a big deal. BTW: Our C/C++ implementation of UTF8/UTF16 conversions seems to be leaving perf on the table, but it have not showed up on the radar except for this PR micro-benchmark: #102424 (comment)
I do not see a problem here.
My list would be:
|
No, for some of the properties the source of truth is a pointer - some properties are function pointers. One example is PINVOKE_OVERRIDE, we do: char *pinvokeOverride = strdup_printf ("%p", &xamarin_pinvoke_override); // xamarin_pinvoke_override is a C function
coreclr_initialize (..., pinvokeOverride, ...); // very pseudo code here |
For iOS apps we generate C code, so we can provide TRUSTED_PLATFORM_ASSEMBLIES in pretty much any format. One idea could be a constant presorted list of constant (UTF8 or UTF16) strings the runtime can do a binary search in for instance. |
This should be resurrectable. Had a hard time with the hosting tests if I remember correctly. |
Ah ok, this one. I disagree that this negatively impacts the performance during startup. The overhead of these roundtrips is miniscule. I agree that this is not pretty, and it is fine to consider it during the design.
The runtime does not actually need the list. The runtime just needs a callback that returns the payload (file name, pointer to mapped memory, etc.) for the given assembly name. |
Let me describe the process here:
This process is repeated for up to 3 pointers currently (if the host optimizes it, it's just a single pointer). None of those steps are necessary, and it's a 100% waste of time. Dismissing such "insignificant" delays is what leads to software that underperforms. It is especially important on mobile to not ignore such time and resource waste. |
|
I fully agree with @rolfbjarne that we need simple, directly callable API and that hoxstfxr is a serious overkill for both iOS and Android apps. We have a huge opportunity to optimize for zero-cost operations for many things here. Properties, for instance, could be pre-converted at build time to Unicode and their values sent to the runtime not as strings, but as concrete (limited) types (e.g. bool, int, float) - other property values could be sent as Unicode strings. |
Here's an example of what happens when a tagged pointer is broken: It will become a more pronounced problem as more devices use ARMv8 with tagging extensions or ARMv9. We most certainly cannot ignore it nor can we risk breaking any pointers (via e.g. casting) in any way. |
The overhead of the entire property set (30 properties, 60 strings) conversion costs 15% of the total |
100% agreed, but as long as R2R is a resource embedded in an assembly this cannot be avoided. The best approach here is to place the code in a separate shared library and request it via an API similar to
Android already does all the JSON processing at build time. We generate native code (via LLVM IR) which contains environment variables, system properties as well as runtime properties (those from
Again, 100% agreed - I would add not converting pointers to strings to pointers as the lazy optimization here :) |
You are pointing out that the implementation of the whole thing is not very efficient today and it can be streamlined. I agree with that. The overhead of the
CoreCLR has number of issues/bugs related with tagged pointers. This is just one of many.
Right, we will need something along these lines. |
I'm pointing out that conversion from pointer-to-string-to-pointer is an entirely unnecessary process. We are running in a single process, we call the API directly, there's no reason to pass a pointer as a string. And note -
The overhead is 100% wasted time - this is the main point. Dismissing such "insignificant" things isn't going to lead to better performance.
They must be fixed then, including this one. |
Discussing a few hundred nanosecond savings is a distraction in this scenario at this point. It will lead to worse performance at the end it since we will pay less attention to improving things that actually matter. |
It's not a distraction since the code has already been written to work around it. I don't see why fixing this obvious inefficiency appears to be a problem. Maybe if I understood the reason behind this conversion I wouldn't object it. And conversion of pointer-to-string-to-pointer does actually matter - since it has the potential to break tagged pointers as we pointed out before. |
I don't think there is a problem. I've maintained that we get a big picture sense of where we are and attack the most glaring areas. I feel Jan is saying something similar. |
Should we also attempt to look at any RISC-V specific concerns as part of this? I don't know if there are any, not do I have any of the relevant experience, but I do know it's being worked on here, so if there are architecture concerns like this, it seems like it should be brought up.... |
Currently there is no riscv64 mobile platform support in runtime, and desktop linux-riscv64 does not use this API with non-stringly key-value arrays. runtime/src/coreclr/dlls/mscoree/exports.cpp Lines 235 to 242 in 7279752 on windows, corehost uses utf-16 and utf-8 on unix, there is no conversion happening on unix in corehost (from main() -> coreclr_initialize() call path). The issue raised above is about passing function pointer in propertyValues array on mobile platform, for that integer to string conversion is necessary.
(perhaps xamarin could use |
There should not be any architecture specific concerns to worry about for the design tracked by this issue. Wrt. tagged pointers: Tagged pointers are 64-bit values just like regular pointers on 64-bit platforms. I do not see anything fundamental why converting the value to string and back is incompatible with it. It is no different from storing to value to a field or a static variable and reading it back. My guess is that the C/C++ compiler is getting confused where to add and validate tags on the roundtrip code path. It should be like 2-line fix somewhere to fix this confusion once it is understood what's going on. I expect that there are number of other places in coreclr with the same problem. We should have a separate issue about it if tagged data pointers are something to worry about on Android. |
Tagged pointers are incompatible with round-tripping via strings because tagged pointers are explicitly designed to be incompatible with round-tripping, for security. See e.g. Pointer Tagging for Memory Safety:
When you pass pointers through strings, you bypass all of the pointer tagging going on behind the scenes. By doing so, the pointer is different; it has the same value, but it lacks the tags, and thus using it will result in a crash. This is all very much by design and the only way to "fix" it is to stop doing that: pointers must be passed around as pointers, not strings! This has the "nice" benefit -- in the context of this issue -- that we need to stop using strings to store pointers, which means less parsing overhead during startup, and better performance. Win-Win! |
I disagree with that. As I have said, the roundtripping through string is no different from storing the pointer in a field or a static variable and reading it back. Depending on the pointer tagging plan that the memory location is on, the roundtrip of storing the pointer to a field or a static variable may strip the tag on store and re-attach the tag on load; or store and load the pointer with the tag. The exact same options exist when roundtripping the pointer through a string. |
It's nice that you disagree with that, but copying a pointer value is not the same as roundtripping through a string. Consider this short C++ program: #include <stdio.h>
void invoke_me ()
{
printf("Hello, world!\n");
}
int main()
{
void (*p)() = invoke_me;
p(); /* works */
void (*q)() = p;
q(); /* works */
char *buf = new char[100];
snprintf(buf, sizeof(buf), "%p", p);
void (*r)() = nullptr;
sscanf(buf, "%p", &r);
r(); /* boom */
return 0;
}I you compile it on macOS with clang using clang -fsanitize=address ptr-tag.cc -lc++and run it, it crashes, big-time: Why? Because that's what Clang's AddressSanitizer is for. See also Hardware-assisted AddressSanitizer, which on arm64 uses hardware pointer tagging to implement the (software based) AddressSanitizer which caused the above crash. Going through strings is bad. @grendello has a variation on the above app crashing on an Android device, with real pointer tagging. Same basic scenario, similar basic crash (but with hardware support!). |
|
For reference, this is the Android 15 Pixel 8 crash: shiba:/data/local/tmp $ ./test
Hello, world!
Hello, world!
Bus error |
|
Right, I have said: "My guess is that the C/C++ compiler is getting confused where to add and validate tags on the roundtrip code path. It should be like 2-line fix somewhere to fix this confusion once it is understood what's going on." You are saying that the roundtripping pointers through a string is not pretty and that the way we do that today may not be compatible with pointer tagging. I agree with that. I disagree that roundtripping a pointer through a string is fundamentally incompatible with pointer tagging. The example that you have included in your reply has a trivial bug: |
|
There is also the Android documentation that describes the use of upper bits in the pointer, not two extra bits along side the 64-bit pointer. See https://source.android.com/docs/security/test/tagged-pointers. |
Description
It is needless to say that startup performance is critical for mobile applications. As we are exploring the support of CoreCLR for mobile we have identified several inefficiencies with currently available hosting APIs which need to be addressed in order to achieve best performance possible.
One example of such inefficiency is related to how the runtime properties can be passed to the runtime during initialization.
If we consider CoreCLR's private hosting APIs, like
coreclr_initialize, runtime configuration properties' values are accepted as UTF8 strings. Which means that pointers need to be converted to UTF8 strings which are internally converted back to pointers. The conversion pointer-string-pointer seems redundant and negatively impacts the performance during startup. Additionally, this conversion could break tagged pointers onARMv8+.On the other hand, the public hosting hostfxr APIs support passing runtime configuration properties as a json file, which is also not ideal.
Going forward, we would like to avoid using private hosting APIs and use hostfxr APIs instead.
The goal of this issue is to open up a discussion between our hosting and iOS/Android/WASM experts in order to collect list of requirements specific to each platform and from that design new highly performant hosting API to successfully support CoreCLR on them.
The text was updated successfully, but these errors were encountered: