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.

Sign up for GitHub

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

Jump to bottom

Benchamark Test panicking for GnoVM #2711

Closed
sw360cab opened this issue Aug 20, 2024 · 2 comments · Fixed by #2792
Closed

Benchamark Test panicking for GnoVM #2711

sw360cab opened this issue Aug 20, 2024 · 2 comments · Fixed by #2792
Labels
🐞 bug Something isn't working help wanted Want to contribute? We recommend these issues.

Comments

@sw360cab
Copy link
Contributor

Benchamark Test panicking for GnoVM

Description

There is a test a case in GnoVM package that panics and causes a failure of benchmark tests

Your environment

  • go1.22.4
  • darwin/amd64
  • Gno commit: 8bd07c971e7ed9ee94af03d7145f247224683694

Steps to reproduce

  • Launching
go test -benchmem -bench=. -run=^$ ./gnovm/...

the benchmark tests fail

  • Skipping BenchmarkPreprocess at gnovm/pkg/gnolang/gno_test.go, the benchmark tests succeed

Logs

BenchmarkPreprocess-12                  panic: name mx not declared [recovered]
        panic: /:0:0: name mx not declared:
--- preprocess stack ---
stack 1: func main() { mx<VPUverse(0)> := 1000000; for i<VPUverse(0)> := 0; i<VPUverse(0)> < mx<VPUverse(0)>; i<VPUverse(0)>++ {  } }
stack 0: package(main)
------------------------

goroutine 11 [running]:
github.com/gnolang/gno/gnovm/pkg/gnolang.Preprocess.func1.1()
       gno/gnovm/pkg/gnolang/preprocess.go:414 +0x35f
panic({0x82f62a0?, 0xc00036c090?})
        /Users/sergio/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.22.4.darwin-amd64/src/runtime/panic.go:770 +0x132
github.com/gnolang/gno/gnovm/pkg/gnolang.(*StaticBlock).GetPathForName(0xc000020030, {0x0, 0x0}, {0xc0000140d8, 0x2})
       gno/gnovm/pkg/gnolang/nodes.go:1651 +0x594
github.com/gnolang/gno/gnovm/pkg/gnolang.fillNameExprPath({0x83f9db8, 0xc000020008}, 0xc000118870, 0x2?)
       gno/gnovm/pkg/gnolang/preprocess.go:3718 +0xc2
github.com/gnolang/gno/gnovm/pkg/gnolang.Preprocess.func1({0xc000250600, 0x2, 0x20}, 0x23, 0x0, {0x83ef0f0, 0xc000118870}, 0x3)
       gno/gnovm/pkg/gnolang/preprocess.go:937 +0x138e
github.com/gnolang/gno/gnovm/pkg/gnolang.transcribe(0xc0000519e0, {0xc000250600, 0x2, 0x20}, 0x23, 0x0, {0x83ef0f0, 0xc000118870}, 0xc0000f940f)
       gno/gnovm/pkg/gnolang/transcribe.go:737 +0x1c0d
github.com/gnolang/gno/gnovm/pkg/gnolang.transcribe(0xc0000f99e0, {0xc000250600, 0x1, 0x20}, 0x46, 0x0, {0x83ef078, 0xc0001a9440}, 0xc0000f9697)
       gno/gnovm/pkg/gnolang/transcribe.go:364 +0x6c12
github.com/gnolang/gno/gnovm/pkg/gnolang.transcribe(0xc0000f99e0, {0xc000250600, 0x0, 0x20}, 0x0, 0x0, {0x83eef88, 0xc000020008}, 0xc0000f991f)
       gno/gnovm/pkg/gnolang/transcribe.go:679 +0x840b
github.com/gnolang/gno/gnovm/pkg/gnolang.Transcribe({0x83eef88, 0xc000020008}, 0xc0001359e0)
       gno/gnovm/pkg/gnolang/transcribe.go:133 +0xb6
github.com/gnolang/gno/gnovm/pkg/gnolang.Preprocess({0x0, 0x0}, {0x83f9958, 0xc000000308}, {0x83eef88, 0xc000020008?})
       gno/gnovm/pkg/gnolang/preprocess.go:388 +0x286
github.com/gnolang/gno/gnovm/pkg/gnolang.BenchmarkPreprocess(0xc0001dc788)
       gno/gnovm/pkg/gnolang/gno_test.go:366 +0x4bc
testing.(*B).runN(0xc0001dc788, 0x1)
        /Users/sergio/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.22.4.darwin-amd64/src/testing/benchmark.go:193 +0xf8
testing.(*B).run1.func1()
        /Users/sergio/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.22.4.darwin-amd64/src/testing/benchmark.go:215 +0x4e
created by testing.(*B).run1 in goroutine 1
        /Users/sergio/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.22.4.darwin-amd64/src/testing/benchmark.go:208 +0x90
exit status 2
FAIL    github.com/gnolang/gno/gnovm/pkg/gnolang        6.366s
@sw360cab
Copy link
Contributor Author

cc @thehowl

@sw360cab sw360cab mentioned this issue Aug 20, 2024
Merged
@zivkovicmilos zivkovicmilos added the help wanted Want to contribute? We recommend these issues. label Sep 12, 2024
thehowl added a commit that referenced this issue Sep 13, 2024
@thehowl
test(gnovm): run initStaticBlocks in BenchmarkPreprocess
1d3612c 
fixes #2711
@thehowl thehowl mentioned this issue Sep 13, 2024
Merged
7 tasks
thehowl added a commit that referenced this issue Sep 16, 2024
@thehowl
test(gnovm): run initStaticBlocks in BenchmarkPreprocess (#2792)
5503cca 
fixes #2711

The bug in the benchmark was introduced in #2418, which requires to run
initStaticBlocks before running Preprocess, in most cases. (in normal
scenarios, this is run by PredefineFileSet; however this benchmark is
deep into the internals).

Related: #2716. cc/ @sw360cab 

<!-- please provide a detailed description of the changes made in this
pull request. -->

<details><summary>Contributors' checklist...</summary>

- [ ] Added new tests, or not needed, or not feasible
- [ ] Provided an example (e.g. screenshot) to aid review or the PR is
self-explanatory
- [ ] Updated the official documentation or not needed
- [ ] No breaking changes were made, or a `BREAKING CHANGE: xxx` message
was included in the description
- [ ] Added references to related issues and PRs
- [ ] Provided any useful hints for running manual tests
- [ ] Added new benchmarks to [generated
graphs](https://gnoland.github.io/benchmarks), if any. More info
[here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md).
</details>
sw360cab added a commit that referenced this issue Sep 26, 2024
@sw360cab
chore(benchmarks): Rehauling Benchmark Workloads (#2716)
577c462 
## Things changes

* using new action plugin:
[benchmark-action/github-action-benchmark](https://github.com/benchmark-action/github-action-benchmark)
* previous library was
[bobheadxi/gobenchdata](https://github.com/bobheadxi/gobenchdata)
* creating alerts when benchmarks results are worse

## Things unchanged

* Pushing benchmarks results onto a local branch, then `benchmark`
repository will fetch from this into his own `gh-pages`
  * direct push is possible but requires PAT token to be used 

## Things TODO (before merging)

* restore benchmarks actions which were manually disabled directly in
the `Action` menu in Github
* drop and recreate branch  `gh-repository`
* fix benchmark tests in Gno repo (see #2711 and #2714)
* Evaluate alternative configuration possibilities
[here](https://github.com/benchmark-action/github-action-benchmark?tab=readme-ov-file#action-inputs)

## Things to consider (performance)

* Should solve #2432 
* are resources available in the Github basic runners enough for
benchmarks?
* are multiple runs spawned in the same hw conditions?
* consider using [Larger
Runner](https://docs.github.com/en/actions/using-github-hosted-runners/using-larger-runners/managing-larger-runners#adding-a-larger-runner-to-an-organization)
or restoring a [Self-Hosted
Runner](https://docs.github.com/en/actions/hosting-your-own-runners/managing-self-hosted-runners/adding-self-hosted-runners#adding-a-self-hosted-runner-to-an-organization)
@linear linear bot reopened this Nov 1, 2024
@thehowl
Copy link
Member

thehowl commented Nov 6, 2024

Linear re-opened this, but it's incorrect.

@thehowl thehowl closed this as completed Nov 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
🐞 bug Something isn't working help wanted Want to contribute? We recommend these issues.
Projects
🧙‍♂️gno.land core team
Archived in project
🚀 main.gno.land launch 🚀
Status: Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

test(gnovm): run initStaticBlocks in BenchmarkPreprocess gnolang/gno
3 participants
@sw360cab @thehowl @zivkovicmilos