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

experiment: selectively inline share_code helpers #4207

Closed
wants to merge 4 commits into from

Conversation

crusso
Copy link
Contributor

@crusso crusso commented Sep 14, 2023

to mitigate regression of new cost model, selectively inline share_code helpers in the backend. NB: some recursive ones cannot be unshared/inlined (e.g. seriliazation code and code that explicitly returns rather than returning control flow)

For profiling results ( (sans wasm-opt) against base with new cost-model, see:

dfinity/canister-profiling#80

Overall Statistics
binary_size: 7.50% [5.67%, 9.33%]
max_mem: no change
cycles: -7.15% [-8.32%, -5.99%]

Thanks to @chenyan-dfinity for producing a series of these!

(Note, this includes some Rust benchmarks that are, of course, unchanged. Typical Motoko only improvements are higher)

Coming soon... wasm-opt numbers.

aggressive wasm-opt

dfinity/canister-profiling#82

Overall Statistics
binary_size: 93.01% [67.73%, 118.30%]
max_mem: no change
cycles: -14.23% [-16.95%, -11.50%]

wasm-opt O4

dfinity/canister-profiling#84

Overall Statistics
binary_size: -10.97% [-12.49%, -9.45%]
max_mem: no change
cycles: -7.97% [-9.17%, -6.78%]

wasm-opt O3

dfinity/canister-profiling#83

Overall Statistics
binary_size: -10.50% [-11.82%, -9.18%]
max_mem: no change
cycles: -8.05% [-9.12%, -6.97%]

@crusso crusso added the build_artifacts Upload moc binary as workflow artifacts label Sep 14, 2023
@@ -1665,7 +1687,7 @@ module Tagged = struct
let ext = if Numerics.Nat16.(to_int (popcnt (of_int n))) = 1 then increment else 0l in
Int32.(logor ext (logand page_mask (shift_left minus_one (16 - Numerics.Nat16.(to_int (clz (of_int n))))))) in

Func.share_code0 env name [I32Type] (fun env ->
Func.share_code0 Func.Always env name [I32Type] (fun env ->
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Luc's #4209 inlines this too - change to Func.Never?

@chenyan-dfinity
Copy link
Contributor

The stats number is Motoko only. We exclude 0s in computing the statistics, and there is no change in the Rust benchmarks.

mergify bot pushed a commit that referenced this pull request Sep 19, 2023
To mitigate cycle perf regression of new cost model, selectively inline `share_code` helpers in the backend using an additional argument `Never | Always` (i.e. always inline vs never inline). Also, add compiler flags to explicitly opt-in or disable the inlining optimization.

NB: some recursive share_code cannot be unshared/inlined (e.g.  recursive serialization code and code that explicitly returns rather than returning control flow). 

Similar to #4207, but also inlines all heap object allocation and adds compiler flags to enable (default)/disable the optimization.
Note users may want to disable the optimization if they can't accept the increase in code size.

# Profiling data 

## new metering, sans wasm-opt
dfinity/canister-profiling#85

Overall Statistics
binary_size: 10.68% [8.58%, 12.79%]
max_mem: no change
cycles: -8.32% [-9.67%, -6.97%]

## new metering with wasm-opt 03

dfinity/canister-profiling#86

Overall Statistics
binary_size: -6.28% [-7.53%, -5.03%]
max_mem: no change
cycles: -18.21% [-20.07%, -16.36%]

## new metering, master (no-inlining) and wasm-opt 03

dfinity/canister-profiling#83

Overall Statistics
binary_size: -13.96% [-14.64%, -13.28%]
max_mem: no change
cycles: -12.46% [-13.51%, -11.41%]

(UPDATE: revised stats after @chenyan-dfinity updates to PRs)
@crusso
Copy link
Contributor Author

crusso commented Sep 19, 2023

superceded by #4212

@crusso crusso closed this Sep 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build_artifacts Upload moc binary as workflow artifacts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants