dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.42k stars 4.76k forks source link

Consider increasing default trampoline size #53033

Closed steveisok closed 3 years ago

steveisok commented 3 years ago

In https://github.com/dotnet/runtime/pull/52935, we needed to increase the default trampoline size in order to run any AOT tests. We are presumably going to push those defaults on the Xamarin sdk's, so does it make sense to increase them rather than embed them in a props file?

Here's what is in the MonoAOTCompiler.props right now:

<MonoAOTCompilerDefaultAotArguments Include="nimt-trampolines=2000">    
<MonoAOTCompilerDefaultAotArguments Include="ntrampolines=10000">
<MonoAOTCompilerDefaultAotArguments Include="nrgctx-fetch-trampolines=256">
<MonoAOTCompilerDefaultAotArguments Include="ngsharedvt-trampolines=4400">
<MonoAOTCompilerDefaultAotArguments Include="nftnptr-arg-trampolines=4000">
<MonoAOTCompilerDefaultAotArguments Include="nrgctx-trampolines=21000">

/cc @vargaz @akoeplinger @lambdageek

ghost commented 3 years ago

Tagging subscribers to this area: @directhex See info in area-owners.md if you want to be subscribed.

Issue Details
In https://github.com/dotnet/runtime/pull/52935, we needed to increase the default trampoline size in order to run any AOT tests. We are presumably going to push those defaults on the Xamarin sdk's, so does it make sense to increase them rather than embed them in a props file? Here's what is in the MonoAOTCompiler.props right now: ``` ``` /cc @vargaz @akoeplinger @lambdageek
Author: steveisok
Assignees: -
Labels: `area-Infrastructure-mono`
Milestone: -
lambdageek commented 3 years ago

I see

#if defined(MONOTOUCH)&& !defined(TARGET_AMD64)
    acfg->aot_opts.use_trampolines_page = TRUE;
#endif

(ie: ios on non-x64 - arm64 and arm will use infinite trampolines)

could we enable this on Android arm64 AOT too? @BrzVlad @vargaz ?

vargaz commented 3 years ago

The infinite trampoline code relies on being able to remap memory pages at a specific address, it might or it might not work on android.

mdh1418 commented 3 years ago

Note: https://github.com/dotnet/runtime/pull/50510 may bump nimt-trampolines to 4000

vargaz commented 3 years ago

So these trampolines are only needed in full-aot mode which should not be used on android.

steveisok commented 3 years ago

@vargaz I still think these are needed for when we test full aot. And from what I currently understand, android full aot is still important to some xamarin customers.

BrzVlad commented 3 years ago

@steveisok what is the use case of full aot on android ?

MaximLipnin commented 3 years ago

If I get

((null) error) Ran out of trampolines of type 3 in '/data/user/0/net.dot.System.Net.Sockets.Tests/files/System.Private.CoreLib.dll' (limit 4400)

on Android AOT with EnableAggressiveTrimming enabled (https://github.com/dotnet/runtime/pull/50510) - should I just try increasing ngsharedvt-trampolines value, say, twice ?

steveisok commented 3 years ago

@steveisok what is the use case of full aot on android ?

My understanding is that certain customers want max speed and do not care about final app size.

steveisok commented 3 years ago

on Android AOT with EnableAggressiveTrimming enabled (https://github.com/dotnet/runtime/pull/50510) - should I just try increasing ngsharedvt-trampolines value, say, twice ?

I think so. Not sure there's any other way

BrzVlad commented 3 years ago

Why would full aot be faster than normal aot ? Full aot is just aot with a bunch of complications and limitations of not being able to generate stuff at runtime. For example we could just dynamically generate those trampolines without having to bloat the application package with thousands of trampolines.

steveisok commented 3 years ago

Good question. I'm just relaying what I know the current use case to be. Obviously, if we can support something better, by all means we should do it.

BrzVlad commented 3 years ago

I think there is a confusion between using aot + llvm for all the application code (with the increase in size drawback but with gain in perf versus normal jit / interp) versus forcing the runtime to not generate any executable code at runtime, which doesn't help with anything since it is a limitation not a feature.

SamMonoRT commented 3 years ago

From discussions above, am getting a sense it is not a required fix for 6.0. @steveisok - can we can move this to 7.0, and not rush in changes ?

steveisok commented 3 years ago

Sure. It's settled enough for now and we can reopen if needed.