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

chore: use json.Marshal for the message context #1975

Merged
merged 1 commit into from Nov 22, 2021

Conversation

zepatrik
Copy link
Member

Related issue(s)

The docs is auto-generating with useless changes, see e.g. 64bf08d because we used sjson which does not order keys.
For less CI spam I am proposing to instead use json.Marshal. A quick benchmark shows that in this specific case sjson is only insignificantly faster:

Go benchmark
package sjsonbench_test

import (
	"encoding/json"
	"github.com/tidwall/sjson"
	"testing"
	"time"
)

var now = time.Now()

func BenchmarkSJSON(b *testing.B) {
	for i := 0; i < b.N; i++ {
		j := "{}"
		j, err := sjson.Set(j, "foobar", "some value")
		if err != nil {
			b.Errorf("error: %s", err)
		}
		j, err = sjson.Set(j, "asdf", 13241345)
		if err != nil {
			b.Errorf("error: %s", err)
		}
		j, err = sjson.Set(j, "jkhad", map[string]interface{}{"expired_at": now})
		if err != nil {
			b.Errorf("error: %s", err)
		}
	}
}

func BenchmarkGoJSON(b *testing.B) {
	for i := 0; i < b.N; i++ {
		_, err := json.Marshal(map[string]interface{}{
			"foobar": "some value",
			"asdf":   13241345,
			"jkhad":  map[string]interface{}{"expired_at": now},
		})
		if err != nil {
			b.Errorf("error: %s", err)
		}
	}
}
$ go test -bench .
goos: linux
goarch: amd64
pkg: sjsonbench
cpu: Intel(R) Core(TM) i7-7500U CPU @ 2.70GHz
BenchmarkSJSON-4          295066              4535 ns/op
BenchmarkGoJSON-4         234032              5213 ns/op
PASS
ok      sjsonbench      3.522s
$ # Without the third key
$ go test -bench .
goos: linux
goarch: amd64
pkg: sjsonbench
cpu: Intel(R) Core(TM) i7-7500U CPU @ 2.70GHz
BenchmarkSJSON-4          895790              1165 ns/op
BenchmarkGoJSON-4         675788              1684 ns/op
PASS
ok      sjsonbench      2.219s

Checklist

  • I have read the contributing guidelines.
  • I have referenced an issue containing the design document if my change
    introduces a new feature.
  • I am following the
    contributing code guidelines.
  • I have read the security policy.
  • I confirm that this pull request does not address a security
    vulnerability. If this pull request addresses a security. vulnerability, I
    confirm that I got green light (please contact
    security@ory.sh) from the maintainers to push
    the changes.
  • I have added tests that prove my fix is effective or that my feature
    works.
  • I have added or changed the documentation.

Further Comments

Alternatively, we could also explicitly sort the keys before rendering the docs pages.

@aeneasr aeneasr merged commit adc748e into master Nov 22, 2021
@aeneasr aeneasr deleted the fix/sorted-message-context-keys branch November 22, 2021 20:28
peturgeorgievv pushed a commit to senteca/kratos-fork that referenced this pull request Jun 30, 2023
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.

None yet

2 participants