errbase: optimize errors.Is

Author: jeffswenson

errbase/encode.go

@@ -305,6 +305,32 @@ func GetTypeMark(err error) errorspb.ErrorTypeMark {
 	return errorspb.ErrorTypeMark{FamilyName: familyName, Extension: extension}
 }
 
+// EqualTypeMark checks whether `GetTypeMark(e1).Equals(GetTypeMark(e2))`. It
+// is written to be be optimized for the case where neither error has
+// serialized type information.
+func EqualTypeMark(e1, e2 error) bool {
+	slowPath := func(err error) bool {
+		switch err.(type) {
+		case *opaqueLeaf:
+			return true
+		case *opaqueLeafCauses:
+			return true
+		case *opaqueWrapper:
+			return true
+		case TypeKeyMarker:
+			return true
+		}
+		return false
+	}
+	if slowPath(e1) || slowPath(e2) {
+		return GetTypeMark(e1).Equals(GetTypeMark(e2))
+	}
+
+	t1 := reflect.TypeOf(e1)
+	t2 := reflect.TypeOf(e2)
+	return t1.PkgPath() == t2.PkgPath() && t1.String() == t2.String()
+}
+
 // RegisterLeafEncoder can be used to register new leaf error types to
 // the library. Registered types will be encoded using their own
 // Go type when an error is encoded. Wrappers that have not been
@@ -385,9 +411,7 @@ func RegisterWrapperEncoder(theType TypeKey, encoder WrapperEncoder) {
 // Note: if the error type has been migrated from a previous location
 // or a different type, ensure that RegisterTypeMigration() was called
 // prior to RegisterWrapperEncoder().
-func RegisterWrapperEncoderWithMessageType(
-	theType TypeKey, encoder WrapperEncoderWithMessageType,
-) {
+func RegisterWrapperEncoderWithMessageType(theType TypeKey, encoder WrapperEncoderWithMessageType) {
 	if encoder == nil {
 		delete(encoders, theType)
 	} else {

markers/markers.go

@@ -75,22 +75,40 @@ func Is(err, reference error) bool {
 		return false
 	}
 
-	// Not directly equal. Try harder, using error marks. We don't do
-	// this during the loop above as it may be more expensive.
-	//
-	// Note: there is a more effective recursive algorithm that ensures
-	// that any pair of string only gets compared once. Should the
-	// following code become a performance bottleneck, that algorithm
-	// can be considered instead.
-	refMark := getMark(reference)
-	for c := err; c != nil; c = errbase.UnwrapOnce(c) {
-		if equalMarks(getMark(c), refMark) {
+	return checkMark(err, reference)
+}
+
+func checkMark(err, reference error) bool {
+	for errNext := err; errNext != nil; errNext = errbase.UnwrapOnce(errNext) {
+		if isMarkEqual(errNext, reference) {
 			return true
 		}
 	}
 	return false
 }
 
+func isMarkEqual(err, reference error) bool {
+	_, errIsMark := err.(*withMark)
+	_, refIsMark := reference.(*withMark)
+	if errIsMark || refIsMark {
+		// If either error is a mark, use the more general
+		// equalMarks() function.
+		return equalMarks(getMark(err), getMark(reference))
+	}
+
+	m1 := err
+	m2 := reference
+	for m1 != nil {
+		if !errbase.EqualTypeMark(m1, m2) {
+			return false
+		}
+		m1 = errbase.UnwrapOnce(m1)
+		m2 = errbase.UnwrapOnce(m2)
+	}
+
+	return safeGetErrMsg(err) == safeGetErrMsg(reference)
+}
+
 func tryDelegateToIsMethod(err, reference error) bool {
 	if x, ok := err.(interface{ Is(error) bool }); ok && x.Is(reference) {
 		return true
@@ -222,6 +240,8 @@ func equalMarks(m1, m2 errorMark) bool {
 		return false
 	}
 	for i, t := range m1.types {
+		// TODO(jeffswenson): I think there is a bug here. What if the chains
+		// are of different lengths?
 		if !t.Equals(m2.types[i]) {
 			return false
 		}
@@ -234,7 +254,10 @@ func getMark(err error) errorMark {
 	if m, ok := err.(*withMark); ok {
 		return m.mark
 	}
-	m := errorMark{msg: safeGetErrMsg(err), types: []errorspb.ErrorTypeMark{errbase.GetTypeMark(err)}}
+	m := errorMark{
+		msg:   safeGetErrMsg(err),
+		types: []errorspb.ErrorTypeMark{errbase.GetTypeMark(err)},
+	}
 	for c := errbase.UnwrapOnce(err); c != nil; c = errbase.UnwrapOnce(c) {
 		m.types = append(m.types, errbase.GetTypeMark(c))
 	}