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

Fix performance degradation while host functions call #48

Merged
merged 2 commits into from
Apr 20, 2023

Conversation

dmvolod
Copy link
Contributor

@dmvolod dmvolod commented Apr 19, 2023

Issue #, if available:
This is probably one of the option to fix performance issue

Description of changes:
Hosts functions performance tests before fix

/u01/git/wasm/go-plugin/tests/host-functions
go test -run='^$' -bench '^Benchmark' ./... -count=6 -timeout 99999s
name              time/op
HostFunctions-12  1.76ms ±12%

name              alloc/op
HostFunctions-12  5.95kB ± 6%

name              allocs/op
HostFunctions-12    25.0 ± 0%

Hosts functions performance tests after fix

/u01/git/wasm/go-plugin/tests/host-functions
go test -run='^$' -bench '^Benchmark' ./... -count=6
HostFunctions-12  13.5µs ±12%

name              alloc/op
HostFunctions-12  6.27kB ± 1%

name              allocs/op
HostFunctions-12    25.0 ± 0%

Hosts functions performance tests for v0.7.0

/u01/git/wasm/go-plugin/tests/host-functions
go test -run='^$' -bench '^Benchmark' ./... -count=6
HostFunctions-12  10.6µs ±11%

name              alloc/op
HostFunctions-12  5.99kB ± 2%

name              allocs/op
HostFunctions-12    24.0 ± 0%

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

cc: @codefromthecrypt , @lburgazzoli

Signed-off-by: Dmitry Volodin <dmvolod@gmail.com>
Signed-off-by: Dmitry Volodin <dmvolod@gmail.com>
@@ -32,6 +32,7 @@ func (h parserLibrary) ParseJson(ctx context.Context, request *ParseJsonRequest)
}
ptr, size := wasm.ByteToPtr(buf)
ptrSize := _parse_json(ptr, size)
wasm.FreePtr(ptr)
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe for this cases we could have another variant of the ByteToPtr that does not perform any allocation ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this can be an option for host functions only, but need to validate that's GC working correctly in this case

@codefromthecrypt
Copy link
Collaborator

ps you can use https://pkg.go.dev/golang.org/x/perf/cmd/benchstat to format the before after so it is easier to read.

@dmvolod
Copy link
Contributor Author

dmvolod commented Apr 20, 2023

ps you can use https://pkg.go.dev/golang.org/x/perf/cmd/benchstat to format the before after so it is easier to read.

Yes, sure. Changed to the new format

Copy link
Collaborator

@codefromthecrypt codefromthecrypt left a comment

Choose a reason for hiding this comment

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

thanks, this is a significant fix!

@codefromthecrypt codefromthecrypt merged commit 3c91ec3 into knqyf263:main Apr 20, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants