all: remove protoreflect.Message.Len
Len looks like it should be O(1), but the need to check for
non-zero-length repeated fields makes it at minimum O(n) where n is
the number of repeated fields. In practice, it's O(n) where n is the
number of fields altogether.
The Len function is not especially useful, easily duplicated with Range
and a counter, and can be surprisingly inefficient. Drop it.
Change-Id: I24b27433217e131e842bd18dd58475bcdf62ef97
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/183678
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
diff --git a/internal/impl/legacy_test.go b/internal/impl/legacy_test.go
index a51ae03..1c758d2 100644
--- a/internal/impl/legacy_test.go
+++ b/internal/impl/legacy_test.go
@@ -342,10 +342,6 @@
m := pimpl.Export{}.MessageOf(new(LegacyTestMessage))
- if n := m.Len(); n != 0 {
- t.Errorf("KnownFields.Len() = %v, want 0", n)
- }
-
// Check that getting the zero value returns the default value for scalars,
// nil for singular messages, and an empty list for repeated fields.
defaultValues := map[int]interface{}{
@@ -444,10 +440,6 @@
}
}
- if n := m.Len(); n != 20 {
- t.Errorf("Message.Len() = %v, want 0", n)
- }
-
// Clear all singular fields and truncate all repeated fields.
for _, xt := range extensionTypes[:len(extensionTypes)/2] {
m.Clear(xt)
@@ -455,17 +447,11 @@
for _, xt := range extensionTypes[len(extensionTypes)/2:] {
m.Get(xt).List().Truncate(0)
}
- if n := m.Len(); n != 10 {
- t.Errorf("Message.Len() = %v, want 10", n)
- }
// Clear all repeated fields.
for _, xt := range extensionTypes[len(extensionTypes)/2:] {
m.Clear(xt)
}
- if n := m.Len(); n != 0 {
- t.Errorf("Message.Len() = %v, want 0", n)
- }
}
func TestExtensionConvert(t *testing.T) {
diff --git a/internal/impl/message.go b/internal/impl/message.go
index 7c4873a..1226511 100644
--- a/internal/impl/message.go
+++ b/internal/impl/message.go
@@ -360,15 +360,6 @@
return m.p.AsIfaceOf(m.mi.GoType.Elem())
}
-func (m *messageReflectWrapper) Len() (cnt int) {
- m.mi.init()
- for _, fi := range m.mi.fields {
- if fi.has(m.p) {
- cnt++
- }
- }
- return cnt + m.mi.extensionMap(m.p).Len()
-}
func (m *messageReflectWrapper) Range(f func(pref.FieldDescriptor, pref.Value) bool) {
m.mi.init()
for _, fi := range m.mi.fields {
@@ -463,12 +454,6 @@
type extensionMap map[int32]ExtensionField
-func (m *extensionMap) Len() int {
- if m != nil {
- return len(*m)
- }
- return 0
-}
func (m *extensionMap) Range(f func(pref.FieldDescriptor, pref.Value) bool) {
if m != nil {
for _, x := range *m {
diff --git a/internal/testprotos/irregular/irregular.go b/internal/testprotos/irregular/irregular.go
index 2a7318b..6685e43 100644
--- a/internal/testprotos/irregular/irregular.go
+++ b/internal/testprotos/irregular/irregular.go
@@ -26,13 +26,6 @@
var fieldDescS = descriptor.Messages().Get(0).Fields().Get(0)
-func (m *message) Len() int {
- if m.set {
- return 1
- }
- return 0
-}
-
func (m *message) Range(f func(pref.FieldDescriptor, pref.Value) bool) {
if m.set {
f(fieldDescS, pref.ValueOf(m.value))
diff --git a/proto/encode.go b/proto/encode.go
index 87a0394..5139cce 100644
--- a/proto/encode.go
+++ b/proto/encode.go
@@ -145,7 +145,7 @@
m.Range(f)
return
}
- fds := make([]protoreflect.FieldDescriptor, 0, m.Len())
+ var fds []protoreflect.FieldDescriptor
m.Range(func(fd protoreflect.FieldDescriptor, _ protoreflect.Value) bool {
fds = append(fds, fd)
return true
diff --git a/proto/equal.go b/proto/equal.go
index 6f64434..1aca670 100644
--- a/proto/equal.go
+++ b/proto/equal.go
@@ -35,11 +35,10 @@
return false
}
- if mx.Len() != my.Len() {
- return false
- }
+ nx := 0
equal := true
mx.Range(func(fd pref.FieldDescriptor, vx pref.Value) bool {
+ nx++
vy := my.Get(fd)
equal = my.Has(fd) && equalField(fd, vx, vy)
return equal
@@ -47,6 +46,14 @@
if !equal {
return false
}
+ ny := 0
+ my.Range(func(fd pref.FieldDescriptor, vx pref.Value) bool {
+ ny++
+ return true
+ })
+ if nx != ny {
+ return false
+ }
return equalUnknown(mx.GetUnknown(), my.GetUnknown())
}
diff --git a/proto/reset_test.go b/proto/reset_test.go
index 9447df6..b9ad5b7 100644
--- a/proto/reset_test.go
+++ b/proto/reset_test.go
@@ -49,9 +49,6 @@
t.Errorf("m.OneofField = %p, want nil", m.OneofField)
}
- if got := m.ProtoReflect().Len(); got != 0 {
- t.Errorf("m.ProtoReflect().Len() = %d, want 0", got)
- }
if got := m.ProtoReflect().GetUnknown(); got != nil {
t.Errorf("m.ProtoReflect().GetUnknown() = %d, want nil", got)
}
diff --git a/reflect/protoreflect/value.go b/reflect/protoreflect/value.go
index 9347e9d..e42549f 100644
--- a/reflect/protoreflect/value.go
+++ b/reflect/protoreflect/value.go
@@ -40,12 +40,9 @@
// returns the underlying ProtoMessage interface.
Interface() ProtoMessage
- // Len reports the number of populated fields (i.e., Has reports true).
- Len() int
-
// Range iterates over every populated field in an undefined order,
// calling f for each field descriptor and value encountered.
- // Range calls f Len times unless f returns false, which stops iteration.
+ // Range returns immediately if f returns false.
// While iterating, mutating operations may only be performed
// on the current field descriptor.
Range(f func(FieldDescriptor, Value) bool)
diff --git a/types/dynamicpb/dynamic.go b/types/dynamicpb/dynamic.go
index 9d2d6fb..cae3399 100644
--- a/types/dynamicpb/dynamic.go
+++ b/types/dynamicpb/dynamic.go
@@ -71,22 +71,6 @@
return m
}
-// Len returns the number of populated fields.
-// See protoreflect.Message for details.
-func (m *Message) Len() int {
- count := 0
- for num, v := range m.known {
- if m.ext[num] != nil {
- count++
- continue
- }
- if isSet(m.desc.Fields().ByNumber(num), v) {
- count++
- }
- }
- return count
-}
-
// Range visits every populated field in undefined order.
// See protoreflect.Message for details.
func (m *Message) Range(f func(pref.FieldDescriptor, pref.Value) bool) {