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

[wasm] Disallow not useful configuration combinations #104149

Merged
merged 11 commits into from
Jul 4, 2024

Conversation

ilonatommy
Copy link
Member

@ilonatommy ilonatommy commented Jun 28, 2024

Fixes #94103.

  • Switches off possibility of using RunAOTCompilation in debug mode - aot doesn't support managed debugging, so this combination does not make sense. Corresponding WBT that were testing this combination are removed. A special test for WASM and for Blazor is added to make sure the combination produces a proper error.

  • One fix in the JS code was needed to fix running profiler sample on browser. Beyond it, profiled aot works fine with and without PublishTrimmed in debug mode, it does not have to be blocked.

@ilonatommy ilonatommy marked this pull request as ready for review July 2, 2024 11:31
@pavelsavara
Copy link
Member

Does AOT + application in Debug have better stack traces than AOT + application in Release ? (runtime being always in Release)

@ilonatommy
Copy link
Member Author

Does AOT + application in Debug have better stack traces than AOT + application in Release ? (runtime being always in Release)

I was assuming it should be better in debug but I tested experimentally, sample app throwing an exception from C# and stack traces do not differ. Do you have a better idea how to check it?

@pavelsavara
Copy link
Member

I would expect Debug to have source code line numbers in the C# stack trace.
And if we have dwarf, something similar could be the case for AOT. But I'm not sure that Mono AOT can produce that.

@ilonatommy
Copy link
Member Author

I would expect Debug to have source code line numbers in the C# stack trace

I understand. Sadly, no line numbers, dwarf present
image

@ilonatommy ilonatommy merged commit 2e585aa into dotnet:main Jul 4, 2024
34 of 36 checks passed
@@ -621,6 +621,8 @@

<Error Condition="'$(RunAOTCompilation)' == 'true' and '$(PublishTrimmed)' != 'true'"
Text="AOT is not supported without IL trimming (PublishTrimmed=true required)." />
<Error Condition="'$(RunAOTCompilation)' == 'true' and '$(Configuration)' == 'Debug'"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the true underlaying requirement for AOT? The configuration name "Debug" is just a name, user can have a custom name.

Copy link
Member Author

@ilonatommy ilonatommy Jul 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We rely on Configuration variable value, comparing it with Release and Debug in multiple places.

<PropertyGroup Condition="'$(WasmNativeDebugSymbols)' == '' and '$(WasmNativeStrip)' == '' and '$(WasmBuildingForNestedPublish)' != 'true' and '$(WasmBuildNative)' == 'true' and '$(Configuration)' == 'Debug'">

<WasmBuildNative Condition="'$(WasmBuildNative)' == '' and '$(Configuration)' == 'Release'">true</WasmBuildNative>

<_MonoCPPFLAGS Condition="'$(Configuration)' == 'Debug'" Include="-D_DEBUG" />

Do you mean that there's a better property to check if we have debug symbols available?

Copy link
Member

@maraf maraf Jul 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm aware of other places (that they exist) and every now and then they fail for some user with custom config name (there not much of them, but such users exist). So I would like to know if there is a better property to check and hence the question what is the true underlaying requirement for AOT compiler, so we could check that requirement instead.

If the requirement is one of the places where we check for config==debug, there is nothing to do about it. If the requirement is something different with some concrete property value, it would be great to check that property.

Copy link
Member Author

@ilonatommy ilonatommy Jul 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. The requirement: runtime without optimization (so debug-friendly) will take long time to build app with AOT and in the end it won't support managed debugging, so there was no point. Useful properties would be connected with managed debugging or optimization on/off (looking for such...).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not able to find a good alternative

@github-actions github-actions bot locked and limited conversation to collaborators Aug 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-Build-mono
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[wasm] Some AOT configuration combinations to disallow
4 participants