Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

desc: fix caching when wrapping protoreflect.Descriptor instances #606

Merged
merged 1 commit into from
Apr 9, 2024
Merged

desc: fix caching when wrapping protoreflect.Descriptor instances #606

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 0 additions & 9 deletions desc/cache.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,3 @@ func (c mapCache) get(d protoreflect.Descriptor) Descriptor {
func (c mapCache) put(key protoreflect.Descriptor, val Descriptor) {
c[key] = val
}

type noopCache struct{}

func (noopCache) get(protoreflect.Descriptor) Descriptor {
return nil
}

func (noopCache) put(protoreflect.Descriptor, Descriptor) {
}
4 changes: 1 addition & 3 deletions desc/convert.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,7 @@ func convertFile(d protoreflect.FileDescriptor, fd *descriptorpb.FileDescriptorP
ret.deps = make([]*FileDescriptor, len(fd.GetDependency()))
for i := 0; i < d.Imports().Len(); i++ {
f := d.Imports().Get(i).FileDescriptor
if c := cache.get(f); c != nil {
ret.deps[i] = c.(*FileDescriptor)
} else if c, err := wrapFile(f, cache); err != nil {
if c, err := wrapFile(f, cache); err != nil {
return nil, err
} else {
ret.deps[i] = c
Expand Down
7 changes: 0 additions & 7 deletions desc/load.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,6 @@ func LoadFileDescriptor(file string) (*FileDescriptor, error) {

var fd *FileDescriptor
loadedDescriptors.withLock(func(cache descriptorCache) {
// double-check cache, in case it was concurrently added while
// we were waiting for the lock
f := cache.get(d)
if f != nil {
fd = f.(*FileDescriptor)
return
}
fd, err = wrapFile(d, cache)
})
return fd, err
Expand Down
21 changes: 12 additions & 9 deletions desc/wrap.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ type DescriptorWrapper interface {
// WrapDescriptor wraps the given descriptor, returning a desc.Descriptor
// value that represents the same element.
func WrapDescriptor(d protoreflect.Descriptor) (Descriptor, error) {
return wrapDescriptor(d, noopCache{})
return wrapDescriptor(d, mapCache{})
}

func wrapDescriptor(d protoreflect.Descriptor, cache descriptorCache) (Descriptor, error) {
Expand Down Expand Up @@ -65,18 +65,21 @@ func WrapFiles(d []protoreflect.FileDescriptor) ([]*FileDescriptor, error) {
// WrapFile wraps the given file descriptor, returning a *desc.FileDescriptor
// value that represents the same file.
func WrapFile(d protoreflect.FileDescriptor) (*FileDescriptor, error) {
return wrapFile(d, noopCache{})
return wrapFile(d, mapCache{})
}

func wrapFile(d protoreflect.FileDescriptor, cache descriptorCache) (*FileDescriptor, error) {
if res := cache.get(d); res != nil {
return res.(*FileDescriptor), nil
}
fdp := protoutil.ProtoFromFileDescriptor(d)
return convertFile(d, fdp, cache)
}

// WrapMessage wraps the given message descriptor, returning a *desc.MessageDescriptor
// value that represents the same message.
func WrapMessage(d protoreflect.MessageDescriptor) (*MessageDescriptor, error) {
return wrapMessage(d, noopCache{})
return wrapMessage(d, mapCache{})
}

func wrapMessage(d protoreflect.MessageDescriptor, cache descriptorCache) (*MessageDescriptor, error) {
Expand All @@ -97,7 +100,7 @@ func wrapMessage(d protoreflect.MessageDescriptor, cache descriptorCache) (*Mess
// WrapField wraps the given field descriptor, returning a *desc.FieldDescriptor
// value that represents the same field.
func WrapField(d protoreflect.FieldDescriptor) (*FieldDescriptor, error) {
return wrapField(d, noopCache{})
return wrapField(d, mapCache{})
}

func wrapField(d protoreflect.FieldDescriptor, cache descriptorCache) (*FieldDescriptor, error) {
Expand All @@ -121,7 +124,7 @@ func wrapField(d protoreflect.FieldDescriptor, cache descriptorCache) (*FieldDes
// WrapOneOf wraps the given oneof descriptor, returning a *desc.OneOfDescriptor
// value that represents the same oneof.
func WrapOneOf(d protoreflect.OneofDescriptor) (*OneOfDescriptor, error) {
return wrapOneOf(d, noopCache{})
return wrapOneOf(d, mapCache{})
}

func wrapOneOf(d protoreflect.OneofDescriptor, cache descriptorCache) (*OneOfDescriptor, error) {
Expand All @@ -138,7 +141,7 @@ func wrapOneOf(d protoreflect.OneofDescriptor, cache descriptorCache) (*OneOfDes
// WrapEnum wraps the given enum descriptor, returning a *desc.EnumDescriptor
// value that represents the same enum.
func WrapEnum(d protoreflect.EnumDescriptor) (*EnumDescriptor, error) {
return wrapEnum(d, noopCache{})
return wrapEnum(d, mapCache{})
}

func wrapEnum(d protoreflect.EnumDescriptor, cache descriptorCache) (*EnumDescriptor, error) {
Expand All @@ -159,7 +162,7 @@ func wrapEnum(d protoreflect.EnumDescriptor, cache descriptorCache) (*EnumDescri
// WrapEnumValue wraps the given enum value descriptor, returning a *desc.EnumValueDescriptor
// value that represents the same enum value.
func WrapEnumValue(d protoreflect.EnumValueDescriptor) (*EnumValueDescriptor, error) {
return wrapEnumValue(d, noopCache{})
return wrapEnumValue(d, mapCache{})
}

func wrapEnumValue(d protoreflect.EnumValueDescriptor, cache descriptorCache) (*EnumValueDescriptor, error) {
Expand All @@ -176,7 +179,7 @@ func wrapEnumValue(d protoreflect.EnumValueDescriptor, cache descriptorCache) (*
// WrapService wraps the given service descriptor, returning a *desc.ServiceDescriptor
// value that represents the same service.
func WrapService(d protoreflect.ServiceDescriptor) (*ServiceDescriptor, error) {
return wrapService(d, noopCache{})
return wrapService(d, mapCache{})
}

func wrapService(d protoreflect.ServiceDescriptor, cache descriptorCache) (*ServiceDescriptor, error) {
Expand All @@ -193,7 +196,7 @@ func wrapService(d protoreflect.ServiceDescriptor, cache descriptorCache) (*Serv
// WrapMethod wraps the given method descriptor, returning a *desc.MethodDescriptor
// value that represents the same method.
func WrapMethod(d protoreflect.MethodDescriptor) (*MethodDescriptor, error) {
return wrapMethod(d, noopCache{})
return wrapMethod(d, mapCache{})
}

func wrapMethod(d protoreflect.MethodDescriptor, cache descriptorCache) (*MethodDescriptor, error) {
Expand Down