Using strings.Builder in golang v1.10 to reduce memory allocations

[jmoh932]

According issue #240 , there is no reason why we don't, so I've tried.

And we are facing the real problems, the json.Unmarshal has been in our service's hot path. The Iterator.ReadString and Iterator.readStringSlowPath has been called multiple times per second. Any reasonable patch which could reduce the memory allocation will be valuable.

I've tried to avoid to use any incompatible trick of Golang. So if there is any plan to support new features of golang v1.10, this PR should be helpful I think.

[codecov]

Codecov Report

Merging #367 into master will increase coverage by 0.31%.
The diff coverage is 89.83%.

@@            Coverage Diff             @@
##           master     #367      +/-   ##
==========================================
+ Coverage   81.46%   81.78%   +0.31%     
==========================================
  Files          41       41              
  Lines        5008     5034      +26     
==========================================
+ Hits         4080     4117      +37     
+ Misses        809      796      -13     
- Partials      119      121       +2

Impacted Files	Coverage Δ
config.go	89.58% <100%> (+0.22%)	⬆️
pool.go	100% <100%> (ø)	⬆️
iter.go	89.89% <100%> (+0.15%)	⬆️
iter_str.go	90.34% <87.23%> (-0.57%)	⬇️
stream_float.go	91.17% <0%> (-8.83%)	⬇️
reflect_struct_decoder.go	48.74% <0%> (+2.5%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1bc9828...6461c1c. Read the comment docs.

[jmoh932]

I've checked the issue of codeclimate, it's just facing a bug. The codeclimate has counted the nested function return statement. For the code readability, I think the code should be like this.

[taowen]

is there any benchmark to support this is actual a improvement? why ret = strf.NewString(iter.buf[iter.head:i]) is faster?

[jmoh932]

here is a benchmark in github.com/junchih/stringx package, https://github.com/junchih/stringx/blob/master/factory_test.go#L18 . you can go with go test -gcflags=-N -bench=. -benchmem ./... to get the result.

the reason why it is faster, you could figure out from the implementation of stringx and this function https://golang.org/src/strings/builder.go?s=1379:1412#L36 .

I'm not sure should I copy the code to json-iterator/go or just imported it. I created junchih/stringx separately cause our production might need it, and I might be able to do a PR for golang.

I'm working some benchmarks to this PR.

[taowen]

iter.buf might be reused, so we are decoding from a reader. Cast the buffer to string, will cause the string point to invalid region later. I used the impl in very early days, and fall back to current impl due to the bug introduced.

[jmoh932]

hi @taowen , sorry for so long delay. I just made the benchmark to compare before this PR and after. the logs has been a little longer, below.

there are two benchmarks cost a little logger time comparing to before. Benchmark_jsoniter_nested and Benchmark_jsoniter_skip. I did the pprof, the line where ReadString and Parse both stands made the different time costing. so I just add another benchmark Benchmark_jsoniter_string. now everything make sense, you could figure out by yourself from the code.

and at the latest two commits, I just move the string factory object to the Iterator struct to reduce the time costing.

at last, to answer your question above, I'm not sure I got your point, sorry. but those calling trace might answer your worry. here junchih/stringx/factory.go and then here strings/builder.go.

for any further questions, feel free to comment, I'd like to fix any.

json-iterator:master
$ go test -bench=. -benchmem ./...
...
goos: linux
goarch: amd64
pkg: github.com/json-iterator/go/benchmarks
Benchmark_encode_string_with_SetEscapeHTML       2000000               759 ns/op             760 B/op             5 allocs/op
Benchmark_jsoniter_large_file                     100000             12801 ns/op            4888 B/op            79 allocs/op
Benchmark_json_large_file                          50000             36893 ns/op            6912 B/op            18 allocs/op
...
pkg: github.com/json-iterator/go/misc_tests
Benchmark_jsoniter_array        10000000               202 ns/op               0 B/op          0 allocs/op
Benchmark_json_array             1000000              1656 ns/op             480 B/op         12 allocs/op
Benchmark_jsoniter_float        50000000                29.5 ns/op             0 B/op          0 allocs/op
Benchmark_json_float             3000000               518 ns/op             312 B/op          4 allocs/op
Benchmark_jsoniter_encode_int   100000000               19.1 ns/op             0 B/op          0 allocs/op
Benchmark_itoa                  30000000                61.8 ns/op            16 B/op          1 allocs/op
Benchmark_jsoniter_int          100000000               23.3 ns/op             0 B/op          0 allocs/op
Benchmark_json_int               3000000               507 ns/op             312 B/op          4 allocs/op
Benchmark_jsoniter_nested        3000000               527 ns/op             256 B/op          8 allocs/op
Benchmark_jsoniter_string        3000000               480 ns/op              96 B/op          2 allocs/op
Benchmark_json_nested             500000              2671 ns/op             552 B/op         10 allocs/op
...
pkg: github.com/json-iterator/go/skip_tests
Benchmark_jsoniter_skip          1000000              1662 ns/op             224 B/op         16 allocs/op
Benchmark_json_skip               200000              6948 ns/op             480 B/op         10 allocs/op

junchih:master
$ go test -bench=. -benchmem ./...
...
goos: linux
goarch: amd64
pkg: github.com/json-iterator/go/benchmarks
Benchmark_encode_string_with_SetEscapeHTML       2000000               764 ns/op             760 B/op          5 allocs/op
Benchmark_jsoniter_large_file                     100000             12973 ns/op            5432 B/op         14 allocs/op
Benchmark_json_large_file                          50000             34490 ns/op            6912 B/op         18 allocs/op
...
pkg: github.com/json-iterator/go/misc_tests
Benchmark_jsoniter_array        10000000               206 ns/op               0 B/op          0 allocs/op
Benchmark_json_array             1000000              1681 ns/op             480 B/op         12 allocs/op
Benchmark_jsoniter_float        50000000                28.4 ns/op             0 B/op          0 allocs/op
Benchmark_json_float             3000000               516 ns/op             312 B/op          4 allocs/op
Benchmark_jsoniter_encode_int   100000000               19.2 ns/op             0 B/op          0 allocs/op
Benchmark_itoa                  30000000                62.1 ns/op            16 B/op          1 allocs/op
Benchmark_jsoniter_int          100000000               23.1 ns/op             0 B/op          0 allocs/op
Benchmark_json_int               3000000               506 ns/op             312 B/op          4 allocs/op
Benchmark_jsoniter_nested        2000000               708 ns/op             344 B/op          7 allocs/op
Benchmark_jsoniter_string        3000000               454 ns/op              92 B/op          1 allocs/op
Benchmark_json_nested             500000              2643 ns/op             552 B/op         10 allocs/op
...
pkg: github.com/json-iterator/go/skip_tests
Benchmark_jsoniter_skip          1000000              1955 ns/op             440 B/op          7 allocs/op
Benchmark_json_skip               200000              6583 ns/op             480 B/op         10 allocs/op
...

[jmoh932]

hi @taowen , will this be merged or not?! it's been about a week, we really need this improvement.

if there is any blur, please comment it.

[taowen]

ret = iter.strf.NewString(iter.buf[iter.head:i]) the underlying iter.buf will change. it is not safe to use unsafe pointer to cast the byte array to string. It may work for some benchmark, it will cause problem when the input is a reader, and reading data in chunks.

[jmoh932]

ret = iter.strf.NewString(iter.buf[iter.head:i]) the underlying iter.buf will change. it is not safe to use unsafe pointer to cast the byte array to string. It may work for some benchmark, it will cause problem when the input is a reader, and reading data in chunks.

do you read the implementation of function iter.strf.NewString? you should not give two comments based on your same old illusion in this PR discussion, since I've already give you code line number like below

the reason why it is faster, you could figure out from the implementation of stringx and this function https://golang.org/src/strings/builder.go?s=1379:1412#L36 .

Very sorry, since so long there is no more progress about this discussion, I'll just close this PR for no more responsibilities from me. Happy day!