encoding/json: reduce unmarshal mallocs for unmapped fields

JSON decoding performs poorly for unmapped and ignored fields. We noticed better
performance when unmarshalling unused fields. The loss comes mostly from calls
to scanner.error as described at #17914.

benchmark                 old ns/op     new ns/op     delta
BenchmarkIssue10335-8     431           408           -5.34%
BenchmarkUnmapped-8       1744          1314          -24.66%

benchmark                 old allocs     new allocs     delta
BenchmarkIssue10335-8     4              3              -25.00%
BenchmarkUnmapped-8       18             4              -77.78%

benchmark                 old bytes     new bytes     delta
BenchmarkIssue10335-8     320           312           -2.50%
BenchmarkUnmapped-8       568           344           -39.44%

Fixes #17914, improves #10335

Change-Id: I7d4258a94eb287c0fe49e7334795209b90434cd0
Reviewed-on: https://go-review.googlesource.com/33276Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

src/encoding/json/bench_test.go

@@ -221,3 +221,14 @@ func BenchmarkIssue10335(b *testing.B) {
 		}
 	}
 }
+
+func BenchmarkUnmapped(b *testing.B) {
+	b.ReportAllocs()
+	var s struct{}
+	j := []byte(`{"s": "hello", "y": 2, "o": {"x": 0}, "a": [1, 99, {"x": 1}]}`)
+	for n := 0; n < b.N; n++ {
+		if err := Unmarshal(j, &s); err != nil {
+			b.Fatal(err)
+		}
+	}
+}

src/encoding/json/decode.go

@@ -359,47 +359,40 @@ func (d *decodeState) scanWhile(op int) int {
 	return newOp
 }
 
+// discardObject and discardArray are dummy data targets
+// used by the (*decodeState).value method, which
+// accepts a zero reflect.Value to discard a value.
+// The (*decodeState).object and (*decodeState).array methods,
+// however, require a valid reflect.Value destination.
+// These are the target values used when the caller of value
+// wants to skip a field.
+//
+// Because these values refer to zero-sized objects
+// and thus can't be mutated, they're safe for concurrent use
+// by different goroutines unmarshalling skipped fields.
+var (
+	discardObject = reflect.ValueOf(struct{}{})
+	discardArray  = reflect.ValueOf([0]interface{}{})
+)
+
 // value decodes a JSON value from d.data[d.off:] into the value.
-// it updates d.off to point past the decoded value.
+// It updates d.off to point past the decoded value. If v is
+// invalid, the JSON value is discarded.
 func (d *decodeState) value(v reflect.Value) {
-	if !v.IsValid() {
-		_, rest, err := nextValue(d.data[d.off:], &d.nextscan)
-		if err != nil {
-			d.error(err)
-		}
-		d.off = len(d.data) - len(rest)
-
-		// d.scan thinks we're still at the beginning of the item.
-		// Feed in an empty string - the shortest, simplest value -
-		// so that it knows we got to the end of the value.
-		if d.scan.redo {
-			// rewind.
-			d.scan.redo = false
-			d.scan.step = stateBeginValue
-		}
-		d.scan.step(&d.scan, '"')
-		d.scan.step(&d.scan, '"')
-
-		n := len(d.scan.parseState)
-		if n > 0 && d.scan.parseState[n-1] == parseObjectKey {
-			// d.scan thinks we just read an object key; finish the object
-			d.scan.step(&d.scan, ':')
-			d.scan.step(&d.scan, '"')
-			d.scan.step(&d.scan, '"')
-			d.scan.step(&d.scan, '}')
-		}
-
-		return
-	}
-
 	switch op := d.scanWhile(scanSkipSpace); op {
 	default:
 		d.error(errPhase)
 
 	case scanBeginArray:
+		if !v.IsValid() {
+			v = discardArray
+		}
 		d.array(v)
 
 	case scanBeginObject:
+		if !v.IsValid() {
+			v = discardObject
+		}
 		d.object(v)
 
 	case scanBeginLiteral:
@@ -517,8 +510,7 @@ func (d *decodeState) array(v reflect.Value) {
 		d.off--
 		d.next()
 		return
-	case reflect.Array:
-	case reflect.Slice:
+	case reflect.Array, reflect.Slice:
 		break
 	}
 
@@ -797,7 +789,9 @@ func (d *decodeState) literal(v reflect.Value) {
 	d.off--
 	d.scan.undo(op)
 
-	d.literalStore(d.data[start:d.off], v, false)
+	if v.IsValid() {
+		d.literalStore(d.data[start:d.off], v, false)
+	}
 }
 
 // convertNumber converts the number literal s to a float64 or a Number