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

bench: add stdlib benchmark old vs new compiler #1878

Merged
merged 15 commits into from
Dec 20, 2023
Merged

bench: add stdlib benchmark old vs new compiler #1878

merged 15 commits into from
Dec 20, 2023

Conversation

evacchi
Copy link
Contributor

@evacchi evacchi commented Dec 18, 2023

Creating in draft for early feedback

Signed-off-by: Edoardo Vacchi <[email protected]>
Signed-off-by: Edoardo Vacchi <[email protected]>
Signed-off-by: Edoardo Vacchi <[email protected]>
@evacchi
Copy link
Contributor Author

evacchi commented Dec 18, 2023

notice that currently, these aren't running on CI as the corresponding testdata dir is empty!

Signed-off-by: Edoardo Vacchi <[email protected]>
Signed-off-by: Edoardo Vacchi <[email protected]>
@evacchi
Copy link
Contributor Author

evacchi commented Dec 19, 2023

pushed a new version. It is likely to break due to old cache IDs. It is currently broken on Windows. My guess is because local, Windows-style paths are not compatible with the Unix-style wasm paths.

e.g. C:\....\testdata\some.file => while ./testdata/some.file is expected

Signed-off-by: Edoardo Vacchi <[email protected]>
@evacchi
Copy link
Contributor Author

evacchi commented Dec 19, 2023

too many issues with paths to write one test case that works also on Windows, I'll return to this later; let's see if it works on CI this way for now:

  • docker+cli on arm64 (no way around this unless we use a go image instead of scratch)
  • cli on Windows
  • test on linux/macos x64

@evacchi
Copy link
Contributor Author

evacchi commented Dec 19, 2023

I suspect the CygWin/MSYS shells on CI are masking a lot of path conversion issues that are explicit if we just run the cases using go test -bench 🤔

@evacchi
Copy link
Contributor Author

evacchi commented Dec 20, 2023

huh potential failure in src/reflect "due" to the perfmap feature, see https://github.com/tetratelabs/wazero/actions/runs/7267487881/job/19801438025?pr=1878

EDIT: oh it's just a nil pointer
EDIT: still weird since I had not rebased this branch yet; I don't think I can reproduce either 🤔

/opt/hostedtoolcache/go/1.21.5/x64/src/reflect ~/work/wazero/wazero
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x8 pc=0x22aaa0]

goroutine 1 [running]:
github.com/tetratelabs/wazero/internal/engine/wazevo/wazevoapi.(*Perfmap).Clear(...)
	/home/runner/work/wazero/wazero/internal/engine/wazevo/wazevoapi/perfmap.go:83
github.com/tetratelabs/wazero/internal/engine/wazevo/backend/isa/arm64.(*machine).ResolveRelativeAddresses(0x400014ab40, {0x367df8, 0x579d80})
	/home/runner/work/wazero/wazero/internal/engine/wazevo/backend/isa/arm64/machine.go:452 +0x200
github.com/tetratelabs/wazero/internal/engine/wazevo/backend.(*compiler).Finalize(0x4000071500, {0x367df8, 0x579d80})
	/home/runner/work/wazero/wazero/internal/engine/wazevo/backend/compiler.go:190 +0x6c
github.com/tetratelabs/wazero/internal/engine/wazevo/backend.(*compiler).Compile(0x4000071500, {0x367df8, 0x579d80})
	/home/runner/work/wazero/wazero/internal/engine/wazevo/backend/compiler.go:166 +0x40
github.com/tetratelabs/wazero/internal/engine/wazevo.(*engine).compileLocalWasmFunction(0x2b6da0?, {0x367df8, 0x579d80}, 0x400014ab40?, 0x36fa18?, 0x4000071500?, {0x371120, 0x400014a480}, {0x36fa18, 0x4000071500}, ...)
	/home/runner/work/wazero/wazero/internal/engine/wazevo/engine.go:359 +0x128
github.com/tetratelabs/wazero/internal/engine/wazevo.(*engine).compileModule(0x4000010420, {0x367df8, 0x579d80}, 0x4000082340, {0x0, 0x0, 0x0}, 0x0)
	/home/runner/work/wazero/wazero/internal/engine/wazevo/engine.go:234 +0x3c8
github.com/tetratelabs/wazero/internal/engine/wazevo.(*engine).CompileModule(0x4000082340?, {0x367df8, 0x579d80}, 0x4000082340, {0x0?, 0x0, 0x0}, 0x0?)
	/home/runner/work/wazero/wazero/internal/engine/wazevo/engine.go:133 +0x80
github.com/tetratelabs/wazero.(*runtime).CompileModule(0x4000065bc0, {0x367df8, 0x579d80}, {0x4000180000, 0x87a142, 0x87a143})
	/home/runner/work/wazero/wazero/runtime.go:237 +0x1a8
main.doRun({0x40000100d0, 0x9, 0x9}, {0x366898, 0x4000032030}, {0x367498?, 0x4000032038?})
	/home/runner/work/wazero/wazero/cmd/wazero/wazero.go:327 +0x10d4
main.doMain({0x366898, 0x4000032030}, {0x367498?, 0x4000032038?})
	/home/runner/work/wazero/wazero/cmd/wazero/wazero.go:62 +0x1bc
main.main()
	/home/runner/work/wazero/wazero/cmd/wazero/wazero.go:34 +0x3c

Signed-off-by: Edoardo Vacchi <[email protected]>
@evacchi
Copy link
Contributor Author

evacchi commented Dec 20, 2023

I think this is the best I can do at this point. Unfortunately, CI on Windows has a lot of quirks, it's better just to run the CLI.

@evacchi evacchi marked this pull request as ready for review December 20, 2023 13:34
@evacchi evacchi marked this pull request as draft December 20, 2023 13:43
@evacchi
Copy link
Contributor Author

evacchi commented Dec 20, 2023

redraft: maybe I found something...

Signed-off-by: Edoardo Vacchi <[email protected]>
Signed-off-by: Edoardo Vacchi <[email protected]>
Signed-off-by: Edoardo Vacchi <[email protected]>
@evacchi evacchi marked this pull request as ready for review December 20, 2023 17:25
Copy link
Member

@mathetake mathetake left a comment

Choose a reason for hiding this comment

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

thanks!

internal/integration_test/stdlibs/Makefile Show resolved Hide resolved
@mathetake mathetake merged commit 823d573 into main Dec 20, 2023
56 checks passed
@mathetake mathetake deleted the stdlib-bench branch December 20, 2023 18:42
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.

2 participants