From 380c339ec12310c04cd1c399669ab0bce57b6cd6 Mon Sep 17 00:00:00 2001
From: Mitko Asenov <dasenov@google.com>
Date: Thu, 9 Jun 2022 11:53:27 +0200
Subject: [PATCH] proto: short-circuit Equal when inputs are identical
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

I added benchmarks (measured on Intel(R) Xeon(R) CPU E5-1650 v4 @ 3.60GHz) that show the difference:

name                                  old time/op  new time/op  delta
EqualWithSmallEmpty-12                 241ns ± 6%   242ns ± 6%     ~     (p=0.796 n=10+10)
EqualWithIdenticalPtrEmpty-12          241ns ± 3%     7ns ± 4%  -97.19%  (p=0.000 n=10+10)
EqualWithLargeEmpty-12                2.68µs ± 3%  2.59µs ± 3%   -3.27%  (p=0.000 n=10+10)
EqualWithDeeplyNestedEqual-12         73.9µs ± 3%  71.8µs ± 1%   -2.91%   (p=0.000 n=10+9)
EqualWithDeeplyNestedDifferent-12     20.0µs ± 5%  19.4µs ± 5%   -3.06%  (p=0.029 n=10+10)
EqualWithDeeplyNestedIdenticalPtr-12  73.9µs ± 4%   0.0µs ± 2%  -99.99%  (p=0.000 n=10+10)

Change-Id: I1b83fa477d6432eafd355b322f507cf90b9a6751
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/411377
Reviewed-by: Lasse Folger <lassefolger@google.com>
Reviewed-by: Joseph Tsai <joetsai@digital-static.net>
Reviewed-by: Michael Stapelberg <stapelberg@google.com>
---
 proto/equal.go      |   4 ++
 proto/equal_test.go | 105 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 109 insertions(+)

diff --git a/proto/equal.go b/proto/equal.go
index 5017d540c..49e16ba75 100644
--- a/proto/equal.go
+++ b/proto/equal.go
@@ -33,6 +33,10 @@ func Equal(x, y Message) bool {
 	if x == nil || y == nil {
 		return x == nil && y == nil
 	}
+	if reflect.TypeOf(x).Kind() == reflect.Pointer && x == y {
+		// Avoid an expensive comparison if both inputs are identical pointers.
+		return true
+	}
 	mx := x.ProtoReflect()
 	my := y.ProtoReflect()
 	if mx.IsValid() != my.IsValid() {
diff --git a/proto/equal_test.go b/proto/equal_test.go
index 2def30f18..c17e14ff3 100644
--- a/proto/equal_test.go
+++ b/proto/equal_test.go
@@ -9,6 +9,7 @@ import (
 	"testing"
 
 	"google.golang.org/protobuf/encoding/prototext"
+	"google.golang.org/protobuf/internal/pragma"
 	"google.golang.org/protobuf/proto"
 	"google.golang.org/protobuf/testing/protopack"
 
@@ -17,6 +18,13 @@ import (
 )
 
 func TestEqual(t *testing.T) {
+	identicalPtrPb := &testpb.TestAllTypes{MapStringString: map[string]string{"a": "b", "c": "d"}}
+
+	type incomparableMessage struct {
+		*testpb.TestAllTypes
+		pragma.DoNotCompare
+	}
+
 	tests := []struct {
 		x, y proto.Message
 		eq   bool
@@ -55,6 +63,34 @@ func TestEqual(t *testing.T) {
 			eq: false,
 		},
 
+		// Identical input pointers
+		{
+			x:  identicalPtrPb,
+			y:  identicalPtrPb,
+			eq: true,
+		},
+
+		// Incomparable types. The top-level types are not actually directly
+		// compared (which would panic), but rather the comparison happens on the
+		// objects returned by ProtoReflect(). These tests are here just to ensure
+		// that any short-circuit checks do not accidentally try to compare
+		// incomparable top-level types.
+		{
+			x:  incomparableMessage{TestAllTypes: identicalPtrPb},
+			y:  incomparableMessage{TestAllTypes: identicalPtrPb},
+			eq: true,
+		},
+		{
+			x:  identicalPtrPb,
+			y:  incomparableMessage{TestAllTypes: identicalPtrPb},
+			eq: true,
+		},
+		{
+			x:  identicalPtrPb,
+			y:  &incomparableMessage{TestAllTypes: identicalPtrPb},
+			eq: true,
+		},
+
 		// Proto2 scalars.
 		{
 			x: &testpb.TestAllTypes{OptionalInt32: proto.Int32(1)},
@@ -562,3 +598,72 @@ func TestEqual(t *testing.T) {
 		}
 	}
 }
+
+func BenchmarkEqualWithSmallEmpty(b *testing.B) {
+	x := &testpb.ForeignMessage{}
+	y := &testpb.ForeignMessage{}
+
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		proto.Equal(x, y)
+	}
+}
+
+func BenchmarkEqualWithIdenticalPtrEmpty(b *testing.B) {
+	x := &testpb.ForeignMessage{}
+
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		proto.Equal(x, x)
+	}
+}
+
+func BenchmarkEqualWithLargeEmpty(b *testing.B) {
+	x := &testpb.TestAllTypes{}
+	y := &testpb.TestAllTypes{}
+
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		proto.Equal(x, y)
+	}
+}
+
+func makeNested(depth int) *testpb.TestAllTypes {
+	if depth <= 0 {
+		return nil
+	}
+	return testpb.TestAllTypes_builder{
+		OptionalNestedMessage: testpb.TestAllTypes_NestedMessage_builder{
+			Corecursive: makeNested(depth - 1),
+		}.Build(),
+	}.Build()
+}
+
+func BenchmarkEqualWithDeeplyNestedEqual(b *testing.B) {
+	x := makeNested(20)
+	y := makeNested(20)
+
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		proto.Equal(x, y)
+	}
+}
+
+func BenchmarkEqualWithDeeplyNestedDifferent(b *testing.B) {
+	x := makeNested(20)
+	y := makeNested(21)
+
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		proto.Equal(x, y)
+	}
+}
+
+func BenchmarkEqualWithDeeplyNestedIdenticalPtr(b *testing.B) {
+	x := makeNested(20)
+
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		proto.Equal(x, x)
+	}
+}