From efb4521d0770a5b95f6b302723864fdfaa0c6cb1 Mon Sep 17 00:00:00 2001 From: Bob Glickstein Date: Thu, 14 Nov 2024 07:11:26 -0800 Subject: [PATCH] Optimizations based on commutativity of identical (#27) * Optimization based on commutativity of identical. * Oops. * "go mod tidy" after merge --- cmd/modver/compare.go | 2 +- cmd/modver/options.go | 2 +- cmd/modver/tags.go | 2 +- embed_test.go | 2 +- go.mod | 2 +- go.sum | 2 + identical.go | 10 ++- internal/github.go | 2 +- internal/pr.go | 2 +- modver_test.go | 204 ++++++++++++++++++++++++------------------ 10 files changed, 137 insertions(+), 93 deletions(-) diff --git a/cmd/modver/compare.go b/cmd/modver/compare.go index d78d719..031b183 100644 --- a/cmd/modver/compare.go +++ b/cmd/modver/compare.go @@ -5,8 +5,8 @@ import ( "fmt" "os" + "github.com/bobg/errors" "github.com/google/go-github/v50/github" - "github.com/pkg/errors" "github.com/bobg/modver/v2" "github.com/bobg/modver/v2/internal" diff --git a/cmd/modver/options.go b/cmd/modver/options.go index c69402d..d0d0561 100644 --- a/cmd/modver/options.go +++ b/cmd/modver/options.go @@ -6,7 +6,7 @@ import ( "os" "strings" - "github.com/pkg/errors" + "github.com/bobg/errors" "golang.org/x/mod/semver" ) diff --git a/cmd/modver/tags.go b/cmd/modver/tags.go index 7190b6b..98c2d25 100644 --- a/cmd/modver/tags.go +++ b/cmd/modver/tags.go @@ -6,11 +6,11 @@ import ( "os" "strings" + "github.com/bobg/errors" "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/plumbing" "github.com/go-git/go-git/v5/plumbing/object" "github.com/go-git/go-git/v5/plumbing/storer" - "github.com/pkg/errors" "golang.org/x/mod/semver" "github.com/bobg/modver/v2" diff --git a/embed_test.go b/embed_test.go index 1c6d26d..90c43a7 100644 --- a/embed_test.go +++ b/embed_test.go @@ -8,7 +8,7 @@ import ( "path/filepath" "strings" - "github.com/pkg/errors" + "github.com/bobg/errors" "golang.org/x/tools/go/packages" ) diff --git a/go.mod b/go.mod index f5d6304..4e6df60 100644 --- a/go.mod +++ b/go.mod @@ -3,9 +3,9 @@ module github.com/bobg/modver/v2 go 1.22 require ( + github.com/bobg/errors v1.1.0 github.com/go-git/go-git/v5 v5.12.0 github.com/google/go-github/v50 v50.2.0 - github.com/pkg/errors v0.9.1 golang.org/x/mod v0.20.0 golang.org/x/oauth2 v0.22.0 golang.org/x/tools v0.24.0 diff --git a/go.sum b/go.sum index e067650..2a40eeb 100644 --- a/go.sum +++ b/go.sum @@ -9,6 +9,8 @@ github.com/anmitsu/go-shlex v0.0.0-20200514113438-38f4b401e2be h1:9AeTilPcZAjCFI github.com/anmitsu/go-shlex v0.0.0-20200514113438-38f4b401e2be/go.mod h1:ySMOLuWl6zY27l47sB3qLNK6tF2fkHG55UZxx8oIVo4= github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5 h1:0CwZNZbxp69SHPdPJAN/hZIm0C4OItdklCFmMRWYpio= github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5/go.mod h1:wHh0iHkYZB8zMSxRWpUBQtwG5a7fFgvEO+odwuTv2gs= +github.com/bobg/errors v1.1.0 h1:gsVanPzJMpZQpwY+27/GQYElZez5CuMYwiIpk2A3RGw= +github.com/bobg/errors v1.1.0/go.mod h1:Q4775qBZpnte7EGFJqmvnlB1U4pkI1XmU3qxqdp7Zcc= github.com/bwesterb/go-ristretto v1.2.3/go.mod h1:fUIoIZaG73pV5biE2Blr2xEzDoMj7NFEuV9ekS419A0= github.com/cloudflare/circl v1.3.3/go.mod h1:5XYMA4rFBvNIrhs50XuiBJ15vF2pZn4nnUKZrLbUZFA= github.com/cloudflare/circl v1.3.9 h1:QFrlgFYf2Qpi8bSpVPK1HBvWpx16v/1TZivyo7pGuBE= diff --git a/identical.go b/identical.go index 7a5ef9b..6452251 100644 --- a/identical.go +++ b/identical.go @@ -8,6 +8,11 @@ func (c *comparer) identical(a, b types.Type) (res bool) { if res, ok := c.identicache[tp]; ok { return res } + // identical(a, b) = identical(b, a) + tp.a, tp.b = b, a + if res, ok := c.identicache[tp]; ok { + return res + } defer func() { c.identicache[tp] = res }() if types.Identical(a, b) { @@ -20,8 +25,11 @@ func (c *comparer) identical(a, b types.Type) (res bool) { if a == pair.a && b == pair.b { return true } + if a == pair.b && b == pair.a { + return true + } } - c.stack = append(c.stack, typePair{a: a, b: b}) + c.stack = append(c.stack, tp) defer func() { c.stack = c.stack[:len(c.stack)-1] }() if na, ok := a.(*types.Named); ok { diff --git a/internal/github.go b/internal/github.go index f50625c..71be4ab 100644 --- a/internal/github.go +++ b/internal/github.go @@ -7,8 +7,8 @@ import ( "strconv" "strings" + "github.com/bobg/errors" "github.com/google/go-github/v50/github" - "github.com/pkg/errors" "golang.org/x/oauth2" ) diff --git a/internal/pr.go b/internal/pr.go index ca7c52b..2e81097 100644 --- a/internal/pr.go +++ b/internal/pr.go @@ -10,8 +10,8 @@ import ( "strings" "text/template" + "github.com/bobg/errors" "github.com/google/go-github/v50/github" - "github.com/pkg/errors" "github.com/bobg/modver/v2" ) diff --git a/modver_test.go b/modver_test.go index 625046f..02c308b 100644 --- a/modver_test.go +++ b/modver_test.go @@ -4,7 +4,6 @@ import ( "bufio" "bytes" "context" - "errors" "fmt" "io" "log" @@ -13,9 +12,19 @@ import ( "strings" "testing" "text/template" + + "github.com/bobg/errors" ) func TestCompare(t *testing.T) { + tbCompare(t) +} + +func BenchmarkCompare(b *testing.B) { + tbCompare(b) +} + +func tbCompare(tb testing.TB) { cases := []struct { dir string want ResultCode @@ -30,15 +39,26 @@ func TestCompare(t *testing.T) { }} for _, c := range cases { - runtest(t, c.dir, c.want) + runtest(tb, c.dir, c.want) + } +} + +func tbRun(tb testing.TB, name string, f func(testing.TB)) { + switch tb := tb.(type) { + case *testing.T: + tb.Run(name, func(t *testing.T) { f(tb) }) + case *testing.B: + tb.Run(name, func(b *testing.B) { f(tb) }) } } -func runtest(t *testing.T, typ string, want ResultCode) { +func runtest(tb testing.TB, typ string, want ResultCode) { + b, _ := tb.(*testing.B) + tree := filepath.Join("testdata", typ) entries, err := os.ReadDir(tree) if err != nil { - t.Fatal(err) + tb.Fatal(err) } for _, entry := range entries { if entry.IsDir() { @@ -48,103 +68,117 @@ func runtest(t *testing.T, typ string, want ResultCode) { continue } name := strings.TrimSuffix(entry.Name(), ".tmpl") - t.Run(fmt.Sprintf("%s/%s", typ, name), func(t *testing.T) { - tmpls, err := template.ParseFiles(filepath.Join(tree, entry.Name())) - if err != nil { - t.Fatal(err) - } + tbRun(tb, fmt.Sprintf("%s/%s", typ, name), func(tb testing.TB) { + err := withTestDirs(tree, name, func(olderTestDir, newerTestDir string) { + if b != nil { + b.ResetTimer() + for i := 0; i < b.N; i++ { + _, err := CompareDirs(olderTestDir, newerTestDir) + if err != nil { + b.Fatal(err) + } + } + return + } - tmpdir, err := os.MkdirTemp("", "modver") + got, err := CompareDirs(olderTestDir, newerTestDir) + if err != nil { + tb.Fatal(err) + } + if got.Code() != want { + tb.Errorf("want %s, got %s", want, got) + } else { + tb.Log(got) + } + }) if err != nil { - t.Fatal(err) + tb.Fatal(err) } - defer os.RemoveAll(tmpdir) + }) + } +} - var ( - olderTestDir = filepath.Join(tmpdir, "older") - newerTestDir = filepath.Join(tmpdir, "newer") - ) +func withTestDirs(tree, name string, f func(olderTestDir, newerTestDir string)) error { + tmpls, err := template.ParseFiles(filepath.Join(tree, name+".tmpl")) + if err != nil { + return errors.Wrap(err, "parsing templates") + } - err = os.Mkdir(olderTestDir, 0755) - if err != nil { - t.Fatal(err) - } - err = os.Mkdir(newerTestDir, 0755) - if err != nil { - t.Fatal(err) - } + tmpdir, err := os.MkdirTemp("", "modver") + if err != nil { + return errors.Wrap(err, "creating temp dir") + } + defer os.RemoveAll(tmpdir) - var sawGomod bool - for _, tmpl := range tmpls.Templates() { - // Skip the top-level template - if strings.HasSuffix(tmpl.Name(), ".tmpl") { - continue - } + var ( + olderTestDir = filepath.Join(tmpdir, "older") + newerTestDir = filepath.Join(tmpdir, "newer") + ) - sawGomod = sawGomod || (filepath.Base(tmpl.Name()) == "go.mod") + if err = os.Mkdir(olderTestDir, 0755); err != nil { + return errors.Wrap(err, "creating older test dir") + } + if err = os.Mkdir(newerTestDir, 0755); err != nil { + return errors.Wrap(err, "creating newer test dir") + } - parts := strings.Split(tmpl.Name(), "/") - if !strings.Contains(parts[len(parts)-1], ".") { - parts = append(parts, "x.go") - } + var sawGomod bool + for _, tmpl := range tmpls.Templates() { + // Skip the top-level template + if strings.HasSuffix(tmpl.Name(), ".tmpl") { + continue + } - if len(parts) == 1 { - // Only a filename is given. - // Write it to both older and newer dirs. - buf := new(bytes.Buffer) - err = executeTmpl(tmpl, buf) - if err != nil { - t.Fatal(err) - } - for _, subdir := range []string{olderTestDir, newerTestDir} { - filename := filepath.Join(subdir, parts[0]) - err = os.WriteFile(filename, buf.Bytes(), 0644) - if err != nil { - t.Fatal(err) - } - } - continue - } + sawGomod = sawGomod || (filepath.Base(tmpl.Name()) == "go.mod") - if len(parts) > 1 { - dirparts := append([]string{tmpdir}, parts[:len(parts)-1]...) - dirname := filepath.Join(dirparts...) - err = os.MkdirAll(dirname, 0755) - if err != nil { - t.Fatal(err) - } - } - fileparts := append([]string{tmpdir}, parts...) - filename := filepath.Join(fileparts...) - err = executeTmplToFile(tmpl, filename) - if err != nil { - t.Fatal(err) - } + parts := strings.Split(tmpl.Name(), "/") + if !strings.Contains(parts[len(parts)-1], ".") { + parts = append(parts, "x.go") + } + + if len(parts) == 1 { + // Only a filename is given. + // Write it to both older and newer dirs. + buf := new(bytes.Buffer) + if err = executeTmpl(tmpl, buf); err != nil { + return errors.Wrap(err, "executing template") } - if !sawGomod { - buf := new(bytes.Buffer) - fmt.Fprintf(buf, "module %s\n\ngo 1.18\n", name) - err = os.WriteFile(filepath.Join(olderTestDir, "go.mod"), buf.Bytes(), 0644) - if err != nil { - t.Fatal(err) - } - err = os.WriteFile(filepath.Join(newerTestDir, "go.mod"), buf.Bytes(), 0644) - if err != nil { - t.Fatal(err) + for _, subdir := range []string{olderTestDir, newerTestDir} { + filename := filepath.Join(subdir, parts[0]) + if err = os.WriteFile(filename, buf.Bytes(), 0644); err != nil { + return errors.Wrapf(err, "writing file %s", filename) } } + continue + } - got, err := CompareDirs(olderTestDir, newerTestDir) - if err != nil { - t.Fatal(err) + if len(parts) > 1 { + dirparts := append([]string{tmpdir}, parts[:len(parts)-1]...) + dirname := filepath.Join(dirparts...) + if err = os.MkdirAll(dirname, 0755); err != nil { + return errors.Wrapf(err, "creating dir %s", dirname) } - if got.Code() != want { - t.Errorf("want %s, got %s", want, got) - } else { - t.Log(got) - } - }) + } + fileparts := append([]string{tmpdir}, parts...) + filename := filepath.Join(fileparts...) + if err = executeTmplToFile(tmpl, filename); err != nil { + return errors.Wrapf(err, "executing template to file %s", filename) + } } + if !sawGomod { + buf := new(bytes.Buffer) + fmt.Fprintf(buf, "module %s\n\ngo 1.18\n", name) + if err = os.WriteFile(filepath.Join(olderTestDir, "go.mod"), buf.Bytes(), 0644); err != nil { + return errors.Wrap(err, "writing older go.mod") + } + if err = os.WriteFile(filepath.Join(newerTestDir, "go.mod"), buf.Bytes(), 0644); err != nil { + return errors.Wrap(err, "writing newer go.mod") + } + } + + f(olderTestDir, newerTestDir) + + return nil } func executeTmpl(tmpl *template.Template, w io.Writer) error {