From 170b2bfca6e6b56c3de20f939798b56635e600bf Mon Sep 17 00:00:00 2001
From: Damien Neil <dneil@google.com>
Date: Fri, 24 Jan 2020 16:42:42 -0800
Subject: [PATCH] internal/impl: precompute required bit in validator
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Required field validation populates a bitmask of observed required
fields. Store a uint64 containing the bit to set in the validationInfo
rather than the index of the bit. Provides a noticeable speed increase
in validation.

name                             old time/op  new time/op  delta
EmptyMessage/Wire/Unmarshal      40.2ns ± 1%  40.2ns ± 2%    ~     (p=0.860 n=35+37)
EmptyMessage/Wire/Unmarshal-12   7.13ns ± 5%  7.12ns ± 1%    ~     (p=0.112 n=37+37)
RepeatedInt32/Wire/Unmarshal     6.57µs ± 1%  6.46µs ± 1%  -1.56%  (p=0.000 n=39+35)
RepeatedInt32/Wire/Unmarshal-12  1.05µs ± 2%  1.05µs ± 2%    ~     (p=0.659 n=37+33)
Required/Wire/Unmarshal           258ns ± 1%   251ns ± 1%  -2.87%  (p=0.000 n=32+38)
Required/Wire/Unmarshal-12       44.3ns ± 2%  42.4ns ± 1%  -4.36%  (p=0.000 n=36+37)

Change-Id: Ib1cb74d3e348355a6a2f66aecf8fdc4b58cd84d4
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/216420
Reviewed-by: Joe Tsai <joetsai@google.com>
---
 internal/impl/decode.go   |  5 +++--
 internal/impl/validate.go | 18 ++++++++++--------
 2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/internal/impl/decode.go b/internal/impl/decode.go
index 4a1d6312..4b1bc6d6 100644
--- a/internal/impl/decode.go
+++ b/internal/impl/decode.go
@@ -143,9 +143,10 @@ func (mi *MessageInfo) unmarshalPointer(b []byte, p pointer, groupTag wire.Numbe
 			var o unmarshalOutput
 			o, err = f.funcs.unmarshal(b, p.Apply(f.offset), wtyp, opts)
 			n = o.n
-			if reqi := f.validation.requiredIndex; reqi > 0 && err == nil {
-				requiredMask |= 1 << (reqi - 1)
+			if err != nil {
+				break
 			}
+			requiredMask |= f.validation.requiredBit
 			if f.funcs.isInit != nil && !o.initialized {
 				initialized = false
 			}
diff --git a/internal/impl/validate.go b/internal/impl/validate.go
index cb6a820f..093f0784 100644
--- a/internal/impl/validate.go
+++ b/internal/impl/validate.go
@@ -77,11 +77,13 @@ type validationInfo struct {
 	typ              validationType
 	keyType, valType validationType
 
-	// For non-required fields, requiredIndex is 0.
+	// For non-required fields, requiredBit is 0.
 	//
-	// For required fields, requiredIndex is unique index in the range
-	// (0, MessageInfo.numRequiredFields].
-	requiredIndex uint8
+	// For required fields, requiredBit's nth bit is set, where n is a
+	// unique index in the range [0, MessageInfo.numRequiredFields).
+	//
+	// If there are more than 64 required fields, requiredBit is 0.
+	requiredBit uint64
 }
 
 type validationType uint8
@@ -131,7 +133,7 @@ func newFieldValidationInfo(mi *MessageInfo, si structInfo, fd pref.FieldDescrip
 		// of the required fields past 64.
 		if mi.numRequiredFields < math.MaxUint8 {
 			mi.numRequiredFields++
-			vi.requiredIndex = mi.numRequiredFields
+			vi.requiredBit = 1 << (mi.numRequiredFields - 1)
 		}
 	}
 	return vi
@@ -272,7 +274,7 @@ State:
 				case 2:
 					vi.typ = st.valType
 					vi.mi = st.mi
-					vi.requiredIndex = 1
+					vi.requiredBit = 1
 				}
 			default:
 				var f *coderFieldInfo
@@ -321,7 +323,7 @@ State:
 					vi = getExtensionFieldInfo(xt).validation
 				}
 			}
-			if vi.requiredIndex > 0 {
+			if vi.requiredBit != 0 {
 				// Check that the field has a compatible wire type.
 				// We only need to consider non-repeated field types,
 				// since repeated fields (and maps) can never be required.
@@ -337,7 +339,7 @@ State:
 					ok = wtyp == wire.BytesType
 				}
 				if ok {
-					st.requiredMask |= 1 << (vi.requiredIndex - 1)
+					st.requiredMask |= vi.requiredBit
 				}
 			}
 			switch vi.typ {