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

[release/7.0][browser] improve default initial memory size #80849

Conversation

pavelsavara
Copy link
Member

@pavelsavara pavelsavara commented Jan 19, 2023

Simplified backport of #80257 to release/7.0

The default value of 512MB for initial memory is too high for mobile devices.
This PR changes the default to 16MB for builds with wasm-tools workload and without AOT.
This PR changes the default to 128MB for builds with wasm-tools workload and with AOT.

Customer Impact

Fixes #61925
Fixes #66313
Fixes #79909
Fixes #73949

Workaround

Adding a property to the customer project file will achieve the same outcome as this PR,

  • for the older releases of Net7 <EmccInitialHeapSize>16777216</EmccInitialHeapSize>
  • for Net6 <EmccTotalMemory>16777216</EmccTotalMemory>

Testing

  • Manual
  • WBT on CI

Risk

The fix for Net8 uses more complex logic and we decided to reduce the risk by making backport logic simpler.

This PR makes initial memory for AOT build lower than it was in Net7 so far.
It may lead to new compilation error similar to:

Linking for initial memory $(EmccInitialHeapSize)=134217728 bytes.
...
wasm-ld : error : initial memory too small, 536870912 bytes needed

Which could be resolved by adding property into the customer project file.
<EmccInitialHeapSize>536870912</EmccInitialHeapSize>

The EmccInitialHeapSize value need to be rounded up to 16KB page size.

@ghost
Copy link

ghost commented Jan 19, 2023

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

Simplified backport of #80257 to release/7.0

The default value of 512MB for initial memory is too high for mobile devices.
This PR changes the default to 16MB for builds with wasm-tools workload and without AOT.
This PR changes the default to 128MB for builds with wasm-tools workload and with AOT.

Customer Impact

Fixes #61925
Fixes #66313
Fixes #79909
Fixes #73949

Workaround

Adding a property to the customer project file will achieve the same outcome as this PR, for the older releases of Net7.
<EmccInitialHeapSize>16777216</EmccInitialHeapSize>

Testing

  • Manual
  • WBT on CI

Risk

The fix for Net8 uses more complex logic and we decided to reduce the risk by making backport logic simpler.

This PR makes initial memory for AOT build lower than it was in Net7 so far.
It may lead to new compilation error similar to:

Linking for initial memory $(EmccInitialHeapSize)=134217728 bytes.
...
wasm-ld : error : initial memory too small, 536870912 bytes needed

Which could be resolved by adding property into the customer project file.
<EmccInitialHeapSize>536870912</EmccInitialHeapSize>

The EmccInitialHeapSize value need to be rounded up to 16KB page size.

Author: pavelsavara
Assignees: pavelsavara
Labels:

arch-wasm, area-System.Runtime.InteropServices.JavaScript

Milestone: 7.0.x

@pavelsavara
Copy link
Member Author

/azp run runtime-wasm

Copy link
Member

@maraf maraf left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@pavelsavara pavelsavara added the Servicing-consider Issue for next servicing release review label Jan 24, 2023
@pavelsavara
Copy link
Member Author

was approved by email.

@carlossanlop
Copy link
Member

Approved by Tactics for 7.0.4.
Signed off by area owners.
No OOB changes needed.
CI is green.
Ready to merge. :shipit:

@carlossanlop carlossanlop merged commit 1335040 into dotnet:release/7.0 Feb 8, 2023
@GeeSuth
Copy link

GeeSuth commented Feb 9, 2023

For older .NET 6, you need to add

<EmccInitialHeapSize>16777216</EmccInitialHeapSize>

Manually?

@maraf
Copy link
Member

maraf commented Feb 9, 2023

For older .NET 6, you need to add

<EmccInitialHeapSize>16777216</EmccInitialHeapSize>

Manually?

In .NET 6 the property name is EmccTotalMemory and yes, you need to set it manually.

@ghost ghost locked as resolved and limited conversation to collaborators Mar 11, 2023
@pavelsavara pavelsavara deleted the backport/simplified-pr-80507-to-release/7.0 branch September 2, 2024 15:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-System.Runtime.InteropServices.JavaScript Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants