Skip to content

[wasm] Disable interp pgo recording by default since it adds startup overhead #101566

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Apr 27, 2024
Merged

[wasm] Disable interp pgo recording by default since it adds startup overhead #101566

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f0ae55b
Disable interp pgo recording by default since it adds startup overhead
kg Apr 25, 2024
e9bd16f
Re-enable interp pgo recording for the test used by WBT
kg Apr 26, 2024
3b4edb2
Make withRuntimeOptions not trample existing runtime options
kg Apr 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions src/mono/browser/runtime/loader/run.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,10 @@ export class HostBuilder implements DotnetHostBuilder {
interpreterPgo: value,
interpreterPgoSaveDelay: autoSaveDelay
});
if (monoConfig.runtimeOptions)
monoConfig.runtimeOptions.push("--interp-pgo-recording");
else
monoConfig.runtimeOptions = ["--interp-pgo-recording"];
Comment on lines 158 to +164
Copy link
Member

@maraf maraf Apr 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The idea around here is that deep_merge_config solves this type of conditional assignement

Suggested change
interpreterPgo: value,
interpreterPgoSaveDelay: autoSaveDelay
});
if (monoConfig.runtimeOptions)
monoConfig.runtimeOptions.push("--interp-pgo-recording");
else
monoConfig.runtimeOptions = ["--interp-pgo-recording"];
interpreterPgo: value,
interpreterPgoSaveDelay: autoSaveDelay,
runtimeOptions: ["--interp-pgo-recording"]
});

Copy link
Member

@maraf maraf Apr 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

runtime/src/mono/browser/runtime/loader/config.ts

Lines 36 to 37 in 09b983e

if (providedConfig.runtimeOptions !== undefined && providedConfig.runtimeOptions !== target.runtimeOptions) {
providedConfig.runtimeOptions = [...(target.runtimeOptions || []), ...(providedConfig.runtimeOptions || [])];

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 see, thanks. I'll make a follow-up PR to clean this up.

return this;
} catch (err) {
mono_exit(1, err);
Expand Down Expand Up @@ -268,9 +272,10 @@ export class HostBuilder implements DotnetHostBuilder {
withRuntimeOptions (runtimeOptions: string[]): DotnetHostBuilder {
try {
mono_assert(runtimeOptions && Array.isArray(runtimeOptions), "must be array of strings");
deep_merge_config(monoConfig, {
runtimeOptions
});
if (monoConfig.runtimeOptions)
monoConfig.runtimeOptions.push(...runtimeOptions);
else
monoConfig.runtimeOptions = runtimeOptions;
return this;
} catch (err) {
mono_exit(1, err);
Expand Down
2 changes: 1 addition & 1 deletion src/mono/mono/utils/options-def.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ DEFINE_BOOL(wasm_exceptions, "wasm-exceptions", FALSE, "Enable codegen for WASM
DEFINE_BOOL(aot_lazy_assembly_load, "aot-lazy-assembly-load", FALSE, "Load assemblies referenced by AOT images lazily")

#if HOST_BROWSER
DEFINE_BOOL(interp_pgo_recording, "interp-pgo-recording", TRUE, "Record interpreter tiering information for automatic PGO")
DEFINE_BOOL(interp_pgo_recording, "interp-pgo-recording", FALSE, "Record interpreter tiering information for automatic PGO")
#else
DEFINE_BOOL(interp_pgo_recording, "interp-pgo-recording", FALSE, "Record interpreter tiering information for automatic PGO")
DEFINE_BOOL(wasm_gc_safepoints, "wasm-gc-safepoints", FALSE, "Use GC safepoints on WASM")
Expand Down
5 changes: 0 additions & 5 deletions src/mono/sample/wasm/browser-bench/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,11 +225,6 @@ try {
// console to see statistics on how much code it generated and whether any new opcodes
// are causing traces to fail to compile
.withRuntimeOptions(["--jiterpreter-stats-enabled"])
// We enable interpreter PGO so that you can exercise it in local tests, i.e.
// run browser-bench one, then refresh the tab to measure the performance improvement
// on the second run of browser-bench. The overall speed of the benchmarks won't
// improve much, but the time spent generating code during the run will go down
.withInterpreterPgo(true, 30)
.withElementOnExit()
.withExitCodeLogging()
.create();
Expand Down
6 changes: 3 additions & 3 deletions src/mono/wasm/testassets/WasmBasicTestApp/App/wwwroot/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ switch (testCase) {
let alreadyFailed = [];
dotnet.withDiagnosticTracing(true).withResourceLoader((type, name, defaultUri, integrity, behavior) => {
if (type === "dotnetjs") {
// loadBootResource could return string with unqualified name of resource.
// loadBootResource could return string with unqualified name of resource.
// It assumes that we resolve it with document.baseURI
// we test it here
return `_framework/${name}`;
Expand Down Expand Up @@ -131,8 +131,8 @@ try {
}
});
const iterationCount = params.get("iterationCount") ?? 70;
for (let i = 0; i < iterationCount; i++) {
exports.InterpPgoTest.Greeting();
for (let i = 0; i < iterationCount; i++) {
exports.InterpPgoTest.Greeting();
};
await INTERNAL.interp_pgo_save_data();
exit(0);
Expand Down