Skip to content

Commit

Permalink
perf: reduce allocations in MergedFileDescriptors (#59)
Browse files Browse the repository at this point in the history
  • Loading branch information
mark-rushakoff authored Apr 13, 2023
1 parent 0c06433 commit 7dca9a2
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 13 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## Unreleased

### Improvements

- [#59](https://github.com/cosmos/gogoproto/pull/59) Reuse buffers and gzip readers to reduce memory allocations during MergedFileDescriptors.

## [v1.4.7](https://github.com/cosmos/gogoproto/releases/tag/v1.4.7) - 2023-03-30

### Bug Fixes
Expand Down
37 changes: 25 additions & 12 deletions proto/merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"compress/gzip"
"errors"
"fmt"
"io"
"strings"

"github.com/google/go-cmp/cmp"
Expand Down Expand Up @@ -39,7 +38,11 @@ func DebugFileDescriptorsMismatch() error {
}

func mergedFileDescriptors(debug bool) (*descriptorpb.FileDescriptorSet, error) {
fds := &descriptorpb.FileDescriptorSet{}
fds := &descriptorpb.FileDescriptorSet{
// Pre-size the Files since we are going to copy them
// when we range over protoregistry.GlobalFiles.
File: make([]*descriptorpb.FileDescriptorProto, 0, protoregistry.GlobalFiles.NumFiles()),
}

// While combing through the file descriptors, we'll also log any errors
// we encounter.
Expand All @@ -60,26 +63,36 @@ func mergedFileDescriptors(debug bool) (*descriptorpb.FileDescriptorSet, error)
return true
})

// load gogo proto file descriptors
gogoFds := AllFileDescriptors()
for _, compressedBz := range gogoFds {
rdr, err := gzip.NewReader(bytes.NewReader(compressedBz))
if err != nil {
// Reuse a single gzip reader throughout the loop,
// so we don't have to repeatedly allocate new readers.
gzr := new(gzip.Reader)

// Also reuse a single byte buffer for each gzip read.
buf := new(bytes.Buffer)

// Load gogo proto file descriptors.
// Normal usage would go through the AllFileDescriptors method,
// which returns a copy of the package-level map.
//
// In tests especially, this method can be part of a hot call stack.
// Because we are in the same package and we know what we're doing,
// we can read from the raw map.
for _, compressedBz := range protoFiles {
if err := gzr.Reset(bytes.NewReader(compressedBz)); err != nil {
return nil, err
}

bz, err := io.ReadAll(rdr)
if err != nil {
buf.Reset()
if _, err := buf.ReadFrom(gzr); err != nil {
return nil, err
}

fd := &descriptorpb.FileDescriptorProto{}
err = protov2.Unmarshal(bz, fd)
if err != nil {
if err := protov2.Unmarshal(buf.Bytes(), fd); err != nil {
return nil, err
}

err = CheckImportPath(fd.GetName(), fd.GetPackage())
err := CheckImportPath(fd.GetName(), fd.GetPackage())
if err != nil {
checkImportErr = append(checkImportErr, err.Error())
}
Expand Down
2 changes: 1 addition & 1 deletion proto/properties.go
Original file line number Diff line number Diff line change
Expand Up @@ -614,7 +614,7 @@ func FileDescriptor(filename string) []byte { return protoFiles[filename] }
// FileDescriptorProto.
func AllFileDescriptors() map[string][]byte {
// we clone the map to prevent the caller from mutating it
cloned := map[string][]byte{}
cloned := make(map[string][]byte, len(protoFiles))
for file, bz := range protoFiles {
cloned[file] = bz
}
Expand Down

0 comments on commit 7dca9a2

Please sign in to comment.