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

performance issue #256

Open
jiacai2050 opened this issue Oct 27, 2019 · 6 comments
Open

performance issue #256

jiacai2050 opened this issue Oct 27, 2019 · 6 comments

Comments

@jiacai2050
Copy link

type Demo struct {
	A int    `json:"a,omitempty"`
	B string `json:"b,omitempty"`

}

func BenchmarkJsoncost(b *testing.B) {
	d := Demo{
		A: 1,
		B: "abc",
	}

	b.ReportAllocs()
	for i := 0; i < b.N; i++ {
		_, err := json.Marshal(d)
		if err != nil {
			b.Errorf("marshal failed: %v", err)
		}
	}
}

go test -run ^NOTHING -bench Jsoncost$ -v

goos: darwin
goarch: amd64
pkg: go-app/basic
BenchmarkJsoncost-8      5000000               318 ns/op              64 B/op          2 allocs/op
PASS

Then, I use ffjson to generate custom MarshalJSON for Demo struct, update json.Marshal(d) to ffjson.Marshal(d), run benchmark again

goos: darwin
goarch: amd64
pkg: go-app/basic
BenchmarkJsoncost-8      5000000               355 ns/op              64 B/op          2 allocs/op
PASS

It seems ffjson is slower than encoding/json, is there anything I'm missing ?

@erikdubbelboer
Copy link
Contributor

I haven't used ffjson in a long time so I might be wrong, but this is the only way I could get your code fast:

func BenchmarkFFJson(b *testing.B) {
  d := Demo{
    A: 1,
    B: "asd",
  }

  buf := &fflib.Buffer{}
  for i := 0; i < b.N; i++ {
    if err := d.MarshalJSONBuf(buf); err != nil {
      panic(err)
    }
    _ = buf.Bytes()

    buf.Reset()
  }
}

I also had to apply this patch to get ffjson down to 0 allocations: https://gist.github.com/erikdubbelboer/898d219bbc997ab1e9aeee2d64aafa04
It seems like ffjson isn't doing it's best to get hot path allocations down to 0. @pquerna let me know if you want me to make a pull request for this?

The result:

BenchmarkEncodingJson-16    	 4652290       249 ns/op	64 B/op	 2 allocs/op
BenchmarkFFJson-16          	13343437      90.7 ns/op	 0 B/op	 0 allocs/op

@jiacai2050
Copy link
Author

Hi @erikdubbelboer,
convert []byte to string no longer require SliceHeader, just *(*string)(unsafe.Pointer(&bs)) is fine.

After dig into this more, I compare ffjson/easyjson/jsoniter, it's surprising ffjson is the slowest, with highest allocs.

BenchmarkJsoncost/std-8                  1000000              1052 ns/op             240 B/op          4 allocs/op
BenchmarkJsoncost/jsoniter-8             1000000              1268 ns/op             272 B/op          5 allocs/op
BenchmarkJsoncost/easyjson-8             2000000               629 ns/op             336 B/op          4 allocs/op
BenchmarkJsoncost/ffjson-8               1000000              1044 ns/op             480 B/op          7 allocs/op

repo for this benchmark:

@erikdubbelboer
Copy link
Contributor

Correct but this is to convert a string to a []byte which does require SliceHeader as you need to set the capacity which a string doesn't have in it's header.

I always use easyjson, that's why I said I haven't used ffjson in a long time.

@jiacai2050
Copy link
Author

jiacai2050 commented Oct 27, 2019

if convert []byte to string, *(*[]byte)(unsafe.Pointer(&s)) would be fine IMO.

@erikdubbelboer
Copy link
Contributor

That will only work in cases where you never look at the capacity of the returned slice. If you do something that looks at the capacity it will fail horribly as it will use the memory after the string header as capacity and you have on idea what is stored there.

@jiacai2050
Copy link
Author

I got your point, thanks for your explanation.

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

No branches or pull requests

2 participants