utilities.IOReaderFactory is inefficient

[leungster]

🐛 Bug Report

utilities.IOReaderFactory relies on io.ReadAll which is documented to be inefficient for large inputs.
For our use case, we had a 200MB JSON body that resulted in > 5GB transient memory allocations.

It looks like in most cases, the req.Body could be passed directly into the marshaler. The exception to that is when field_masks are used then the body is parsed twice.

To Reproduce

Create a proto definition with repeated fields and send a relatively large input payload to monitor memory utilization.

Expected behavior

Memory utilization doesn't grow several times the input size.

Actual Behavior

Our 200MB input turned into more than 5GB memory allocations that the GC had to reclaim. This transient explosion in memory usage causes our service to run out of memory.

[johanbrandhorst]

Hi, thanks for your issue report. I'm sorry to hear that you had a bad experience. I think there is something we can do here. We introduced the IOReaderFactory to support patch requests, but we don't really need it if the user doesn't enable the patch feature. Here is the relevant logic:

grpc-gateway/protoc-gen-grpc-gateway/internal/gengateway/template.go

Lines 336 to 354 in 6dff994

	newReader, berr := utilities.IOReaderFactory(req.Body)
	if berr != nil {
	return nil, metadata, status.Errorf(codes.InvalidArgument, "%v", berr)
	}
	{{- $protoReq := .Body.AssignableExprPrep "protoReq" .Method.Service.File.GoPkg.Path -}}
	{{- if ne "" $protoReq }}
	{{printf "%s" $protoReq }}
	{{- end}}
	if err := marshaler.NewDecoder(newReader()).Decode(&{{.Body.AssignableExpr "protoReq" .Method.Service.File.GoPkg.Path}}); err != nil && err != io.EOF {
	return nil, metadata, status.Errorf(codes.InvalidArgument, "%v", err)
	}
	{{- if and $AllowPatchFeature (eq (.HTTPMethod) "PATCH") (.FieldMaskField) (not (eq "*" .GetBodyFieldPath)) }}
	if protoReq.{{.FieldMaskField}} == nil || len(protoReq.{{.FieldMaskField}}.GetPaths()) == 0 {
	if fieldMask, err := runtime.FieldMaskFromRequestBody(newReader(), protoReq.{{.GetBodyFieldStructName}}); err != nil {
	return nil, metadata, status.Errorf(codes.InvalidArgument, "%v", err)
	} else {
	protoReq.{{.FieldMaskField}} = fieldMask
	}
	}

. I think we could rewrite this so that we only created a reader factory if $AllowPatchFeature is true, and in other cases we can use req.Body directly. Would you be willing to make such a contribution?

[leungster]

I went ahead and made the change #3727