Skip to content

Commit

Permalink
Small performance fix
Browse files Browse the repository at this point in the history
Move static regexp to a global variable in order to avoid multiple
regexp pattern compilations.
  • Loading branch information
Dmitry Shemin authored and johanbrandhorst committed Sep 4, 2019
1 parent 82d4276 commit 7f57073
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 8 deletions.
7 changes: 4 additions & 3 deletions protoc-gen-swagger/genswagger/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -612,6 +612,8 @@ func resolveFullyQualifiedNameToSwaggerNames(messages []string, useFQNForSwagger
return uniqueNames
}

var canRegexp = regexp.MustCompile("{([a-zA-Z][a-zA-Z0-9_.]*).*}")

// Swagger expects paths of the form /path/{string_value} but grpc-gateway paths are expected to be of the form /path/{string_value=strprefix/*}. This should reformat it correctly.
func templateToSwaggerPath(path string, reg *descriptor.Registry) string {
// It seems like the right thing to do here is to just use
Expand Down Expand Up @@ -670,14 +672,13 @@ func templateToSwaggerPath(path string, reg *descriptor.Registry) string {
// Parts is now an array of segments of the path. Interestingly, since the
// syntax for this subsection CAN be handled by a regexp since it has no
// memory.
re := regexp.MustCompile("{([a-zA-Z][a-zA-Z0-9_.]*).*}")
for index, part := range parts {
// If part is a resource name such as "parent", "name", "user.name", the format info must be retained.
prefix := re.ReplaceAllString(part, "$1")
prefix := canRegexp.ReplaceAllString(part, "$1")
if isResourceName(prefix) {
continue
}
parts[index] = re.ReplaceAllString(part, "{$1}")
parts[index] = canRegexp.ReplaceAllString(part, "{$1}")
}

return strings.Join(parts, "/")
Expand Down
22 changes: 22 additions & 0 deletions protoc-gen-swagger/genswagger/template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -930,6 +930,28 @@ func TestTemplateToSwaggerPath(t *testing.T) {
}
}

func BenchmarkTemplateToSwaggerPath(b *testing.B) {
const input = "/{user.name=prefix1/*/prefix2/*}:customMethod"

b.Run("with JSON names", func(b *testing.B) {
reg := descriptor.NewRegistry()
reg.SetUseJSONNamesForFields(false)

for i := 0; i < b.N; i++ {
_ = templateToSwaggerPath(input, reg)
}
})

b.Run("without JSON names", func(b *testing.B) {
reg := descriptor.NewRegistry()
reg.SetUseJSONNamesForFields(true)

for i := 0; i < b.N; i++ {
_ = templateToSwaggerPath(input, reg)
}
})
}

func TestResolveFullyQualifiedNameToSwaggerName(t *testing.T) {
var tests = []struct {
input string
Expand Down
8 changes: 3 additions & 5 deletions runtime/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,13 @@ import (
"google.golang.org/grpc/grpclog"
)

var valuesKeyRegexp = regexp.MustCompile("^(.*)\\[(.*)\\]$")

// PopulateQueryParameters populates "values" into "msg".
// A value is ignored if its key starts with one of the elements in "filter".
func PopulateQueryParameters(msg proto.Message, values url.Values, filter *utilities.DoubleArray) error {
for key, values := range values {
re, err := regexp.Compile("^(.*)\\[(.*)\\]$")
if err != nil {
return err
}
match := re.FindStringSubmatch(key)
match := valuesKeyRegexp.FindStringSubmatch(key)
if len(match) == 3 {
key = match[1]
values = append([]string{match[2]}, values...)
Expand Down
64 changes: 64 additions & 0 deletions runtime/query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,70 @@ import (
"google.golang.org/genproto/protobuf/field_mask"
)

func BenchmarkPopulateQueryParameters(b *testing.B) {
timeT := time.Date(2016, time.December, 15, 12, 23, 32, 49, time.UTC)
timeStr := timeT.Format(time.RFC3339Nano)

durationT := 13 * time.Hour
durationStr := durationT.String()

fieldmaskStr := "float_value,double_value"

msg := &proto3Message{}
values := url.Values{
"float_value": {"1.5"},
"double_value": {"2.5"},
"int64_value": {"-1"},
"int32_value": {"-2"},
"uint64_value": {"3"},
"uint32_value": {"4"},
"bool_value": {"true"},
"string_value": {"str"},
"bytes_value": {"Ynl0ZXM="},
"repeated_value": {"a", "b", "c"},
"enum_value": {"1"},
"repeated_enum": {"1", "2", "0"},
"timestamp_value": {timeStr},
"duration_value": {durationStr},
"fieldmask_value": {fieldmaskStr},
"wrapper_float_value": {"1.5"},
"wrapper_double_value": {"2.5"},
"wrapper_int64_value": {"-1"},
"wrapper_int32_value": {"-2"},
"wrapper_u_int64_value": {"3"},
"wrapper_u_int32_value": {"4"},
"wrapper_bool_value": {"true"},
"wrapper_string_value": {"str"},
"wrapper_bytes_value": {"Ynl0ZXM="},
"map_value[key]": {"value"},
"map_value[second]": {"bar"},
"map_value[third]": {"zzz"},
"map_value[fourth]": {""},
`map_value[~!@#$%^&*()]`: {"value"},
"map_value2[key]": {"-2"},
"map_value3[-2]": {"value"},
"map_value4[key]": {"-1"},
"map_value5[-1]": {"value"},
"map_value6[key]": {"3"},
"map_value7[3]": {"value"},
"map_value8[key]": {"4"},
"map_value9[4]": {"value"},
"map_value10[key]": {"1.5"},
"map_value11[1.5]": {"value"},
"map_value12[key]": {"2.5"},
"map_value13[2.5]": {"value"},
"map_value14[key]": {"true"},
"map_value15[true]": {"value"},
}
filter := utilities.NewDoubleArray([][]string{
{"bool_value"}, {"repeated_value"},
})

for i := 0; i < b.N; i++ {
_ = runtime.PopulateQueryParameters(msg, values, filter)
}
}

func TestPopulateParameters(t *testing.T) {
timeT := time.Date(2016, time.December, 15, 12, 23, 32, 49, time.UTC)
timeStr := timeT.Format(time.RFC3339Nano)
Expand Down

0 comments on commit 7f57073

Please sign in to comment.