From 43c9116c22de8518055b3eaf94c112ee99377a1e Mon Sep 17 00:00:00 2001 From: Morgan Date: Mon, 2 Dec 2024 16:23:37 +0100 Subject: [PATCH] test(gnovm): test performance improvements (#3210) Fix master CI runs, and miscellaneous improvements locally, too. - ci: switch to using `-covermode=set` rather than atomic, as it significantly degrades performance while not being shown on codecov. [more info](https://github.com/gnolang/gno/pull/3210#issuecomment-2511455953) - gnolang tests: use `t.Parallel()` to parallelize known "long" tests, both in `-short` and long versions. - stdlibs: provide `unicode` native shims for some common functions used in some standard library tests. This may lead to some small inconsistencies between on-chain behaviour and off-chain should the `unicode` packages diverge; but I think we might we might want to consider a native-based `unicode` stdlib, anyway. - thanks to these improvements, there is no longer the need to run `-short` on PRs, as the CI runs in ~9 mins, ie. 8 minutes less than the gno.land tests. --- .github/workflows/gnovm.yml | 2 - .github/workflows/test_template.yml | 5 +- gnovm/pkg/gnolang/files_test.go | 79 ++++++++++++---- gnovm/pkg/test/test.go | 2 + gnovm/stdlibs/bytes/compare_test.gno | 2 + gnovm/tests/stdlibs/generated.go | 114 ++++++++++++++++++++++++ gnovm/tests/stdlibs/unicode/natives.gno | 8 ++ gnovm/tests/stdlibs/unicode/natives.go | 8 ++ 8 files changed, 198 insertions(+), 22 deletions(-) create mode 100644 gnovm/tests/stdlibs/unicode/natives.gno create mode 100644 gnovm/tests/stdlibs/unicode/natives.go diff --git a/.github/workflows/gnovm.yml b/.github/workflows/gnovm.yml index 8311d113047..7e7586b23d9 100644 --- a/.github/workflows/gnovm.yml +++ b/.github/workflows/gnovm.yml @@ -13,8 +13,6 @@ jobs: uses: ./.github/workflows/main_template.yml with: modulepath: "gnovm" - # in pull requests, append -short so that the CI runs quickly. - tests-extra-args: ${{ github.event_name == 'pull_request' && '-short' || '' }} secrets: codecov-token: ${{ secrets.CODECOV_TOKEN }} fmt: diff --git a/.github/workflows/test_template.yml b/.github/workflows/test_template.yml index ccbae792c78..c7956b4caf4 100644 --- a/.github/workflows/test_template.yml +++ b/.github/workflows/test_template.yml @@ -41,11 +41,14 @@ jobs: # Craft a filter flag based on the module path to avoid expanding coverage on unrelated tags. export filter="-pkg=github.com/gnolang/gno/${{ inputs.modulepath }}/..." + # codecov only supports "boolean" coverage (whether a line is + # covered or not); so using -covermode=count or atomic would be + # pointless here. # XXX: Simplify coverage of txtar - the current setup is a bit # confusing and meticulous. There will be some improvements in Go # 1.23 regarding coverage, so we can use this as a workaround until # then. - go test -covermode=atomic -timeout ${{ inputs.tests-timeout }} ${{ inputs.tests-extra-args }} ./... -test.gocoverdir=$GOCOVERDIR + go test -covermode=set -timeout ${{ inputs.tests-timeout }} ${{ inputs.tests-extra-args }} ./... -test.gocoverdir=$GOCOVERDIR # Print results (set +x; echo 'go coverage results:') diff --git a/gnovm/pkg/gnolang/files_test.go b/gnovm/pkg/gnolang/files_test.go index f1bc87d21d8..09be600b198 100644 --- a/gnovm/pkg/gnolang/files_test.go +++ b/gnovm/pkg/gnolang/files_test.go @@ -33,19 +33,26 @@ func (nopReader) Read(p []byte) (int, error) { return 0, io.EOF } // fix a specific test: // go test -run TestFiles/'^bin1.gno' -short -v -update-golden-tests . func TestFiles(t *testing.T) { + t.Parallel() + rootDir, err := filepath.Abs("../../../") require.NoError(t, err) - opts := &test.TestOptions{ - RootDir: rootDir, - Output: io.Discard, - Error: io.Discard, - Sync: *withSync, + newOpts := func() *test.TestOptions { + o := &test.TestOptions{ + RootDir: rootDir, + Output: io.Discard, + Error: io.Discard, + Sync: *withSync, + } + o.BaseStore, o.TestStore = test.Store( + rootDir, true, + nopReader{}, o.WriterForStore(), io.Discard, + ) + return o } - opts.BaseStore, opts.TestStore = test.Store( - rootDir, true, - nopReader{}, opts.WriterForStore(), io.Discard, - ) + // sharedOpts is used for all "short" tests. + sharedOpts := newOpts() dir := "../../tests/" fsys := os.DirFS(dir) @@ -59,7 +66,8 @@ func TestFiles(t *testing.T) { return nil } subTestName := path[len("files/"):] - if strings.HasSuffix(path, "_long.gno") && testing.Short() { + isLong := strings.HasSuffix(path, "_long.gno") + if isLong && testing.Short() { t.Run(subTestName, func(t *testing.T) { t.Skip("skipping in -short") }) @@ -73,6 +81,12 @@ func TestFiles(t *testing.T) { var criticalError error t.Run(subTestName, func(t *testing.T) { + opts := sharedOpts + if isLong { + // Long tests are run in parallel, and with their own store. + t.Parallel() + opts = newOpts() + } changed, err := opts.RunFiletest(path, content) if err != nil { t.Fatal(err.Error()) @@ -94,16 +108,24 @@ func TestFiles(t *testing.T) { // TestStdlibs tests all the standard library packages. func TestStdlibs(t *testing.T) { + t.Parallel() + rootDir, err := filepath.Abs("../../../") require.NoError(t, err) - var capture bytes.Buffer - out := io.Writer(&capture) - if testing.Verbose() { - out = os.Stdout + newOpts := func() (capture *bytes.Buffer, opts *test.TestOptions) { + var out io.Writer + if testing.Verbose() { + out = os.Stdout + } else { + capture = new(bytes.Buffer) + out = capture + } + opts = test.NewTestOptions(rootDir, nopReader{}, out, out) + opts.Verbose = true + return } - opts := test.NewTestOptions(rootDir, nopReader{}, out, out) - opts.Verbose = true + sharedCapture, sharedOpts := newOpts() dir := "../../stdlibs/" fsys := os.DirFS(dir) @@ -118,12 +140,31 @@ func TestStdlibs(t *testing.T) { fp := filepath.Join(dir, path) memPkg := gnolang.ReadMemPackage(fp, path) t.Run(strings.ReplaceAll(memPkg.Path, "/", "-"), func(t *testing.T) { - if testing.Short() { - switch memPkg.Path { - case "bytes", "strconv", "regexp/syntax": + capture, opts := sharedCapture, sharedOpts + switch memPkg.Path { + // Excluded in short + case + "bufio", + "bytes", + "strconv": + if testing.Short() { t.Skip("Skipped because of -short, and this stdlib is very long currently.") } + fallthrough + // Run using separate store, as it's faster + case + "math/rand", + "regexp", + "regexp/syntax", + "sort": + t.Parallel() + capture, opts = newOpts() + } + + if capture != nil { + capture.Reset() } + err := test.Test(memPkg, "", opts) if !testing.Verbose() { t.Log(capture.String()) diff --git a/gnovm/pkg/test/test.go b/gnovm/pkg/test/test.go index 9374db263ee..5de37a68405 100644 --- a/gnovm/pkg/test/test.go +++ b/gnovm/pkg/test/test.go @@ -284,6 +284,8 @@ func (opts *TestOptions) runTestFiles( if opts.Metrics { alloc = gno.NewAllocator(math.MaxInt64) } + // reset store ops, if any - we only need them for some filetests. + opts.TestStore.SetLogStoreOps(false) // Check if we already have the package - it may have been eagerly // loaded. diff --git a/gnovm/stdlibs/bytes/compare_test.gno b/gnovm/stdlibs/bytes/compare_test.gno index f2b1e7c692b..5ebeba33889 100644 --- a/gnovm/stdlibs/bytes/compare_test.gno +++ b/gnovm/stdlibs/bytes/compare_test.gno @@ -66,6 +66,8 @@ func TestCompareIdenticalSlice(t *testing.T) { } func TestCompareBytes(t *testing.T) { + t.Skip("This test takes very long to run on Gno at time of writing, even in its short form") + lengths := make([]int, 0) // lengths to test in ascending order for i := 0; i <= 128; i++ { lengths = append(lengths, i) diff --git a/gnovm/tests/stdlibs/generated.go b/gnovm/tests/stdlibs/generated.go index 2cc904a9170..db5ecdec05d 100644 --- a/gnovm/tests/stdlibs/generated.go +++ b/gnovm/tests/stdlibs/generated.go @@ -9,6 +9,7 @@ import ( gno "github.com/gnolang/gno/gnovm/pkg/gnolang" testlibs_std "github.com/gnolang/gno/gnovm/tests/stdlibs/std" testlibs_testing "github.com/gnolang/gno/gnovm/tests/stdlibs/testing" + testlibs_unicode "github.com/gnolang/gno/gnovm/tests/stdlibs/unicode" ) // NativeFunc represents a function in the standard library which has a native @@ -325,6 +326,118 @@ var nativeFuncs = [...]NativeFunc{ func(m *gno.Machine) { r0 := testlibs_testing.X_unixNano() + m.PushValue(gno.Go2GnoValue( + m.Alloc, + m.Store, + reflect.ValueOf(&r0).Elem(), + )) + }, + }, + { + "unicode", + "IsPrint", + []gno.FieldTypeExpr{ + {Name: gno.N("p0"), Type: gno.X("rune")}, + }, + []gno.FieldTypeExpr{ + {Name: gno.N("r0"), Type: gno.X("bool")}, + }, + false, + func(m *gno.Machine) { + b := m.LastBlock() + var ( + p0 rune + rp0 = reflect.ValueOf(&p0).Elem() + ) + + gno.Gno2GoValue(b.GetPointerTo(nil, gno.NewValuePathBlock(1, 0, "")).TV, rp0) + + r0 := testlibs_unicode.IsPrint(p0) + + m.PushValue(gno.Go2GnoValue( + m.Alloc, + m.Store, + reflect.ValueOf(&r0).Elem(), + )) + }, + }, + { + "unicode", + "IsGraphic", + []gno.FieldTypeExpr{ + {Name: gno.N("p0"), Type: gno.X("rune")}, + }, + []gno.FieldTypeExpr{ + {Name: gno.N("r0"), Type: gno.X("bool")}, + }, + false, + func(m *gno.Machine) { + b := m.LastBlock() + var ( + p0 rune + rp0 = reflect.ValueOf(&p0).Elem() + ) + + gno.Gno2GoValue(b.GetPointerTo(nil, gno.NewValuePathBlock(1, 0, "")).TV, rp0) + + r0 := testlibs_unicode.IsGraphic(p0) + + m.PushValue(gno.Go2GnoValue( + m.Alloc, + m.Store, + reflect.ValueOf(&r0).Elem(), + )) + }, + }, + { + "unicode", + "SimpleFold", + []gno.FieldTypeExpr{ + {Name: gno.N("p0"), Type: gno.X("rune")}, + }, + []gno.FieldTypeExpr{ + {Name: gno.N("r0"), Type: gno.X("rune")}, + }, + false, + func(m *gno.Machine) { + b := m.LastBlock() + var ( + p0 rune + rp0 = reflect.ValueOf(&p0).Elem() + ) + + gno.Gno2GoValue(b.GetPointerTo(nil, gno.NewValuePathBlock(1, 0, "")).TV, rp0) + + r0 := testlibs_unicode.SimpleFold(p0) + + m.PushValue(gno.Go2GnoValue( + m.Alloc, + m.Store, + reflect.ValueOf(&r0).Elem(), + )) + }, + }, + { + "unicode", + "IsUpper", + []gno.FieldTypeExpr{ + {Name: gno.N("p0"), Type: gno.X("rune")}, + }, + []gno.FieldTypeExpr{ + {Name: gno.N("r0"), Type: gno.X("bool")}, + }, + false, + func(m *gno.Machine) { + b := m.LastBlock() + var ( + p0 rune + rp0 = reflect.ValueOf(&p0).Elem() + ) + + gno.Gno2GoValue(b.GetPointerTo(nil, gno.NewValuePathBlock(1, 0, "")).TV, rp0) + + r0 := testlibs_unicode.IsUpper(p0) + m.PushValue(gno.Go2GnoValue( m.Alloc, m.Store, @@ -337,6 +450,7 @@ var nativeFuncs = [...]NativeFunc{ var initOrder = [...]string{ "std", "testing", + "unicode", } // InitOrder returns the initialization order of the standard libraries. diff --git a/gnovm/tests/stdlibs/unicode/natives.gno b/gnovm/tests/stdlibs/unicode/natives.gno new file mode 100644 index 00000000000..c7efaac70cc --- /dev/null +++ b/gnovm/tests/stdlibs/unicode/natives.gno @@ -0,0 +1,8 @@ +package unicode + +// Optimized as native bindings in tests. + +func IsPrint(r rune) bool +func IsGraphic(r rune) bool +func SimpleFold(r rune) rune +func IsUpper(r rune) bool diff --git a/gnovm/tests/stdlibs/unicode/natives.go b/gnovm/tests/stdlibs/unicode/natives.go new file mode 100644 index 00000000000..e627f4fe6be --- /dev/null +++ b/gnovm/tests/stdlibs/unicode/natives.go @@ -0,0 +1,8 @@ +package unicode + +import "unicode" + +func IsPrint(r rune) bool { return unicode.IsPrint(r) } +func IsGraphic(r rune) bool { return unicode.IsGraphic(r) } +func SimpleFold(r rune) rune { return unicode.SimpleFold(r) } +func IsUpper(r rune) bool { return unicode.IsUpper(r) }