| Age | Commit message (Collapse) | Author |
|---|
|
Adds a sample Fuzz test function to package encoding/json following the
guidelines defined in #31309, based on
https://github.com/dvyukov/go-fuzz-corpus/blob/master/json/json.go
Fixes #31309
Updates #19109
Change-Id: I5fe04d9a5f41c0de339f8518dae30896ec14e356
Reviewed-on: https://go-review.googlesource.com/c/go/+/174058
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Dmitry Vyukov <dvyukov@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
|
|
Make explicit that Compact does HTML escaping.
Fixes #30357.
Change-Id: I4648f8f3e907d659db977d07253f716df6e07d7b
Reviewed-on: https://go-review.googlesource.com/c/go/+/173417
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
|
|
In the common case, structs have a handful of fields and most inputs
match struct field names exactly.
The previous code would do a linear search over the fields, stopping at
the first exact match, and otherwise using the first case insensitive
match.
This is unfortunate, because it means that for the common case, we'd do
a linear search with bytes.Equal. Even for structs with only two or
three fields, that is pretty wasteful.
Worse even, up until the exact match was found via the linear search,
all previous fields would run their equalFold functions, which aren't
cheap even in the simple case.
Instead, cache a map along with the field list that indexes the fields
by their name. This way, a case sensitive field search doesn't involve a
linear search, nor does it involve any equalFold func calls.
This patch should also slightly speed up cases where there's a case
insensitive match but not a case sensitive one, as then we'd avoid
calling bytes.Equal on all the fields. Though that's not a common case,
and there are no benchmarks for it.
name old time/op new time/op delta
CodeDecoder-8 11.0ms ± 0% 10.6ms ± 1% -4.42% (p=0.000 n=9+10)
name old speed new speed delta
CodeDecoder-8 176MB/s ± 0% 184MB/s ± 1% +4.62% (p=0.000 n=9+10)
name old alloc/op new alloc/op delta
CodeDecoder-8 2.28MB ± 0% 2.28MB ± 0% ~ (p=0.725 n=10+10)
name old allocs/op new allocs/op delta
CodeDecoder-8 76.9k ± 0% 76.9k ± 0% ~ (all equal)
Updates #28923.
Change-Id: I9929c1f06c76505e5b96914199315dbdaae5dc76
Reviewed-on: https://go-review.googlesource.com/c/go/+/172918
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
|
|
We can work out how many bytes can be unquoted trivially in
rescanLiteral, which already iterates over a string's bytes.
Removing the extra loop in unquoteBytes simplifies the function and
speeds it up, especially when decoding simple strings, which are common.
While at it, we can remove unnecessary checks like len(s)<2 and
s[0]=='"'. Add a comment explaining why.
name old time/op new time/op delta
CodeDecoder-8 11.2ms ± 0% 11.1ms ± 1% -1.63% (p=0.000 n=9+10)
name old speed new speed delta
CodeDecoder-8 173MB/s ± 0% 175MB/s ± 1% +1.66% (p=0.000 n=9+10)
Updates #28923.
Change-Id: I2436a3a7f8148a2f7a6a4cdbd7dec6b32ef5e20c
Reviewed-on: https://go-review.googlesource.com/c/go/+/151157
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
|
|
readValue is a hot function, clocking in at ~13% flat CPU use in
CodeDecoder. In particular, looping over the bytes is slow. That's
partially because the code contains a bounds check at the start of the
loop.
The source of the problem is that scanp is a signed integer, and comes
from a field, so the compiler doesn't know that it's non-negative. Help
it with a simple and comparatively cheap hint.
While at it, use scanp as the index variable directly, removing the need
for a duplicate index variable which is later added back into scanp.
name old time/op new time/op delta
CodeDecoder-8 11.3ms ± 1% 11.2ms ± 1% -0.98% (p=0.000 n=9+9)
name old speed new speed delta
CodeDecoder-8 172MB/s ± 1% 174MB/s ± 1% +0.99% (p=0.000 n=9+9)
Updates #28923.
Change-Id: I138f83babdf316fc97697cc18f595c3403c1ddb7
Reviewed-on: https://go-review.googlesource.com/c/go/+/170939
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
|
|
This was the only benchmark missing the SetBytes call, as spotted
earlier by Bryan.
It's not required to make the benchmark useful, but it can still be a
good way to see how its speed is affected by the reduced allocations:
name time/op
CodeUnmarshal-8 12.1ms ± 1%
CodeUnmarshalReuse-8 11.4ms ± 1%
name speed
CodeUnmarshal-8 161MB/s ± 1%
CodeUnmarshalReuse-8 171MB/s ± 1%
name alloc/op
CodeUnmarshal-8 3.28MB ± 0%
CodeUnmarshalReuse-8 1.94MB ± 0%
name allocs/op
CodeUnmarshal-8 92.7k ± 0%
CodeUnmarshalReuse-8 77.6k ± 0%
While at it, remove some unnecessary empty lines.
Change-Id: Ib2bd92d5b3237b8f3092e8c6f863dab548fee2f5
Reviewed-on: https://go-review.googlesource.com/c/go/+/170938
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
|
|
Decoder.Decode and Unmarshal actually scan the input bytes twice - the
first time to check for syntax errors and the length of the value, and
the second to perform the decoding.
It's in the second scan that we actually tokenize the bytes. Since
syntax errors aren't a possibility, we can take shortcuts.
In particular, literals such as quoted strings are very common in JSON,
so we can avoid a lot of work by special casing them.
name old time/op new time/op delta
CodeDecoder-8 10.3ms ± 1% 9.1ms ± 0% -11.89% (p=0.002 n=6+6)
UnicodeDecoder-8 342ns ± 0% 283ns ± 0% -17.25% (p=0.000 n=6+5)
DecoderStream-8 239ns ± 0% 230ns ± 0% -3.90% (p=0.000 n=6+5)
CodeUnmarshal-8 11.0ms ± 0% 9.8ms ± 0% -11.45% (p=0.002 n=6+6)
CodeUnmarshalReuse-8 10.3ms ± 0% 9.0ms ± 0% -12.72% (p=0.004 n=5+6)
UnmarshalString-8 104ns ± 0% 92ns ± 0% -11.35% (p=0.002 n=6+6)
UnmarshalFloat64-8 93.2ns ± 0% 87.6ns ± 0% -6.01% (p=0.010 n=6+4)
UnmarshalInt64-8 74.5ns ± 0% 71.5ns ± 0% -3.91% (p=0.000 n=5+6)
name old speed new speed delta
CodeDecoder-8 189MB/s ± 1% 214MB/s ± 0% +13.50% (p=0.002 n=6+6)
UnicodeDecoder-8 40.9MB/s ± 0% 49.5MB/s ± 0% +20.96% (p=0.002 n=6+6)
CodeUnmarshal-8 176MB/s ± 0% 199MB/s ± 0% +12.93% (p=0.002 n=6+6)
Updates #28923.
Change-Id: I7a5e2aef51bd4ddf2004aad24210f6f50e01eaeb
Reviewed-on: https://go-review.googlesource.com/c/go/+/151042
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
|
|
In golang.org/cl/145218, a feature was added where the JSON decoder
would keep track of the entire path to a field when reporting an
UnmarshalTypeError.
However, we all failed to check if this affected the benchmarks - myself
included, as a reviewer. Below are the numbers comparing the CL's parent
with itself, once it was merged:
name old time/op new time/op delta
CodeDecoder-8 12.9ms ± 1% 28.2ms ± 2% +119.33% (p=0.002 n=6+6)
name old speed new speed delta
CodeDecoder-8 151MB/s ± 1% 69MB/s ± 3% -54.40% (p=0.002 n=6+6)
name old alloc/op new alloc/op delta
CodeDecoder-8 2.74MB ± 0% 109.39MB ± 0% +3891.83% (p=0.002 n=6+6)
name old allocs/op new allocs/op delta
CodeDecoder-8 77.5k ± 0% 168.5k ± 0% +117.30% (p=0.004 n=6+5)
The reason why the decoder got twice as slow is because it now allocated
~40x as many objects, which puts a lot of pressure on the garbage
collector.
The reason is that the CL concatenated strings every time a nested field
was decoded. In other words, practically every field generated garbage
when decoded. This is hugely wasteful, especially considering that the
vast majority of JSON decoding inputs won't return UnmarshalTypeError.
Instead, use a stack of fields, and make sure to always use the same
backing array, to ensure we only need to grow the slice to the maximum
depth once.
The original CL also introduced a bug. The field stack string wasn't
reset to its original state when reaching "d.opcode == scanEndObject",
so the last field in a decoded struct could leak. For example, an added
test decodes a list of structs, and encoding/json before this CL would
fail:
got: cannot unmarshal string into Go struct field T.Ts.Y.Y.Y of type int
want: cannot unmarshal string into Go struct field T.Ts.Y of type int
To fix that, simply reset the stack after decoding every field, even if
it's the last.
Below is the original performance versus this CL. There's a tiny
performance hit, probably due to the append for every decoded field, but
at least we're back to the usual ~150MB/s.
name old time/op new time/op delta
CodeDecoder-8 12.9ms ± 1% 13.0ms ± 1% +1.25% (p=0.009 n=6+6)
name old speed new speed delta
CodeDecoder-8 151MB/s ± 1% 149MB/s ± 1% -1.24% (p=0.009 n=6+6)
name old alloc/op new alloc/op delta
CodeDecoder-8 2.74MB ± 0% 2.74MB ± 0% +0.00% (p=0.002 n=6+6)
name old allocs/op new allocs/op delta
CodeDecoder-8 77.5k ± 0% 77.5k ± 0% +0.00% (p=0.002 n=6+6)
Finally, make all of these benchmarks report allocs by default. The
decoder ones are pretty sensitive to generated garbage, so ReportAllocs
would have made the performance regression more obvious.
Change-Id: I67b50f86b2e72f55539429450c67bfb1a9464b67
Reviewed-on: https://go-review.googlesource.com/c/go/+/167978
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
|
|
Most of the decoding time is spent in the first Decode loop, since the
rest of the function only deals with the few remaining bytes. Any
unnecessary work done in that loop body matters tremendously.
One such unnecessary bottleneck was the use of the enc.decodeMap table.
Since enc is a pointer receiver, and the field is used within the
non-inlineable function decode64, the decoder must perform a nil check
at every iteration.
To fix that, move the enc.decodeMap uses to the parent function, where
we can lift the nil check outside the loop. That gives roughly a 15%
speed-up. The function no longer performs decoding per se, so rename it.
While at it, remove the now unnecessary receivers.
An unfortunate side effect of this change is that the loop now contains
eight bounds checks on src instead of just one. However, not having to
slice src plus the nil check removal well outweigh the added cost.
The other piece that made decode64 slow was that it wasn't inlined, and
had multiple branches. Use a simple bitwise-or trick suggested by Roger
Peppe, and collapse the rest of the bitwise logic into a single
expression. Inlinability and the reduced branching give a further 10%
speed-up.
Finally, add these two functions to TestIntendedInlining, since we want
them to stay inlinable.
Apply the same refactor to decode32 for consistency, and to let 32-bit
architectures see a similar performance gain for large inputs.
name old time/op new time/op delta
DecodeString/2-8 47.3ns ± 1% 45.8ns ± 0% -3.28% (p=0.002 n=6+6)
DecodeString/4-8 55.8ns ± 2% 51.5ns ± 0% -7.71% (p=0.004 n=5+6)
DecodeString/8-8 64.9ns ± 0% 61.7ns ± 0% -4.99% (p=0.004 n=5+6)
DecodeString/64-8 238ns ± 0% 198ns ± 0% -16.54% (p=0.002 n=6+6)
DecodeString/8192-8 19.5µs ± 0% 14.6µs ± 0% -24.96% (p=0.004 n=6+5)
name old speed new speed delta
DecodeString/2-8 84.6MB/s ± 1% 87.4MB/s ± 0% +3.38% (p=0.002 n=6+6)
DecodeString/4-8 143MB/s ± 2% 155MB/s ± 0% +8.41% (p=0.004 n=5+6)
DecodeString/8-8 185MB/s ± 0% 195MB/s ± 0% +5.29% (p=0.004 n=5+6)
DecodeString/64-8 369MB/s ± 0% 442MB/s ± 0% +19.78% (p=0.002 n=6+6)
DecodeString/8192-8 560MB/s ± 0% 746MB/s ± 0% +33.27% (p=0.004 n=6+5)
Updates #19636.
Change-Id: Ib839577b0e3f5a2bb201f5cae580c61365d92894
Reviewed-on: https://go-review.googlesource.com/c/go/+/151177
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: roger peppe <rogpeppe@gmail.com>
|
|
If a for loop has a simple condition and begins with a simple
"if x { break; }"; we can simply add "!x" to the loop's condition.
While at it, simplify a few assignments to use the common pattern
"x := staticDefault; if cond { x = otherValue(); }".
Finally, simplify a couple of var declarations.
Change-Id: I413982c6abd32905adc85a9a666cb3819139c19f
Reviewed-on: https://go-review.googlesource.com/c/go/+/165342
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
|
It was just out of date.
Fixes #30656
Change-Id: I1fab7dd93091865a8240769eca5dd19cdbc78b81
Reviewed-on: https://go-review.googlesource.com/c/go/+/166177
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
|
|
I had been finding these over a year or so, but none were big enough
changes to warrant CLs. They're a handful now, so clean them all up in a
single commit.
The smaller bodies get a bit simpler, but most importantly, the larger
bodies get unindented.
Change-Id: I5707a6fee27d4c9ff9efd3d363af575d7a4bf2aa
Reviewed-on: https://go-review.googlesource.com/c/go/+/165340
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
|
|
Base64-encoding 32 bytes results in a 44-byte string.
While in general a 44-byte string might decode to 33 bytes,
if you take a 44-byte string that actually only encodes 32 bytes,
and you try to decode it into 32 bytes, that should succeed.
Instead it fails trying to do a useless dst[33:] slice operation.
Delete that slice operation.
Noticed while preparing CL 156322.
Change-Id: I8024bf28a65e2638675b980732b2ff91c66c62cf
Reviewed-on: https://go-review.googlesource.com/c/go/+/164628
Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
|
Two additions are faster than two multiplications and one addition. The
code seems simpler to me too, as it's more obvious that we advance two
destination bytes for each source byte.
name old time/op new time/op delta
Encode/256-8 374ns ± 0% 331ns ± 0% -11.44% (p=0.008 n=5+5)
Encode/1024-8 1.47µs ± 0% 1.29µs ± 0% -11.89% (p=0.004 n=6+5)
Encode/4096-8 5.85µs ± 1% 5.15µs ± 0% -11.89% (p=0.004 n=6+5)
Encode/16384-8 23.3µs ± 0% 20.6µs ± 0% -11.68% (p=0.004 n=6+5)
Change-Id: Iabc63616c1d9fded55fa668ff41dd49efeaa2ea4
Reviewed-on: https://go-review.googlesource.com/c/go/+/151198
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: roger peppe <rogpeppe@gmail.com>
|
|
When parsing nested object, UnmarshalTypeError does not contain actual
path to nested field in original JSON.
This commit change Field to contain the full path to that field. One
can get the Field name by stripping all the leading path elements.
Fixes #22369
Change-Id: I6969cc08abe8387a351e3fb2944adfaa0dccad2a
Reviewed-on: https://go-review.googlesource.com/c/go/+/145218
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
|
|
Add documentation that individual Write calls are buffered and
copy documentation from bufio.Writer notifying the user to call
Flush and Error when all writes are complete. Remove reference
to "file" since the implementation is general and allows any
io.Writer.
Fixes #30045
Change-Id: I50165470e548f296494e764707fbabe36c665015
Reviewed-on: https://go-review.googlesource.com/c/160680
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
|
|
Most of the encoding time is spent in the first Encode loop, since the
rest of the function only deals with the few remaining bytes. Any
unnecessary work done in that loop body matters tremendously.
One such unnecessary bottleneck was the use of the enc.encode table.
Since enc is a pointer receiver, and the field is first used within the
loop, the encoder must perform a nil check at every iteration.
Add a dummy use of the field before the start of the loop, to move the
nil check there. After that line, the compiler now knows that enc can't
be nil, and thus the hot loop is free of nil checks.
name old time/op new time/op delta
EncodeToString-4 14.7µs ± 0% 13.7µs ± 1% -6.53% (p=0.000 n=10+10)
name old speed new speed delta
EncodeToString-4 559MB/s ± 0% 598MB/s ± 1% +6.99% (p=0.000 n=10+10)
Updates #20206.
Change-Id: Icbb523a7bd9e470a8be0a448d1d78ade97ed4ff6
Reviewed-on: https://go-review.googlesource.com/c/151158
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
|
|
encoding/base64 already skips \r and \n when decoding, so this package
must only deal with spaces and tabs. Those aren't nearly as common, so
we can add a fast path with bytes.ContainsAny to skip the costly alloc
and filtering code.
name old time/op new time/op delta
Decode-8 279µs ± 0% 259µs ± 1% -7.07% (p=0.002 n=6+6)
name old speed new speed delta
Decode-8 319MB/s ± 0% 343MB/s ± 1% +7.61% (p=0.002 n=6+6)
name old alloc/op new alloc/op delta
Decode-8 164kB ± 0% 74kB ± 0% -54.90% (p=0.002 n=6+6)
name old allocs/op new allocs/op delta
Decode-8 12.0 ± 0% 11.0 ± 0% -8.33% (p=0.002 n=6+6)
Change-Id: Idfca8700c52f46eb70a4a7e0d2db3bf0124e4699
Reviewed-on: https://go-review.googlesource.com/c/155964
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
|
|
Remove all multiplications and divisions from the main decoding loop.
name old time/op new time/op delta
Decode/256-8 323ns ± 0% 293ns ± 0% -9.29% (p=0.000 n=5+4)
Decode/1024-8 1.26µs ± 0% 1.14µs ± 0% -9.48% (p=0.000 n=6+5)
Decode/4096-8 4.99µs ± 0% 4.51µs ± 0% -9.55% (p=0.002 n=6+6)
Decode/16384-8 20.0µs ± 0% 18.1µs ± 0% -9.54% (p=0.002 n=6+6)
name old speed new speed delta
Decode/256-8 791MB/s ± 0% 872MB/s ± 0% +10.34% (p=0.002 n=6+6)
Decode/1024-8 814MB/s ± 0% 899MB/s ± 0% +10.48% (p=0.004 n=6+5)
Decode/4096-8 821MB/s ± 0% 908MB/s ± 0% +10.55% (p=0.002 n=6+6)
Decode/16384-8 821MB/s ± 0% 908MB/s ± 0% +10.54% (p=0.002 n=6+6)
Change-Id: Ie9f91242ce04c130a77c1184379e3b9de38fe713
Reviewed-on: https://go-review.googlesource.com/c/151199
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
|
|
Change-Id: I4a0d5b2f76138895567939920fa5d83cbdec17d2
GitHub-Last-Rev: 061d9d1d5655a6a9d8371f08d2f77a0ed7a495cc
GitHub-Pull-Request: golang/go#30008
Reviewed-on: https://go-review.googlesource.com/c/160432
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
|
|
Change-Id: I8aaa3d1d2797f3ace34bc09f5123538f6a77efce
GitHub-Last-Rev: 2758c462041ff5e444651b7927d53e809d2efe4d
GitHub-Pull-Request: golang/go#30009
Reviewed-on: https://go-review.googlesource.com/c/160433
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
|
|
Comparing errors using DeepEqual breaks if frame information
is added as proposed in Issue #29934.
Updates #29934.
Change-Id: Ib430c9ddbe588dd1dd51314c408c74c07285e1ff
Reviewed-on: https://go-review.googlesource.com/c/162179
Run-TryBot: Marcel van Lohuizen <mpvl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
|
|
First, we can lift the enc.decodeMap nil check out of the loop.
Second, we can make it clear to the compiler that 'in := src[0]' doesn't
need a bounds check, by making len(src)==0 a single if check that always
stops the loop. This is by far the largest speed-up.
Third, we can use a dst slice index instead of reslicing dst, which
removes work from the loop body.
While at it, we can merge the two 'switch dlen' pieces of code, which
simplifies the code and doesn't affect performance.
name old time/op new time/op delta
DecodeString-8 80.2µs ± 0% 67.5µs ± 0% -15.81% (p=0.002 n=6+6)
name old speed new speed delta
DecodeString-8 163MB/s ± 0% 194MB/s ± 0% +18.78% (p=0.002 n=6+6)
Change-Id: Iefeaae94c03453f8760452b1da706a77b3522718
Reviewed-on: https://go-review.googlesource.com/c/154422
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
|
|
Change-Id: Ib00fcfd46eae27eea0a3d4cab4406f4c461fb57b
Reviewed-on: https://go-review.googlesource.com/c/160517
Reviewed-by: Andrew Bonventre <andybons@golang.org>
Run-TryBot: Andrew Bonventre <andybons@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
|
|
Change-Id: Iaabbfe5a4c1bbedd19d4087f1b79e5a38bdd3878
GitHub-Last-Rev: 55c91fc19074dacc66623aa7ff2286b11ccd5340
GitHub-Pull-Request: golang/go#29752
Reviewed-on: https://go-review.googlesource.com/c/157958
Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
|
Fixes #29416
Change-Id: I24364bfee77aceace53f85f1046ef4d73f8feebb
Change-Id: I24364bfee77aceace53f85f1046ef4d73f8feebb
GitHub-Last-Rev: ad9f31145763dc16f53dd9f3154667b162759f69
GitHub-Pull-Request: golang/go#29417
Reviewed-on: https://go-review.googlesource.com/c/155742
Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
|
Both the encoding/xml and encoding/json packages support custom
marshalers for JSON and XML, as well as the basic encoding.TextMarshaler
and encoding.TextUnmarshaler interfaces, but the docs and examples for
these are missing.
There are docs for how to use encoding.TextMarshaler and
encoding.TextUnmarshaler in encoding/json, but not encoding/xml. There
are no examples for how to use them with either json or xml. This commit
includes docs for encoding/xml and examples for both encoding/json and
encoding/xml.
There is an example using custom marshalers MarshalJSON and
UnmarshalJSON in encoding/json, but not MarshalXML and UnmarshalXML in
encoding/json. These docs are more so necessary for encoding/xml because
the complexities of XML documents is significantly greater than JSON
documents which more often leads to the need for custom marshaling. The
encoding/json package includes an example of how to write a custom
marshaler, and this commit includes the same example for the xml
package.
All examples are mirrored off the existing custom marshaler example in
encoding/json.
Fixes #6859
Change-Id: Ic93abc27c0b4d5e48dea6ede4e20b1bedca4ab39
Reviewed-on: https://go-review.googlesource.com/c/76350
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
|
Change-Id: I7a1046f5e0aedbbdd1106a616de410fe4e0cb7d8
Reviewed-on: https://go-review.googlesource.com/c/92295
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
|
Go documentation style for boolean funcs is to say:
// Foo reports whether ...
func Foo() bool
(rather than "returns true if")
This CL also replaces 4 uses of "iff" with the same "reports whether"
wording, which doesn't lose any meaning, and will prevent people from
sending typo fixes when they don't realize it's "if and only if". In
the past I think we've had the typo CLs updated to just say "reports
whether". So do them all at once.
(Inspired by the addition of another "returns true if" in CL 146938
in fd_plan9.go)
Created with:
$ perl -i -npe 's/returns true if/reports whether/' $(git grep -l "returns true iff" | grep -v vendor)
$ perl -i -npe 's/returns true if/reports whether/' $(git grep -l "returns true if" | grep -v vendor)
Change-Id: Ided502237f5ab0d25cb625dbab12529c361a8b9f
Reviewed-on: https://go-review.googlesource.com/c/147037
Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
|
Change-Id: I411483d76a2ca91cd15ff42ae1adb9134486d183
Reviewed-on: https://go-review.googlesource.com/c/145278
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
|
|
Now that the library allows much larger data, it can kill
machines with less memory.
Fixes #28321
Change-Id: I98e1a5fdf812fd75adfb22bf01542423de405fe2
Reviewed-on: https://go-review.googlesource.com/c/143817
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
|
|
A little shift magic makes it easy to adjust the maximum buffer
size on machines with larger integers.
Fixes #27635
Change-Id: I1f26b07a363fbb9730df2377052475fa88bbb781
Reviewed-on: https://go-review.googlesource.com/c/143678
Run-TryBot: Rob Pike <r@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
|
Calling .Interface on a struct field's reflect.Value isn't always safe.
For example, if that field is an unexported anonymous struct.
We only descended into this branch if the struct type had any methods,
so this bug had gone unnoticed for a few release cycles.
Add the check, and add a simple test case.
Fixes #28145.
Change-Id: I02f7e0ab9a4a0c18a5e2164211922fe9c3d30f64
Reviewed-on: https://go-review.googlesource.com/c/141537
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
|
Given a program as follows:
data := []byte(`{"F": {
"a": 2,
"3": 4
}}`)
json.Unmarshal(data, &map[string]map[int]int{})
The JSON package should error, as "a" is not a valid integer. However,
we'd encounter a panic:
panic: JSON decoder out of sync - data changing underfoot?
The reason was that decodeState.object would return a nil error on
encountering the invalid map key string, while saving the key type error
for later. This broke if we were inside another object, as we would
abruptly end parsing the nested object, leaving the decoder in an
unexpected state.
To fix this, simply avoid storing the map element and continue decoding
the object, to leave the decoder state exactly as if we hadn't seen an
invalid key type.
This affected both signed and unsigned integer keys, so fix both and add
two test cases.
Updates #28189.
Change-Id: I8a6204cc3ff9fb04ed769df7a20a824c8b94faff
Reviewed-on: https://go-review.googlesource.com/c/142518
Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
|
Change-Id: I643540bcea574d8a70b79237d97097dcc7368766
GitHub-Last-Rev: e2be58d1ab84f91dfbba1067aae7145f24fd650d
GitHub-Pull-Request: golang/go#28125
Reviewed-on: https://go-review.googlesource.com/c/141119
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
|
|
This change makes stateEndTop use isSpace instead of specifically
recreating the same functionality.
Change-Id: I81f8f51682e46e7f8e2b9fed423a968457200625
Reviewed-on: https://go-review.googlesource.com/c/121797
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
|
|
I omitted vendor directories and anything necessary for bootstrapping.
(Tested by bootstrapping with Go 1.4)
Updates #27864
Change-Id: I7d9b68d0372d3a34dee22966cca323513ece7e8a
Reviewed-on: https://go-review.googlesource.com/137856
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
|
Having these panic-like errors used to be ok, since they were used in
the internal decoder state instead of passed around via return
parameters.
Recently, the decoder was rewritten to use explicit error returns
instead. This error is a terrible fit for error returns; a handful of
functions must return an error because of it, and their callers must
check for an error that should never happen.
This is precisely what panics are for, so use them. The test coverage of
the package goes up from 91.3% to 91.6%, and performance is unaffected.
We can also get rid of some unnecessary verbosity in the code.
name old time/op new time/op delta
CodeDecoder-4 27.5ms ± 1% 27.5ms ± 1% ~ (p=0.937 n=6+6)
Change-Id: I01033b3f5b7c0cf0985082fa272754f96bf6353c
Reviewed-on: https://go-review.googlesource.com/134835
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
|
|
The overall coverage of the json package goes up from 90.8% to 91.3%.
While at it, apply two minor code simplifications found while inspecting
the HTML coverage report.
Change-Id: I0fba968afeedc813b1385e4bde72d93b878854d7
Reviewed-on: https://go-review.googlesource.com/134735
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
|
Fixes: #27464
Change-Id: I270c56fd0d5ae8787a1293029aff3072f4f52f33
Reviewed-on: https://go-review.googlesource.com/132955
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
|
|
There's no need to manually manage the backing slice for bs.
Removing it simplifies the code, removes some allocations,
and speeds it up slightly.
Fixes #27403
name old time/op new time/op delta
ReadSlice1000Int32s-8 6.39µs ± 1% 6.31µs ± 1% -1.37% (p=0.000 n=27+27)
ReadStruct-8 1.25µs ± 2% 1.23µs ± 2% -1.06% (p=0.003 n=30+29)
ReadInts-8 301ns ± 0% 297ns ± 1% -1.21% (p=0.000 n=27+30)
WriteInts-8 325ns ± 1% 320ns ± 1% -1.59% (p=0.000 n=26+29)
WriteSlice1000Int32s-8 6.60µs ± 0% 6.52µs ± 0% -1.23% (p=0.000 n=28+27)
PutUint16-8 0.72ns ± 2% 0.71ns ± 2% ~ (p=0.286 n=30+30)
PutUint32-8 0.71ns ± 1% 0.71ns ± 0% -0.42% (p=0.003 n=30+25)
PutUint64-8 0.78ns ± 2% 0.78ns ± 0% -0.55% (p=0.001 n=30+27)
LittleEndianPutUint16-8 0.57ns ± 0% 0.57ns ± 0% ~ (all equal)
LittleEndianPutUint32-8 0.57ns ± 0% 0.57ns ± 0% ~ (all equal)
LittleEndianPutUint64-8 0.57ns ± 0% 0.57ns ± 0% ~ (all equal)
PutUvarint32-8 23.1ns ± 1% 23.1ns ± 1% ~ (p=0.925 n=26+29)
PutUvarint64-8 57.5ns ± 2% 57.3ns ± 1% ~ (p=0.338 n=30+26)
[Geo mean] 23.0ns 22.9ns -0.61%
name old speed new speed delta
ReadSlice1000Int32s-8 626MB/s ± 1% 634MB/s ± 1% +1.38% (p=0.000 n=27+27)
ReadStruct-8 60.2MB/s ± 2% 60.8MB/s ± 2% +1.08% (p=0.002 n=30+29)
ReadInts-8 100MB/s ± 1% 101MB/s ± 1% +1.24% (p=0.000 n=27+30)
WriteInts-8 92.2MB/s ± 1% 93.6MB/s ± 1% +1.56% (p=0.000 n=26+29)
WriteSlice1000Int32s-8 606MB/s ± 0% 614MB/s ± 0% +1.24% (p=0.000 n=28+27)
PutUint16-8 2.80GB/s ± 1% 2.80GB/s ± 1% ~ (p=0.095 n=28+29)
PutUint32-8 5.61GB/s ± 1% 5.62GB/s ± 1% ~ (p=0.069 n=27+28)
PutUint64-8 10.2GB/s ± 1% 10.2GB/s ± 0% +0.15% (p=0.039 n=27+27)
LittleEndianPutUint16-8 3.50GB/s ± 1% 3.50GB/s ± 1% ~ (p=0.552 n=30+29)
LittleEndianPutUint32-8 7.01GB/s ± 1% 7.02GB/s ± 1% ~ (p=0.160 n=29+27)
LittleEndianPutUint64-8 14.0GB/s ± 1% 14.0GB/s ± 1% ~ (p=0.413 n=29+29)
PutUvarint32-8 174MB/s ± 1% 173MB/s ± 1% ~ (p=0.648 n=25+30)
PutUvarint64-8 139MB/s ± 2% 140MB/s ± 1% ~ (p=0.271 n=30+26)
[Geo mean] 906MB/s 911MB/s +0.55%
name old alloc/op new alloc/op delta
ReadSlice1000Int32s-8 4.14kB ± 0% 4.13kB ± 0% -0.19% (p=0.000 n=30+30)
ReadStruct-8 200B ± 0% 200B ± 0% ~ (all equal)
ReadInts-8 64.0B ± 0% 32.0B ± 0% -50.00% (p=0.000 n=30+30)
WriteInts-8 112B ± 0% 64B ± 0% -42.86% (p=0.000 n=30+30)
WriteSlice1000Int32s-8 4.14kB ± 0% 4.13kB ± 0% -0.19% (p=0.000 n=30+30)
PutUint16-8 0.00B 0.00B ~ (all equal)
PutUint32-8 0.00B 0.00B ~ (all equal)
PutUint64-8 0.00B 0.00B ~ (all equal)
LittleEndianPutUint16-8 0.00B 0.00B ~ (all equal)
LittleEndianPutUint32-8 0.00B 0.00B ~ (all equal)
LittleEndianPutUint64-8 0.00B 0.00B ~ (all equal)
PutUvarint32-8 0.00B 0.00B ~ (all equal)
PutUvarint64-8 0.00B 0.00B ~ (all equal)
[Geo mean] 476B 370B -22.22%
name old allocs/op new allocs/op delta
ReadSlice1000Int32s-8 3.00 ± 0% 2.00 ± 0% -33.33% (p=0.000 n=30+30)
ReadStruct-8 16.0 ± 0% 16.0 ± 0% ~ (all equal)
ReadInts-8 8.00 ± 0% 8.00 ± 0% ~ (all equal)
WriteInts-8 14.0 ± 0% 14.0 ± 0% ~ (all equal)
WriteSlice1000Int32s-8 3.00 ± 0% 2.00 ± 0% -33.33% (p=0.000 n=30+30)
PutUint16-8 0.00 0.00 ~ (all equal)
PutUint32-8 0.00 0.00 ~ (all equal)
PutUint64-8 0.00 0.00 ~ (all equal)
LittleEndianPutUint16-8 0.00 0.00 ~ (all equal)
LittleEndianPutUint32-8 0.00 0.00 ~ (all equal)
LittleEndianPutUint64-8 0.00 0.00 ~ (all equal)
PutUvarint32-8 0.00 0.00 ~ (all equal)
PutUvarint64-8 0.00 0.00 ~ (all equal)
[Geo mean] 6.94 5.90 -14.97%
Change-Id: I3790b93e4190d98621d5f2c47e42929a18f56c2e
Reviewed-on: https://go-review.googlesource.com/133135
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
|
|
Fixes #22238
Change-Id: I8184f789bd4120f3e71c9374c7c2fcbfa95935bf
Reviewed-on: https://go-review.googlesource.com/132635
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
|
|
Fixes #26444
Fixes #27275
Change-Id: I9e8cbff79f7643ca8964c572c1a98172b6831730
GitHub-Last-Rev: 7eea2158b67ccab34b45a21e8f4289c36de02d93
GitHub-Pull-Request: golang/go#26719
Reviewed-on: https://go-review.googlesource.com/126897
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
|
|
Given the following types:
type S2 struct{ Field string }
type S struct{ *S2 }
Marshalling a value of type T1 should result in "{}", as there's no way
to access any value of T2.Field. This is how Go 1.10 and earlier
versions behave.
However, in the recent refactor golang.org/cl/125417 I broke this logic.
When the encoder found an anonymous struct pointer field that was nil,
it no longer skipped the embedded fields underneath it. This can be seen
in the added test:
--- FAIL: TestAnonymousFields/EmbeddedFieldBehindNilPointer (0.00s)
encode_test.go:430: Marshal() = "{\"Field\":\"\\u003c*json.S2 Value\\u003e\"}", want "{}"
The human error was a misplaced label, meaning we weren't actually
skipping the right loop iteration. Fix that.
Change-Id: Iba8a4a77d358dac73dcba4018498fe4f81afa263
Reviewed-on: https://go-review.googlesource.com/131376
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
|
|
TestRawMessage now passes without the need for the RawMessage field to
be a pointer. The TODO dates all the way back to 2010, so I presume the
issue has since been fixed.
TestNullRawMessage tested the decoding of a JSON null into a
*RawMessage. The existing behavior was correct, but for the sake of
completeness a non-pointer RawMessage field has been added too. The
non-pointer field behaves differently, as one can read in the docs:
To unmarshal JSON into a value implementing the Unmarshaler
interface, Unmarshal calls that value's UnmarshalJSON method,
including when the input is a JSON null.
Change-Id: Iabaed75d4ed10ea427d135ee1b80c6e6b83b2e6e
Reviewed-on: https://go-review.googlesource.com/131377
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
|
|
Encoders like map and array can use the much cheaper "i > 0" check to
see if we're not writing the first element. However, since struct fields
support omitempty, we need to keep track of that separately.
This is much more expensive - after calling the field encoder itself,
and retrieving the field via reflection, this branch was the third most
expensive piece of this field loop.
Instead, hoist the branch logic outside of the loop. The code doesn't
get much more complex, since we just delay the writing of each byte
until the next iteration. Yet the performance improvement is noticeable,
even when the struct types in CodeEncoder only have 2 and 7 fields,
respectively.
name old time/op new time/op delta
CodeEncoder-4 5.39ms ± 0% 5.31ms ± 0% -1.37% (p=0.010 n=4+6)
name old speed new speed delta
CodeEncoder-4 360MB/s ± 0% 365MB/s ± 0% +1.39% (p=0.010 n=4+6)
Updates #5683.
Change-Id: I2662cf459e0dfd68e56fa52bc898a417e84266c2
Reviewed-on: https://go-review.googlesource.com/131401
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
|
|
A few encoder struct types, such as map and slice, only encapsulate
other prepared encoder funcs. Using pointer receivers has no advantage,
and makes calling these methods slightly more expensive.
Not a huge performance win, but certainly an easy one. The struct types
used in the benchmark below contain one slice field and one pointer
field.
name old time/op new time/op delta
CodeEncoder-4 5.48ms ± 0% 5.39ms ± 0% -1.66% (p=0.010 n=6+4)
name old speed new speed delta
CodeEncoder-4 354MB/s ± 0% 360MB/s ± 0% +1.69% (p=0.010 n=6+4)
Updates #5683.
Change-Id: I9f78dbe07fcc6fbf19a6d96c22f5d6970db9eca4
Reviewed-on: https://go-review.googlesource.com/131400
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
|
|
First, use a dummy slice access on decode64 and decode32 to ensure that
there is a single bounds check for src.
Second, move the PutUint64/PutUint32 calls out of these functions,
meaning that they are simpler and smaller. This may also open the door
to inlineability in the future, but for now, they both go past the
budget.
While at it, get rid of the ilen and olen variables, which have no
impact whatsoever on performance. At least, not measurable by any of the
benchmarks.
name old time/op new time/op delta
DecodeString/2-4 54.3ns ± 1% 55.2ns ± 2% +1.60% (p=0.017 n=5+6)
DecodeString/4-4 66.6ns ± 1% 66.8ns ± 2% ~ (p=0.903 n=6+6)
DecodeString/8-4 79.3ns ± 2% 79.6ns ± 1% ~ (p=0.448 n=6+6)
DecodeString/64-4 300ns ± 1% 281ns ± 3% -6.54% (p=0.002 n=6+6)
DecodeString/8192-4 27.4µs ± 1% 23.7µs ± 2% -13.47% (p=0.002 n=6+6)
name old speed new speed delta
DecodeString/2-4 73.7MB/s ± 1% 72.5MB/s ± 2% -1.55% (p=0.026 n=5+6)
DecodeString/4-4 120MB/s ± 1% 120MB/s ± 2% ~ (p=0.851 n=6+6)
DecodeString/8-4 151MB/s ± 2% 151MB/s ± 1% ~ (p=0.485 n=6+6)
DecodeString/64-4 292MB/s ± 1% 313MB/s ± 3% +7.03% (p=0.002 n=6+6)
DecodeString/8192-4 399MB/s ± 1% 461MB/s ± 2% +15.58% (p=0.002 n=6+6)
For #19636.
Change-Id: I0dfbdafa2a41dc4c582f63aef94b90b8e473731c
Reviewed-on: https://go-review.googlesource.com/113776
Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
|
Some WriteByte('\\') calls can be deduplicated.
fillField is used in two occasions, but it is unnecessary when adding
fields to the "next" stack, as those aren't used for the final encoding.
Inline the func with its only remaining call.
Finally, unindent a default-if block.
The performance of the encoder is unaffected:
name old time/op new time/op delta
CodeEncoder-4 6.65ms ± 1% 6.65ms ± 0% ~ (p=0.662 n=6+5)
Change-Id: Ie55baeab89abad9b9f13e9f6ca886a670c30dba9
Reviewed-on: https://go-review.googlesource.com/122461
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
|
|
This function was only used in a single place - in the field encoding
loop within the struct encoder.
Inlining the function call manually lets us get rid of the call
overhead. But most importantly, it lets us simplify the logic afterward.
We no longer need to use reflect.Value{} and !fv.IsValid(), as we can
skip the field immediately.
The two factors combined (mostly just the latter) give a moderate speed
improvement to this hot loop.
name old time/op new time/op delta
CodeEncoder-4 6.01ms ± 1% 5.91ms ± 1% -1.66% (p=0.002 n=6+6)
name old speed new speed delta
CodeEncoder-4 323MB/s ± 1% 328MB/s ± 1% +1.69% (p=0.002 n=6+6)
Updates #5683.
Change-Id: I12757c325a68abb2856026cf719c122612a1f38e
Reviewed-on: https://go-review.googlesource.com/125417
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
|
