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>
This commit is contained in:
Damien Neil 2019-06-24 12:58:17 -07:00
parent 20aefe9c5e
commit a9940822d4
8 changed files with 12 additions and 63 deletions

View File

@ -342,10 +342,6 @@ func TestLegacyExtensions(t *testing.T) {
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 @@ func TestLegacyExtensions(t *testing.T) {
}
}
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 @@ func TestLegacyExtensions(t *testing.T) {
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) {

View File

@ -360,15 +360,6 @@ func (m *messageReflectWrapper) ProtoUnwrap() interface{} {
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 @@ func (m *messageReflectWrapper) checkField(fd pref.FieldDescriptor) (*fieldInfo,
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 {

View File

@ -26,13 +26,6 @@ func (m *message) Interface() pref.ProtoMessage { return (*IrregularMessag
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))

View File

@ -145,7 +145,7 @@ func (o MarshalOptions) rangeFields(m protoreflect.Message, f func(protoreflect.
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

View File

@ -35,11 +35,10 @@ func equalMessage(mx, my pref.Message) bool {
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 @@ func equalMessage(mx, my pref.Message) bool {
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())
}

View File

@ -49,9 +49,6 @@ func TestReset(t *testing.T) {
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)
}

View File

@ -40,12 +40,9 @@ type Message interface {
// 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)

View File

@ -71,22 +71,6 @@ func (m *Message) Interface() pref.ProtoMessage {
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) {