-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add -aot
parameter to the console template
#31739
Conversation
This is similar to the `api` template in ASP.NET that also allows `-aot` (or `--publish-native-aot`) option. We set similar properties: * `<PublishAot>true</PublishAot>` (this is the obvious one) * `<InvariantGlobalization>true</InvariantGlobalization>` to get rid of ICU dependency on Linux and make the hello world executable ~20% smaller in general, even outside Linux. * a commented out `<StripSymbols>true</StripSymbols>`. This is for discoverability. We default this to `false` due to platform conventions, but many users would be fine setting this to `true`. I took the string from ASP.NET. I could have also copied the localized strings but I guess it's better to not step on the toes of the localization team.
I've not done anything for F# since even a hello world produces both trimming and AOT warnings and it's not the experience we're shooting for with PublishAot. |
<InvariantGlobalization>true</InvariantGlobalization> | ||
|
||
<!-- Uncomment below line to make native publish outputs a lot smaller on Linux and macOS --> | ||
<!-- <StripSymbols>true</StripSymbols> --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've struggled a bit with the level of detail here. We don't want paragraphs of text that explain native debuggers/profilers/etc may or may not support GNU debuglink and if they don't extra steps are needed to make sense of the resulting binary, etc. Most people will be fine because most tools do support it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about making an aka.ms link, and then you can point it to docs that explains it fully?
<InvariantGlobalization>true</InvariantGlobalization> | ||
|
||
<!-- Uncomment below line to make native publish outputs a lot smaller on Linux and macOS --> | ||
<!-- <StripSymbols>true</StripSymbols> --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about making an aka.ms link, and then you can point it to docs that explains it fully?
<InvariantGlobalization>true</InvariantGlobalization> | ||
|
||
<!-- Uncomment below line to make native publish outputs a lot smaller on Linux and macOS --> | ||
<!-- <StripSymbols>true</StripSymbols> --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should follow the same approach for ASP.NET and Hosting templates. We shouldn't have different strategies for the different templates if there are no technical reasons for it. We should decide here if this is the strategy we want to take, and apply it in all templates.
I also think that this just clutters the template. IMHO - I think StripSymbols=true
should be the default without specifying it in the .csproj. It is what most normal .NET devs expect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think StripSymbols=true should be the default without specifying it in the .csproj. It is what most normal .NET devs expect.
I think this is the crux of it. Lot of feedback I heard was that .NET Devs expect debuggability by default, which would mean we have to leave the symbols there. Another view is what is the typical behavior for the target platform (Linux, containers).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I forgot to add that I think there's a difference between symbols for managed assemblies and symbols for native AOT. Managed assemblies are debugable even without symbols, the symbols only improve the experience somewhat. Native AOT is completely undebugable without symbols. To me that is the key difference between the two and why I think we should treat them separately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we base it on Release
or not?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally I don't think so - but I have no hard data.
In short - I think the number of people publishing AOT with Debug will be tiny. Non-AOT .NET makes little difference between Debug and Release - the debugging experience is very similar. So I think that lot of people don't think about the configuration setting when considering if something is debuggable or not. But these are just my personal guesses...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we decide to default StripSymbols
to true on the SDK side, I have some additional questions:
- .NET developers are used to symbols having the .pdb extension, no matter their underlying representation (managed/portable are very different file formats). Should we also name the native symbol file with the Windows (and by extension .NET) convention? If we don't want people to learn the platform conventions, this follows the principle of least surprise.
- Is there any point in the AOT console template? My main motivation for it was really just about the discoverability of
StripSymbols
.<PublishAot>true</PublishAot>
doesn't seem to be interesting enough as a differentiator. I've also put<InvariantGlobalization>true</InvariantGlobalization>
because it gets rid of ICU on Linux and helps discoverability, but that one is basically just getting a ride. Are the things that are left still important enough that we need a template?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's still worth adding the option to the template if for nothing else that visibility and discovery that native AOT is now a first-class option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we also name the native symbol file with the Windows (and by extension .NET) convention?
This would not work well even if we wanted to. Many tools infer the file format from the extension. If we started giving .pdb extension to DWARF files, it would be endless source of breaks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could maybe go with .pdb.dbg/.pdb.dwarf. It's just whether we want people to know these are the debugging symbols, not a vital part of a non-single-file app. We assume .NET developers don't know about the underlying platform. But I guess this would be orthogonal to this PR and at this point it would also be a "breaking" change from 7.0 (so will be the new default though).
@dotnet/templating-engine-maintainers could someone have a look? I think we settled on the non-technical questions and we might tweak this based on the defaults in the coming weeks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could maybe go with .pdb.dbg/.pdb.dwarf. It's just whether we want people to know these are the debugging symbols, not a vital part of a non-single-file app.
I do not think we need to go this far. People will need to learn some amount of platform specific details in any case. The problem is that we do not have a good trigger for learning about symbols today. People do not read the manual; they just try to do stuff. We have seen many variants of a story that goes like this: Somebody builds a native AOT binary on Linux for the first time, sees a single giant binary in the publish folder, and concludes that this technology is unusable. I think that having two files - one smaller executable and one large non-executable - would be good enough to trigger the learning even without the .pdb hint. Seeing two files will make people ask why there are two files, whether they need both, etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
If we choose to change the StripSymbols defaults, it can be done as a follow up across the board.
@baronfel could you please review as this is a modification to console template? Thanks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please consider adding tests to https://github.com/dotnet/sdk/blob/main/src/Tests/dotnet-new.Tests/CommonTemplatesTests.cs
@baronfel do you have any opinions? I'd like this to make it into Preview 4 and it's getting close. |
LGTM - wanted to make sure the parameters aligned with ASP.NET's template (of course they do). Any reason why the F# console template can't have this as well? |
|
Ah, thanks :) IIRC @KevinRansom is doing some more trimming/aot work right now that might make that disappear. @vzarytovskii want to make a note to check the experience after that lands and update the F# console template with this option as well? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
See discussion in dotnet/sdk#31739 (comment). Once this merges, we should: - [ ] Update the `dotnet new console -aot` template - [ ] Update the `dotnet new api -aot` template - [ ] Update docs and declare different defaults.
Thanks @MichalStrehovsky! It would be nice(r) if we use single or double hyphens consistently with |
@DamianEdwards what do you think? (For this template I just aligned with dotnet/aspnetcore#46064 and there wasn't much to think about because the precedent was set.) |
I'm happy to say from now on we should strive to match the Unix-y tool convention and change the |
Definitely would echo this - I was biasing towards compat with aspnetcore here, but I would prefer POSIX-y. |
Should we consider ret-conning this across all templates, by adding |
See discussion in dotnet/sdk#31739 (comment).
@MichalStrehovsky - I am able to successfully publish and run an F# "worker" app with AOT with no warnings: What warnings were you getting with a hello world F# app? |
Oh, I just tried it manually, and it appears to be coming from
I'll remove the F# worker template's |
Heh, F# bug: dotnet/fsharp#15140. Nevertheless, we should not crash. This should be in a try/catch and we should ignore the invalid blobs here. |
Huh, interesting, it seems it was caused by our recent (last week) change in DU codegen, when we try to account for refleciton we do. |
This is similar to the
api
template in ASP.NET that also allows-aot
(or--publish-native-aot
) option (see dotnet/aspnetcore#46064).We set similar properties:
<PublishAot>true</PublishAot>
(this is the obvious one)<InvariantGlobalization>true</InvariantGlobalization>
to get rid of ICU dependency on Linux and make the hello world executable ~20% smaller in general, even outside Linux.<StripSymbols>true</StripSymbols>
. This is for discoverability. We default this tofalse
due to platform conventions, but many users would be fine setting this totrue
.true
is the .NET convention.I took the string from ASP.NET. I could have also copied the localized strings but I guess it's better to not step on the toes of the localization team.
Cc @dotnet/ilc-contrib @DamianEdwards