go/json-iterator
1
0
Fork 0
mirror of https://github.com/json-iterator/go.git synced 2025-04-20 11:28:49 +02:00
Code Issues Releases Activity

(*Stream).WriteMore: remove implicit Flush

Browse Source 
I believe that WriteMore should not call Flush for these reasons:

1. This is surprising for users because of inconsistency. Why call Flush in
   WriteMore and not in WriteObjectEnd?

2. It is not necessary; callers are free to call Flush if their use case demands
   it.

3. It harms performance in the common case by flushing the buffer much more
   frequently than it needs to be flushed.

The stream benchmark shows a 7% benefit to removing the Flush call, and I
observed a similar speedup in my real-world use case.

    benchmark                                        old ns/op     new ns/op     delta
    Benchmark_encode_string_with_SetEscapeHTML-8     442           437           -1.13%
    Benchmark_jsoniter_large_file-8                  21222         21062         -0.75%
    Benchmark_json_large_file-8                      40187         40266         +0.20%
    Benchmark_stream_encode_big_object-8             8611          7956          -7.61%

    benchmark                                        old allocs     new allocs     delta
    Benchmark_encode_string_with_SetEscapeHTML-8     6              6              +0.00%
    Benchmark_jsoniter_large_file-8                  78             78             +0.00%
    Benchmark_json_large_file-8                      13             13             +0.00%
    Benchmark_stream_encode_big_object-8             0              0              +0.00%

    benchmark                                        old bytes     new bytes     delta
    Benchmark_encode_string_with_SetEscapeHTML-8     760           760           +0.00%
    Benchmark_jsoniter_large_file-8                  4920          4920          +0.00%
    Benchmark_json_large_file-8                      6640          6640          +0.00%
    Benchmark_stream_encode_big_object-8             0             0             +0.00%

Backwards compatibility - I believe there is little to no risk that this breaks
callers. WriteMore does not leave the JSON in a valid state, so it must be
followed by other Write* methods. To get the finished JSON out, the caller must
already be calling Flush.
This commit is contained in:
Rob Figueiredo 2020-01-17 22:14:50 -05:00
parent 91f4a6405d
commit 11a37a0774
2 changed files with 12 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
stream.go
View File

@ -177,7 +177,6 @@ func (stream *Stream) WriteEmptyObject() {
func (stream *Stream) WriteMore() { func (stream *Stream) WriteMore() {
stream.writeByte(',') stream.writeByte(',')
stream.writeIndention(0) stream.writeIndention(0)
stream.Flush()
} }
// WriteArrayStart write [ with possible indention // WriteArrayStart write [ with possible indention

13
stream_test.go
View File

@ -63,8 +63,19 @@ func (w *NopWriter) Write(p []byte) (n int, err error) {
} }
func Test_flush_buffer_should_stop_grow_buffer(t *testing.T) { func Test_flush_buffer_should_stop_grow_buffer(t *testing.T) {
// Stream an array of a zillion zeros.
writer := new(NopWriter) writer := new(NopWriter)
NewEncoder(writer).Encode(make([]int, 10000000)) stream := NewStream(ConfigDefault, writer, 512)
stream.WriteArrayStart()
for i := 0; i < 10000000; i++ {
stream.WriteInt(0)
stream.WriteMore()
stream.Flush()
}
stream.WriteInt(0)
stream.WriteArrayEnd()
// Confirm that the buffer didn't have to grow.
should := require.New(t) should := require.New(t)
// 512 is the internal buffer size set in NewEncoder // 512 is the internal buffer size set in NewEncoder