Skip to content

[wasm] Enable dedup by default. - #80260

Merged
vargaz merged 5 commits into
dotnet:mainfrom
vargaz:wasm-dedup-enable
Jan 18, 2023
Merged

[wasm] Enable dedup by default.#80260
vargaz merged 5 commits into
dotnet:mainfrom
vargaz:wasm-dedup-enable

Conversation

@vargaz

Copy link
Copy Markdown
Contributor

No description provided.

@vargaz
vargaz requested a review from radical as a code ownerJanuary 5, 2023 19:50
@ghostghost added the area-Build-mono label Jan 5, 2023
@ghostghost assigned vargazJan 5, 2023
@vargaz

Copy link
Copy Markdown
ContributorAuthor

/azp run runtime-wasm

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@vargaz

Copy link
Copy Markdown
ContributorAuthor

/azp run runtime-wasm

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@tannergooding

Copy link
Copy Markdown
Member

Should be unblocked now. Closing and reopening the PR should retrigger CI with the updated main.

@vargaz

Copy link
Copy Markdown
ContributorAuthor

/azp run runtime

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@vargaz

Copy link
Copy Markdown
ContributorAuthor

/azp run runtime-wasm

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@lewing

Copy link
Copy Markdown
Member

Looked like the tests failed because the TupleElementNamesAttribute wasn't preserved?

@vargaz

Copy link
Copy Markdown
ContributorAuthor

/azp run runtime-wasm

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@vargaz
vargaz marked this pull request as draft January 10, 2023 05:19
@vargaz

Copy link
Copy Markdown
ContributorAuthor

/azp run runtime-wasm

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@vargaz

Copy link
Copy Markdown
ContributorAuthor

/azp run runtime-wasm

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@vargaz

Copy link
Copy Markdown
ContributorAuthor

The WasmBuildTests failure looks relevant.

@vargaz

Copy link
Copy Markdown
ContributorAuthor

/azp run runtime-wasm

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@vargaz

Copy link
Copy Markdown
ContributorAuthor

/azp run runtime-wasm

2 similar comments
@vargaz

Copy link
Copy Markdown
ContributorAuthor

/azp run runtime-wasm

@vargaz

Copy link
Copy Markdown
ContributorAuthor

/azp run runtime-wasm

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

2 similar comments
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

.. copied to `aot-in` for the compilation step.
Example:
- when using WasmDedup=true, we get the main assemblies in `publish`
directory after linking, but `aot-instances.dll` is in different directory.
- this causes `MonoAOTCompiler` to copy all of them to a temporary `aot-in` dir for compiling with `mono-aot-cross`.
- And when the output items are set, we get:
```
Output Item(s):
_WasmAssembliesInternal=
obj/Debug/net8.0/browser-wasm/wasm/for-publish/aot-in/Debug_u4nbxx3i.gc5.dll
LlvmBitcodeFile=obj/Debug/net8.0/browser-wasm/wasm/for-publish/Debug_u4nbxx3i.gc5.dll.bc
```
- here the `ItemSpec` is incorrectly set to the temporary `aot-in` path
- which can cause build failures in the following build steps
@radical

Copy link
Copy Markdown
Member

Pushed a fix for the WBT failure.

@lewinglewing left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

:shipit:

@vargaz

Copy link
Copy Markdown
ContributorAuthor

/azp run runtime-wasm

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@lewing

Copy link
Copy Markdown
Member

wbt are green and the build fix makes sense

@vargaz
vargaz marked this pull request as ready for review January 18, 2023 05:50
Comment threadeng/testing/tests.wasm.targets
@vargaz

Copy link
Copy Markdown
ContributorAuthor

Failures are unrelated.

@vargaz
vargaz merged commit f3af676 into dotnet:mainJan 18, 2023
@vargaz
vargaz deleted the wasm-dedup-enable branch January 18, 2023 09:22
@runfoapprunfoappBot mentioned this pull request Jan 18, 2023
@ghostghost locked as resolved and limited conversation to collaborators Feb 17, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@vargaz@tannergooding@lewing@radical