-
Notifications
You must be signed in to change notification settings - Fork 267
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
examples(allocation): free memory after unmarshalling a result from the guest #1368
examples(allocation): free memory after unmarshalling a result from the guest #1368
Conversation
…he guest Signed-off-by: Luca Burgazzoli <[email protected]>
ff401cf
to
0d7d1ea
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good. can you sweep the docs, the tinygo one in site/ and also this one's README to make sure there are no other notes/code that are similar and need an update?
sure thing! It will take a while as I'm travelling this week |
cool. for now this can merge anyway. more commits aren't a problem! |
I reverted this as it caused a large performance regression. We should probably add benchmark tests to tinymem to ensure we don't accidentally change the performance profile without knowing it. Maybe we can find a way to explain why the difference is so large. # without this commit
$ go test -run='^$' -bench '^BenchmarkAllocation.*' ./internal/integration_test/vs/compiler ./internal/integration_test/vs/interpreter -count=6 > old.txt
# with this commit
$ go test -run='^$' -bench '^BenchmarkAllocation.*' ./internal/integration_test/vs/compiler ./internal/integration_test/vs/interpreter -count=6 > new.txt
$ benchstat old.txt new.txt
goos: darwin
goarch: arm64
pkg: github.com/tetratelabs/wazero/internal/integration_test/vs/compiler
│ old.txt │ new.txt │
│ sec/op │ sec/op vs base │
Allocation/Compile-12 3.705m ± 7% 3.647m ± 2% ~ (p=0.132 n=6)
Allocation/Instantiate-12 303.9µ ± 2% 292.3µ ± 1% -3.83% (p=0.002 n=6)
Allocation/Call-12 1.571µ ± 2% 28.759µ ± 3% +1731.20% (p=0.002 n=6)
geomean 120.9µ 313.0µ +158.81%
│ old.txt │ new.txt │
│ B/op │ B/op vs base │
Allocation/Compile-12 2.125Mi ± 0% 2.124Mi ± 0% -0.05% (p=0.002 n=6)
Allocation/Instantiate-12 314.6Ki ± 0% 314.6Ki ± 0% ~ (p=0.331 n=6)
Allocation/Call-12 48.00 ± 0% 735.00 ± 2% +1431.25% (p=0.002 n=6)
geomean 31.77Ki 78.89Ki +148.28%
│ old.txt │ new.txt │
│ allocs/op │ allocs/op vs base │
Allocation/Compile-12 1.852k ± 0% 1.854k ± 0% +0.11% (p=0.002 n=6)
Allocation/Instantiate-12 802.0 ± 0% 802.0 ± 0% ~ (p=1.000 n=6) ¹
Allocation/Call-12 5.000 ± 0% 5.000 ± 0% ~ (p=1.000 n=6) ¹
geomean 195.1 195.2 +0.04%
¹ all samples are equal
pkg: github.com/tetratelabs/wazero/internal/integration_test/vs/interpreter
│ old.txt │ new.txt │
│ sec/op │ sec/op vs base │
Allocation/Compile-12 1.144m ± 0% 1.147m ± 1% ~ (p=0.240 n=6)
Allocation/Instantiate-12 90.26µ ± 0% 91.45µ ± 1% +1.31% (p=0.002 n=6)
Allocation/Call-12 28.65µ ± 1% 67.66µ ± 2% +136.15% (p=0.002 n=6)
geomean 143.6µ 192.2µ +33.86%
│ old.txt │ new.txt │
│ B/op │ B/op vs base │
Allocation/Compile-12 1.449Mi ± 0% 1.449Mi ± 0% ~ (p=0.290 n=6)
Allocation/Instantiate-12 222.7Ki ± 0% 222.7Ki ± 0% ~ (p=0.723 n=6)
Allocation/Call-12 2.254Ki ± 0% 7.155Ki ± 1% +217.39% (p=0.002 n=6)
geomean 90.65Ki 133.2Ki +46.96%
│ old.txt │ new.txt │
│ allocs/op │ allocs/op vs base │
Allocation/Compile-12 1.071k ± 0% 1.074k ± 0% +0.28% (p=0.002 n=6)
Allocation/Instantiate-12 747.0 ± 0% 747.0 ± 0% ~ (p=1.000 n=6) ¹
Allocation/Call-12 99.00 ± 0% 286.50 ± 1% +189.39% (p=0.002 n=6)
geomean 429.5 612.6 +42.64%
¹ all samples are equal |
Hi, can you try this PR again, except make sure the benchmark host code is updated the same way as the example host code was? Maybe this is why the regression happened https://github.com/tetratelabs/wazero/blob/main/internal/integration_test/vs/bench_allocation.go |
…he guest (tetratelabs#1368) Signed-off-by: Luca Burgazzoli <[email protected]>
…he guest (tetratelabs#1368) Signed-off-by: Luca Burgazzoli <[email protected]>
This is a follow up PR to amend tinygo's allocation example according to knqyf263/go-plugin#45