From fb1dff23b904ca7e358434449806e6d03a50d425 Mon Sep 17 00:00:00 2001 From: wgroeneveld Date: Mon, 15 Apr 2024 20:32:15 +0200 Subject: [PATCH] benchmarks protojson + http.json; added repo integration test --- README.md | 45 ++++++++++++++++++++++++++- go.mod | 3 ++ pokemon/repo_test.go | 69 ++++++++++++++++++++++++++++++++++++++++++ rest/restutils.go | 2 +- rest/restutils_test.go | 53 ++++++++++++++++++++++++++++++++ 5 files changed, 170 insertions(+), 2 deletions(-) create mode 100644 pokemon/repo_test.go create mode 100644 rest/restutils_test.go diff --git a/README.md b/README.md index c31d3bc..024e458 100644 --- a/README.md +++ b/README.md @@ -14,14 +14,57 @@ Why? Separation of concerns; do not expose database/domain internals. Structs in - JSON mapping: https://protobuf.dev/programming-guides/proto3/#json - `Pokemons` as object with `entries` is ugly but see https://github.com/golang/protobuf/issues/675 - no way to convert a slice to a protobuf message... +Is this over-engineered or what? On top of that, protojson's performance is much worse: + +``` +goos: darwin +goarch: arm64 +pkg: pokedex/rest +BenchmarkJson-8 1275931 928.9 ns/op +BenchmarkJson-8 1292184 926.6 ns/op +BenchmarkJson-8 1292647 926.0 ns/op +BenchmarkJson-8 1291228 926.3 ns/op +BenchmarkJson-8 1294453 927.3 ns/op +BenchmarkProtoJson-8 772567 1510 ns/op +BenchmarkProtoJson-8 766005 1517 ns/op +BenchmarkProtoJson-8 779281 1512 ns/op +BenchmarkProtoJson-8 774534 1513 ns/op +BenchmarkProtoJson-8 771370 1511 ns/op +PASS +ok pokedex/rest 16.500s +``` + +And that's **with indent**! Without: + +``` +BenchmarkJson-8 4067257 266.6 ns/op +BenchmarkJson-8 4503345 266.5 ns/op +BenchmarkJson-8 4512295 266.9 ns/op +BenchmarkJson-8 4505510 266.7 ns/op +BenchmarkJson-8 4482391 267.4 ns/op +``` + +The question then becomes: why use a `.proto` file to exchange a contract at all, if you're not using gRPC? + ## Swagger - Exposed at `http://localhost:8080/docs` - Regenerate with `swag init`, see https://github.com/swaggo/http-swagger - Annotation format: see https://github.com/swaggo/swag +Big bummer: annotations contain endpoint duplication... + ## Optimizing the binary See `Makefile`; use https://upx.github.io/ to package after stripping some debug info. -Somehow doesn't work on OSX (process killed)? \ No newline at end of file +Somehow doesn't work on OSX (process killed)? + +## Interesting philosophical questions + +1. Where should routing be defined? Central or with the domain package (which doesn't seem to be the idiomatic Go way?) +2. To use or not use DTO or response objects? +3. How to handle dependencies without dragging in _Yet Another Unneeded Framework_? +4. Custom error wrapping or not? +5. Custom Go web frameworks that ease muxing and exponential back-offs or not? +6. Where should db migration go? Reusability in tests? \ No newline at end of file diff --git a/go.mod b/go.mod index 064a50d..9564a6a 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,7 @@ module pokedex go 1.22 require ( + github.com/stretchr/testify v1.9.0 github.com/swaggo/http-swagger v1.3.4 github.com/swaggo/swag v1.16.3 google.golang.org/protobuf v1.33.0 @@ -12,6 +13,7 @@ require ( require ( github.com/KyleBanks/depth v1.2.1 // indirect + github.com/davecgh/go-spew v1.1.1 // indirect github.com/go-openapi/jsonpointer v0.21.0 // indirect github.com/go-openapi/jsonreference v0.21.0 // indirect github.com/go-openapi/spec v0.21.0 // indirect @@ -21,6 +23,7 @@ require ( github.com/josharian/intern v1.0.0 // indirect github.com/mailru/easyjson v0.7.7 // indirect github.com/mattn/go-sqlite3 v1.14.22 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect github.com/swaggo/files v1.0.1 // indirect golang.org/x/net v0.24.0 // indirect golang.org/x/tools v0.20.0 // indirect diff --git a/pokemon/repo_test.go b/pokemon/repo_test.go new file mode 100644 index 0000000..d701365 --- /dev/null +++ b/pokemon/repo_test.go @@ -0,0 +1,69 @@ +package pokemon + +import ( + "github.com/stretchr/testify/assert" + "gorm.io/driver/sqlite" + "gorm.io/gorm" + "testing" +) + +func pokeTest(name string) pokemon { + return pokemon{ + Name: name, + Weight: 124, + Height: 356, + } +} + +func TestRepo_Find(t *testing.T) { + cases := []struct { + label string + searchFor string + pokes []pokemon + expectResult pokemon + }{ + { + label: "No pokes in db", + searchFor: "Jos", + pokes: []pokemon{}, + expectResult: pokemon{}, + }, + { + label: "No pokes matching the name in db", + searchFor: "Jos", + pokes: []pokemon{ + pokeTest("Snul"), + pokeTest("Muesli"), + pokeTest("Bonnie"), + }, + expectResult: pokemon{}, + }, + { + label: "Unique poke name found", + searchFor: "Jos", + pokes: []pokemon{ + pokeTest("Jos"), + pokeTest("Miel"), + }, + expectResult: pokeTest("Jos"), + }, + } + + for _, tc := range cases { + t.Run(tc.label, func(t *testing.T) { + db, err := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{}) + assert.NoError(t, err) + db.AutoMigrate(&pokemon{}) + repo := NewRepo(db) + + for _, p := range tc.pokes { + db.Save(&p) + } + + result, err := repo.Find(tc.searchFor) + assert.NoError(t, err) + assert.Equal(t, tc.expectResult.Name, result.Name) + assert.Equal(t, tc.expectResult.Weight, result.Weight) + }) + } +} diff --git a/rest/restutils.go b/rest/restutils.go index 8b7f700..512e85d 100644 --- a/rest/restutils.go +++ b/rest/restutils.go @@ -8,7 +8,7 @@ import ( ) func Json(w http.ResponseWriter, data any) { - bytes, err := json.MarshalIndent(data, "", " ") + bytes, err := json.Marshal(data) if err != nil { http.Error(w, "Oops, something went wrong", http.StatusInternalServerError) return diff --git a/rest/restutils_test.go b/rest/restutils_test.go new file mode 100644 index 0000000..4f8705b --- /dev/null +++ b/rest/restutils_test.go @@ -0,0 +1,53 @@ +package rest + +import ( + "net/http" + "pokedex/pb" + "testing" +) + +type EmptyResponseWriter struct{} + +func (w EmptyResponseWriter) Header() http.Header { + return nil +} +func (w EmptyResponseWriter) Write([]byte) (int, error) { + return 0, nil +} +func (w EmptyResponseWriter) WriteHeader(statusCode int) { +} + +func pokeTest() *pb.Pokemons { + return &pb.Pokemons{ + Entries: []*pb.Pokemon{ + { + Name: "Charizard", + Weight: 124, + Moves: []*pb.Move{ + { + Name: "Squirt", + Url: "https://boemklets.co", + }, + }, + }, + }, + } +} + +func BenchmarkJson(b *testing.B) { + writer := EmptyResponseWriter{} + pokemon := pokeTest() + + for i := 0; i <= b.N; i++ { + Json(writer, pokemon) + } +} + +func BenchmarkProtoJson(b *testing.B) { + writer := EmptyResponseWriter{} + pokemon := pokeTest() + + for i := 0; i <= b.N; i++ { + ProtoJson(writer, pokemon) + } +}