dotnet / sdk

Core functionality needed to create .NET Core projects, that is shared between Visual Studio and CLI
https://dot.net/core
MIT License
2.68k stars 1.06k forks source link

Consider using `PublishAot` for Mobile Platforms and Blazor WASM #25392

Open agocke opened 2 years ago

agocke commented 2 years ago

Android currently uses the property AotAssemblies. PublishAot seems to do the same thing. Maybe we should accept either for Android, to provide uniformity. Same with Blazor, which uses RunAOTCompilation.

agocke commented 2 years ago

@marek-safar @steveisok Thoughts on the above switch for controlling AOT across Android, iOS, wasm, and NativeAOT?

steveisok commented 2 years ago

Within mono (and Blazor), RunAOTCompilation has been the property we use to run AOT. Naturally, that would be my preference everywhere as (to me) it's clear irrespective of where it runs. We can run aot both during build and publish.

I'm pretty sure AotAssemblies was a holdover from what was done in legacy xamarin-android. @jonathanpeppers can probably fill in the details there.

@lewing Any thoughts?

agocke commented 2 years ago

also + @jkotas @samsp-msft @richlander @vitek-karas

lewing commented 2 years ago

I think there are still distinctions we need to address with build vs publish but I'm also not attached to a particular property name as long as the old one continues to work since it is in net6 for wasm

agocke commented 2 years ago

Yup, no breaking changes suggested here, just a proposal that it would be nice if we could settle on a uniform name for the future.

PublishAot is the suggestion from NativeAOT because it matches PublishSingleFile, PublishTrimmed, and PublishReadyToRun. There's also good reason to put the line directly in the project file, as it should enable the appropriate analyzers. The build/publish UI decision isn't one I really plan to revisit here, but it's worth noting that for NativeAOT, AOT will only happen during publish. I don't know if that's true for these other scenarios.

radical commented 2 years ago

PublishAot is the suggestion from NativeAOT because it matches PublishSingleFile, PublishTrimmed, and PublishReadyToRun.

I like PublishAot, as it also makes it very clear that it will be done only at publish time.

If this is for mobile, then would PublishAotBundle be better?

agocke commented 2 years ago

Is it common to publish AOT without bundling in mobile?

steveisok commented 2 years ago

No, it's usually done as a step towards producing a bundle.

agocke commented 2 years ago

In that case, it sounds like PublishAot should probably imply creating a bundle. If it's not a common scenario to do otherwise, it seems like it would simplify things (and it's a shorter name).

vitek-karas commented 2 years ago

Note that even in NativeAOT PublishAot effectively implies a bundle, in the sense that all of the code in the app is bundled into one executable. Behavior-vise it implies PublishSingleFile (not in implementation though). PublishAot will also set EnableSingleFileAnalyzer (since the app will behave as single-file), this would also make sense for Blazor/Xamarin.

We plan to add guards into the SDK to prevent combinations which don't make sense. For example setting PublishAot=true and PublishSingleFile=false (and several others).

jonathanpeppers commented 2 years ago

AotAssemblies was the setting from Xamarin.Android. We kept it in .NET 6, just to help with developers migrating.

If you use the Project Options screen in a MAUI or .NET 6 Android project:

image

This setting toggles RunAOTCompilation.

In Android projects dotnet build creates a runnable Android app. There really isn't a great reason to use dotnet publish on an Android project, all it does is copy the same files dotnet build produces to $(PublishDir). If I was setting up an Android CI pipeline, the simplest setup is set a $Configuration env var to Release, dotnet build, and then archive any .apk or .aab files as artifacts.

If we pick something for AOT "the official name", I would suggest we use it for all platforms. It would be nice if you could dotnet publish -p:PublishAot=true a .sln and all projects could output their binaries to $(PublishDir).

We also should continue supporting RunAOTCompilation for a period of time? Android can also deprecate/remove AotAssemblies in .NET 7.

agocke commented 2 years ago

In Android projects dotnet build creates a runnable Android app

Right, this is a significant difference between the publish model for net* projects and android (and maybe wasm?) projects -- changing the configuration will change optimization levels, but won't do extra artifact deployment steps, like AOT or R2R or creating NuGet packages. The gesture we use for that right now is publish. I don't really know how to reconcile the differences between project types here.

steveisok commented 2 years ago

I don't really know how to reconcile the differences between project types here.

This is why I'd like to see us move away from publish in the property name. It doesn't accurately describe every scenario we support.

jonathanpeppers commented 2 years ago

Just an idea, but could it just be AOT=true?

agocke commented 2 years ago

I'm warming up slightly to that..., although @vitek-karas pointed out offline that "AOT" doesn't really make sense, and really what we mean is "AOTCompile" or "RunAOTCompilation" (although the second one is longer).

agocke commented 2 years ago

Actually, I remember we talked about this with nullable reference types and we eventually just went with "Nullable", because it was clear to everyone who actually wanted to use it, and I think that's true here as well.

richlander commented 2 years ago

There are at least two topics at play, from what I can see from this conversation (and my own understanding):

I also like the simplicity of the Nullable property. We should bias to the simple.

It would be great if a single property did something useful and expected for each workload and that for most users that was good enough.

marek-safar commented 2 years ago

I like the simplicity and flexibility of

<AOT>enable</AOT>
agocke commented 2 years ago

@richlander Do you have any feeling on the consistency of <Aot> vs <PublishAot>? The problem would be it wouldn't be like <PublishTrimmed>, <PublishSingleFile>, etc, but it would allow for eventual convergence of publish/build.

Another option is to add <Trimmed>, SingleFile, etc for consistency and accept both forms.

richlander commented 2 years ago

One option is to go with PublishAot now and move all the properties to a simpler form when we actually have a real plan on build vs publish. That's possibly the smarter idea if AOT is only available with publish. If we have AOT scenarios that cover both build and publish, then the publish prefix doesn't apply (and in fact would be confusing) and then I'd go with just Aot.

agocke commented 2 years ago

That seems fine with me. I like keeping the consistency right now with the other Publish properties, then adding new affordances all at once.

richlander commented 2 years ago

We have a plan?

ghost commented 1 year ago

@dotnet/linker-contrib a new issue has been filed in the ILLink area, please triage

agocke commented 1 year ago

This has been completed, I think.

lewing commented 1 year ago

reopening this as it was dropped late in net7 but now there are tooling designs that are impacted

eerhardt commented 1 year ago

Note that https://github.com/dotnet/sdk/pull/29785 took the assumption that PublishAot=true implies DynamicCodeSupport=false. This is true for coreclr's "Native AOT" support, but it doesn't hold true for Mono's AOT support. You can "Publish AOT", but still support "Dynamic Code".

We will have to rectify this assumption when we start using the PublishAot MSBuild property for Mono scenarios like Android and Blazor WASM.

samsp-msft commented 1 year ago

Oh 💩. That can kind of muddle the waters as we want to be clear that CoreCLRs native AOT does not include a JIT/dynamic support - some people have been asking for that kind of a hybrid.

MichalStrehovsky commented 1 year ago

It's not just DynamicCodeSupport=false. PublishAot also enables single file analyzer, trimming analyzer, AOT analyzer, disables runtimeconfig.json generation, etc. We'll have to update pretty much every use of this property within the SDK repo to not do the thing it's doing if UseMonoRuntime=true.

https://github.com/dotnet/runtime/pull/74038#issuecomment-1217419038 https://github.com/dotnet/runtime/pull/74038#issuecomment-1217428786

MichalStrehovsky commented 1 year ago

One thing to also consider is how does this unification affect the experimental iOS/tvOS/.../Bionic support with Native AOT we intend to ship in .NET 8 and whether PublishAot should always mean the restrictive form of AOT that drops all dynamicsism for size/startup/memory.

We currently have a scale of "AOT" on both Mono side and CoreCLR side (ReadyToRun) that is hard to capture in a bool, but I also don't have a good idea on how complicated we want to make this for users.

ivanpovazan commented 1 year ago

/cc: @rolfbjarne

rolfbjarne commented 1 year ago

I like the simplicity of just AOT:

<AOT>true</AOT>

I don't particularly like re-using PublishAot, because for iOS/tvOS devices + arm64 Mac Catalyst, some form of AOT is required. This means PublishAot would have to be honored for all builds. We could either do this for all platforms (which would be a breaking change), or differ between platforms (either way, the property name would be confusing).

As a sidenote for mobile targets it's cumbersome to use anything restricted to dotnet publish, because afaik it's not possible to use the IDE to deploy the results of dotnet publish to device, only dotnet build. Making it just work when people did "Run without debugging" would be much nicer than having to copy your app bundle from Windows to Mac manually, then run some obscure internal command-line tool in order to install the app on device (and in a Mac-less (Hot Restart) scenario people would probably have to resort to publishing to TestFlight in order to get the app on their own devices).

My proposal would be the following:

whether PublishAot should always mean the restrictive form of AOT that drops all dynamicsism for size/startup/memory.

Note that there's a difference between full aot (no dynamicisms - which MonoAOT can do, and it's the only thing NativeAOT does), and some intermediate thing, with either additional JIT support (I believe Android does this?) or an interpreter (which we do on Apple platforms).

steveisok commented 1 year ago

I prefer AOT as well. As Rolf mentioned, mobile targets treat publish differently and we have other configurations, like Wasm, that support AOT during dotnet build and dotnet publish. With NativeAOT, It seems we're already aware of needing to operate outside of publish with NativeCompilationDuringPublish.

  • Use UseMonoRuntime to choose between MonoAOT and NativeAOT.

I think it would make sense for one or the other to be true UseMonoRuntime and UseNativeAOTRuntime in order to toggle.

steveisok commented 1 year ago

Given where we are at in .NET 8, I would suggest the following:

akoeplinger commented 1 year ago

Use UseMonoRuntime and UseNativeAOTRuntime as the properties to control what gets enabled / disabled. iOS is the only platform that could toggle between the two.

Desktop platforms (Mac/Windows/Linux) can also choose between all three runtimes.

MichalStrehovsky commented 1 year ago

The main thing about PublishAot (and PublishTrimmed and PublishSingleFile) is that this property serves dual purpose:

  1. It signifies "intent to use AOT/trimming/single file with this project" for the editor: it enables Roslyn analyzers that flag problematic code as the user is editing it
  2. It signifies "intent to use AOT with this project" for the F5 debug cycle: it puts things in runtimeconfig.json that prevent things from working even though we're not doing AOT yet (for example, Reflection.Emit will not work even when debugging with a JIT-capable runtime)
  3. It enables AOT/trimming/single file when publishing

If we want this to work well for targets that do not consider Publish to be part of the cycle, we need to somehow be able to manifest things so that behaviors 1 and 2 still happen. Without those, using AOT is a pretty bad experience because one needs to actually debug things in a configuration that has a slow inner dev cycle (instead of basically using Hot Reload, one needs to wait for the long AOT built times), and has an inferior debugger (no hot reload, etc.). We do not want people to have to debug issues caused by trimming/AOT - they're hard and frustrating to debug.