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

benchmark: Updates wazero to 1.0.0-beta.2 #85

Merged
merged 1 commit into from Aug 31, 2022

Conversation

codefromthecrypt
Copy link
Contributor

@codefromthecrypt codefromthecrypt commented Aug 30, 2022

This updates wazero to 1.0.0-beta.2

Future betas will release at least once each month until 1.0 in February 2023.

Note: Release notes will be posted in the next day or two.

Meanwhile, we've also opened a gophers slack #wazero channel for support, updates and conversation! Note: You may need an invite to join gophers.

@inkeliz
Copy link
Owner

inkeliz commented Aug 30, 2022

Strangely, Zig is falling in the test. I need to investigate.

That might be two reasons:

A) karmem issue: bugs on the generated Zig code trying to read out-of-bounds contents.
B) wazero issue: some bug when read/store unaligned memory or some memory manipulation.

That issue doesn't exists in the previous version.


Some quick investigation:

  1. Using wazero.NewRuntimeConfigInterpreter() works and pass all fuzzing.
  2. Removing the Zig tests, everything else is working (https://builds.sr.ht/~inkeliz/job/834688#task-test_zig).
  3. Using debug also crashes (and the trace is quite useless): https://builds.sr.ht/~inkeliz/job/834679#task-test_zig

I'll try to run some git bisect.

@inkeliz
Copy link
Owner

inkeliz commented Aug 30, 2022

Using git bisect, that is the result:

git.exe bisect bad
0bd2beedac29af6aab324a50556464e32e6bdbcf is the first bad commit

tetratelabs/wazero#761


That is the change which causes the Zig test to fail.

@codefromthecrypt
Copy link
Contributor Author

@inkeliz one major change in that PR is that caching exported functions is no longer supported, could that be the issue? Meanwhile, we'll look deeper.

@inkeliz
Copy link
Owner

inkeliz commented Aug 31, 2022

Well, I'm using the ExportedFunctiosn with:

functions := make(map[string]api.Function, len(fn))
for _, fn := range fn {
functions[fn] = w.mainModule.ExportedFunction(fn)
}

So, if that is "cache", that might be the reason. I'm re-using the same ExportedFunction:

func (w *WasmWazero) Run(s Functions, v ...uint64) ([]uint64, error) {
f, ok := w.functions[s.String()]
if !ok || f == nil {
return nil, errors.New("invalid function of " + s.String())
}
return f.Call(context.Background(), v...)
}

@codefromthecrypt
Copy link
Contributor Author

same pattern in trivy, too. We need to make a call of whether to make functions lazy by default or have projects like these change their calling pattern. Meanwhile, it could be not the root cause of the memory issue.

@inkeliz
Copy link
Owner

inkeliz commented Aug 31, 2022

I try it again, using:

func (w *WasmWazero) Run(s Functions, v ...uint64) ([]uint64, error) {
	f := w.mainModule.ExportedFunction(s.String())
	if f == nil {
		return nil, errors.New("invalid function of " + s.String())
	}
	return f.Call(context.Background(), v...)
}

Seems to work. So, the map[] is the issue. 🤔

@inkeliz
Copy link
Owner

inkeliz commented Aug 31, 2022

However, I don't think the concurrency is the issue. I also tried to include mutex.Lock(); defer mutex.Unlock() inside the t.Fuzz(), so it will not call any function concurrently, and it doesn't work. But, I didn't investigate it deeper, just quick modifications. 😬

@codefromthecrypt
Copy link
Contributor Author

thanks for the help @inkeliz!

@mathetake
Copy link

Thanks for you patience @inkeliz, could you try the fix here? tetratelabs/wazero#783

@codefromthecrypt
Copy link
Contributor Author

@mathetake I added you with write. can you try on my branch?

@mathetake
Copy link

Cool will do

@inkeliz
Copy link
Owner

inkeliz commented Aug 31, 2022

It's working now. 😎

@mathetake
Copy link

cool!

This updates [wazero](https://wazero.io) to 1.0.0-beta.2

Future betas will release at least once each month until 1.0 in February 2023.

Note: [Release notes](https://github.com/tetratelabs/wazero/releases) will be posted in the next day or two.

Meanwhile, we've also opened a [gophers slack](https://gophers.slack.com/) `#wazero` channel for support, updates and conversation! Note: You may need an [invite](https://invite.slack.golangbridge.org/) to join gophers.

Signed-off-by: Adrian Cole <adrian@tetrate.io>
@codefromthecrypt codefromthecrypt changed the title benchmark: Updates wazero to its first beta benchmark: Updates wazero to 1.0.0-beta.2 Aug 31, 2022
@codefromthecrypt
Copy link
Contributor Author

ok now 1.0.0-beta.2 hope all good!

@inkeliz inkeliz merged commit 780b1b3 into inkeliz:master Aug 31, 2022
@mathetake mathetake deleted the wazero-beta branch August 31, 2022 05:12
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