-
Notifications
You must be signed in to change notification settings - Fork 18k
encoding/json: stateEndTop allocates an error it doesn't return #17914
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
Comments
May be worth providing a runnable example |
Actually according to the comment for stateEndTop it seems it is intended to be called only when the top-level closes. However it is called multiple times. I put a print statement in stateEndTop. On the simple example above it gets called twice: once with ' ' and once with '}' as the argument c. // stateEndTop is the state after finishing the top-level value,
// such as after reading `{}` or `[1,2,3]`.
// Only space characters should be seen now.
func stateEndTop(s *scanner, c byte) int { |
At the end of an object or array in the first level of the json object the parseLevel falls back to 0 and the code calls stateEndTop. |
I don't understand the bug here; it looks to me like the Unmarshal call is correctly returning a nil error. It sounds like you think there is some optimization to be done? |
CL https://golang.org/cl/33276 mentions this issue. |
This reverts commit df68afd (https://golang.org/cl/33276) Reason for revert: made other benchmarks worse Fixes #20693 (details) Updates #17914 Updates #10335 Change-Id: If451b620803ccb0536b89c76c4353d2185d57d7e Reviewed-on: https://go-review.googlesource.com/47211 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Russ Cox <rsc@golang.org>
Change https://golang.org/cl/47152 mentions this issue: |
Change https://golang.org/cl/47211 mentions this issue: |
Assigned to @rsc for review of https://go-review.googlesource.com/c/go/+/47152 |
What version of Go are you using (
go version
)?go version go1.7 darwin/amd64
What operating system and processor architecture are you using (
go env
)?GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/almcpherson/gopath"
GORACE=""
GOROOT="/Users/almcpherson/go1.7"
GOTOOLDIR="/Users/almcpherson/go1.7/pkg/tool/darwin_amd64"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/jv/2c5lqc993lgdnm85hggybmw43959v8/T/go-build941373586=/tmp/go-build -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"
What did you do?
Running benchmarks to see why
func stateEndTop(s *scanner, c byte) int
is making calls to
func (s *scanner) error(c byte, context string) int
on valid json.
https://play.golang.org/p/Fsvp3vGwXO
go test -bench "BenchmarkLittleJSON" -cpuprofile c.out
go tool pprof -peek error test.test c.out
What did you expect to see?
no calls to error
What did you see instead?
calls to error
1.18s of 1.18s total ( 100%)
----------------------------------------------------------+-------------
flat flat% sum% cum cum% calls calls% + context
----------------------------------------------------------+-------------
0.21s 100% | encoding/json.stateEndTop
0 0% 0% 0.21s 17.80% | encoding/json.(*scanner).error
0.11s 52.38% | runtime.concatstring4
0.07s 33.33% | encoding/json.quoteChar
0.03s 14.29% | runtime.newobject
----------------------------------------------------------+-------------
The text was updated successfully, but these errors were encountered: