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

Enable WASM AOT microbenchmark run #55353

Merged
merged 125 commits into from
Aug 5, 2021
Merged

Conversation

Lxiamail
Copy link
Member

@Lxiamail Lxiamail commented Jul 8, 2021

No description provided.

…t time. Adjust -aotcompilermode format to "--aotcompilermode=wasm".
…t time. Adjust -aotcompilermode format to "--aotcompilermode=wasm".
payload_directory=
workitem_directory=$source_directory
performance_directory=$workitem_directory
setup_arguments="--perf-hash $commit_sha $common_setup_arguments"
else
git clone --branch main --depth 1 --quiet https://github.com/dotnet/performance $performance_directory
echo "Not running from perf repo"
Copy link
Member Author

@Lxiamail Lxiamail Aug 4, 2021

Choose a reason for hiding this comment

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

Before this PR merge, should revert back to the original code to clone main branch once PR dotnet/performance#1902 is merged

@Lxiamail Lxiamail marked this pull request as ready for review August 4, 2021 16:27
Copy link
Member

@DrewScoggins DrewScoggins left a comment

Choose a reason for hiding this comment

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

LGTM, apart from the few small things I mentioned in comments. Also we need to remove the performance submodule

@@ -228,7 +234,11 @@ if [[ "$mono_dotnet" != "" ]] && [[ "$monointerpreter" == "false" ]]; then
fi

if [[ "$wasm_runtime_loc" != "" ]]; then
configurations="CompilationMode=wasm RunKind=$kind"
if [[ "$wasmaot" == "true" ]]; then
configurations="CompilationMode=wasm AOT=true RunKind=$kind"
Copy link
Member

Choose a reason for hiding this comment

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

AOT -> MonoAOT to match the current configs we have.

Copy link
Member Author

Choose a reason for hiding this comment

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

Is MonoAOT different from wasm aot?

@@ -254,8 +264,10 @@ if [[ "$run_from_perf_repo" = true ]]; then
performance_directory=$workitem_directory
setup_arguments="--perf-hash $commit_sha $common_setup_arguments"
else
git clone --branch main --depth 1 --quiet https://github.com/dotnet/performance $performance_directory

git clone --branch alicial/wasmaotmicro --depth 1 https://github.com/dotnet/performance.git $performance_directory
Copy link
Member

Choose a reason for hiding this comment

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

This needs to switch back to cloning from the performance repo.

# install emsdk, $source_directory/src/mono/wasm/ has the nuget.config with require feed. EMSDK may be available in the payload in a different directory, should visit this install to avoid deplicated payload.
pushd $source_directory/src/mono/wasm/
make provision-wasm
EMSDK_PATH = $source_directory/src/mono/wasm/emsdk
Copy link
Member

Choose a reason for hiding this comment

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

This variable gets set, but it does not ever seem to get used anywhere. What is it for?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, EMSDK_PATH is not used in our script file. It is set here is per document guidance at https://github.com/dotnet/runtime/blob/d953229d5429d2ffde833740dd481aab864d3e0c/src/mono/wasm/README.md

@Lxiamail Lxiamail merged commit 9dc2059 into main Aug 5, 2021
@Lxiamail Lxiamail deleted the alicial/WASMAOTMicrobenchmarks branch August 5, 2021 16:35
@ghost ghost locked as resolved and limited conversation to collaborators Sep 7, 2021
Sign up for free to 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.

5 participants