Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider using PublishAot for Mobile Platforms and Blazor WASM #25392

Open
agocke opened this issue May 12, 2022 · 35 comments
Open

Consider using PublishAot for Mobile Platforms and Blazor WASM #25392

agocke opened this issue May 12, 2022 · 35 comments
Assignees
Milestone

Comments

@agocke
Copy link
Member

agocke commented May 12, 2022

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 agocke changed the title Consider using PublishAot for Android Consider using PublishAot for Android and Blazor WASM May 12, 2022
@agocke
Copy link
Member Author

agocke commented May 12, 2022

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

@agocke agocke added this to the 7.0.1xx milestone May 12, 2022
@steveisok
Copy link
Member

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
Copy link
Member Author

agocke commented May 12, 2022

@lewing
Copy link
Member

lewing commented May 12, 2022

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
Copy link
Member Author

agocke commented May 12, 2022

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
Copy link
Member

radical commented May 12, 2022

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
Copy link
Member Author

agocke commented May 12, 2022

Is it common to publish AOT without bundling in mobile?

@steveisok
Copy link
Member

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

@agocke
Copy link
Member Author

agocke commented May 13, 2022

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member Author

agocke commented May 13, 2022

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
Copy link
Member

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
Copy link
Member

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

@agocke
Copy link
Member Author

agocke commented May 13, 2022

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
Copy link
Member Author

agocke commented May 13, 2022

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
Copy link
Member

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

  • Making the UX for same/similar concept uniform across workloads.
  • AOT may be default in some workloads and optional for in others.
  • There may be more than one form of AOT (either now or in the future) for a given workload.
  • The options to further configure AOT may not be uniform across workloads.
  • We may not have publish forever as a first-class concept.

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
Copy link
Contributor

I like the simplicity and flexibility of

<AOT>enable</AOT>

@agocke
Copy link
Member Author

agocke commented May 16, 2022

@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
Copy link
Member

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
Copy link
Member Author

agocke commented May 24, 2022

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
Copy link
Member

We have a plan?

@ghost
Copy link

ghost commented Oct 26, 2022

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

@agocke
Copy link
Member Author

agocke commented Oct 26, 2022

This has been completed, I think.

@lewing
Copy link
Member

lewing commented May 12, 2023

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

@eerhardt
Copy link
Member

Note that #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
Copy link
Member

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.

@steveisok steveisok changed the title Consider using PublishAot for Android and Blazor WASM Consider using PublishAot for Mobile Platforms and Blazor WASM Jun 15, 2023
@MichalStrehovsky
Copy link
Member

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.

dotnet/runtime#74038 (comment)
dotnet/runtime#74038 (comment)

@MichalStrehovsky
Copy link
Member

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
Copy link
Member

/cc: @rolfbjarne

@rolfbjarne
Copy link
Member

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:

  • A boolean AOT property.

    • Must work both for build and publish.

    • On by default when it has to be (iOS+tvOS device, Mac Catalyst on ARM64), off otherwise.

    • Deprecate PublishAot - people can replicate this behavior themselves by doing:

      <AOT Condition="'$(_IsPublishing)` == 'true'">true</AOT>

      (although we might want to support a non-underscored version of _IsPublishing)

  • Use UseMonoRuntime to choose between MonoAOT and NativeAOT.

    • Default to true for platforms that have traditionally used Mono, otherwise false.
    • Should give a decent error message if the combination isn't supported (today we get a restore failure that's not intuitive at all)
    • I'm not quite sure how ReadyToRun would fit into this?
  • Use UseInterpreter to determine if we're targeting a full aot scenario or not.

    • Show a failure if used with UseMonoRuntime=false (until NativeAOT adds interpreter support, if NativeAOT were to do that)
    • For Mono we also support a more advanced scenario where developers can choose what is interpreted and what is AOT-compiled on an assembly basis. This is rather useful actually: some people want the speed of AOT-compilation, but have some important feature they need that only works with the interpreter, and the ability to pick and choose per assembly makes them able to get the best of both worlds.

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
Copy link
Member

steveisok commented Jun 16, 2023

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
Copy link
Member

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

  • Table any property name switch discussion until .NET 9
  • Use UseMonoRuntime and UseNativeAOTRuntime as the properties to control what gets enabled / disabled. iOS is the only platform that could toggle between the two.
  • Improve the nativeaot targets to be more adaptable to iOS & future configurations.

@akoeplinger
Copy link
Member

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
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests