Skip to content

Commit 8fede0b

Browse files
Make RequestScope be a pointer consistently for better memory use
RequestScope is a large struct and causes stack growth when we pass it by value into multiple stack levels. Avoid the allocations for this read only struct by passing a pointer.
1 parent 1514bb2 commit 8fede0b

File tree

12 files changed

+82
-87
lines changed

12 files changed

+82
-87
lines changed

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -113,13 +113,13 @@ type crdInfo struct {
113113
storages map[string]customresource.CustomResourceStorage
114114

115115
// Request scope per version
116-
requestScopes map[string]handlers.RequestScope
116+
requestScopes map[string]*handlers.RequestScope
117117

118118
// Scale scope per version
119-
scaleRequestScopes map[string]handlers.RequestScope
119+
scaleRequestScopes map[string]*handlers.RequestScope
120120

121121
// Status scope per version
122-
statusRequestScopes map[string]handlers.RequestScope
122+
statusRequestScopes map[string]*handlers.RequestScope
123123

124124
// storageVersion is the CRD version used when storing the object in etcd.
125125
storageVersion string
@@ -444,10 +444,10 @@ func (r *crdHandler) getOrCreateServingInfoFor(crd *apiextensions.CustomResource
444444
}
445445

446446
// Scope/Storages per version.
447-
requestScopes := map[string]handlers.RequestScope{}
447+
requestScopes := map[string]*handlers.RequestScope{}
448448
storages := map[string]customresource.CustomResourceStorage{}
449-
statusScopes := map[string]handlers.RequestScope{}
450-
scaleScopes := map[string]handlers.RequestScope{}
449+
statusScopes := map[string]*handlers.RequestScope{}
450+
scaleScopes := map[string]*handlers.RequestScope{}
451451

452452
for _, v := range crd.Spec.Versions {
453453
safeConverter, unsafeConverter, err := r.converterFactory.NewConverter(crd)
@@ -548,7 +548,7 @@ func (r *crdHandler) getOrCreateServingInfoFor(crd *apiextensions.CustomResource
548548

549549
clusterScoped := crd.Spec.Scope == apiextensions.ClusterScoped
550550

551-
requestScopes[v.Name] = handlers.RequestScope{
551+
requestScopes[v.Name] = &handlers.RequestScope{
552552
Namer: handlers.ContextBasedNaming{
553553
SelfLinker: meta.NewAccessor(),
554554
ClusterScoped: clusterScoped,
@@ -576,19 +576,19 @@ func (r *crdHandler) getOrCreateServingInfoFor(crd *apiextensions.CustomResource
576576
Authorizer: r.authorizer,
577577
}
578578
if utilfeature.DefaultFeatureGate.Enabled(features.ServerSideApply) {
579-
reqScope := requestScopes[v.Name]
579+
reqScope := *requestScopes[v.Name]
580580
reqScope.FieldManager = fieldmanager.NewCRDFieldManager(
581581
reqScope.Convertor,
582582
reqScope.Defaulter,
583583
reqScope.Kind.GroupVersion(),
584584
reqScope.HubGroupVersion,
585585
)
586-
requestScopes[v.Name] = reqScope
586+
requestScopes[v.Name] = &reqScope
587587
}
588588

589589
// override scaleSpec subresource values
590590
// shallow copy
591-
scaleScope := requestScopes[v.Name]
591+
scaleScope := *requestScopes[v.Name]
592592
scaleConverter := scale.NewScaleConverter()
593593
scaleScope.Subresource = "scale"
594594
scaleScope.Serializer = serializer.NewCodecFactory(scaleConverter.Scheme())
@@ -599,19 +599,19 @@ func (r *crdHandler) getOrCreateServingInfoFor(crd *apiextensions.CustomResource
599599
SelfLinkPathPrefix: selfLinkPrefix,
600600
SelfLinkPathSuffix: "/scale",
601601
}
602-
scaleScopes[v.Name] = scaleScope
602+
scaleScopes[v.Name] = &scaleScope
603603

604604
// override status subresource values
605605
// shallow copy
606-
statusScope := requestScopes[v.Name]
606+
statusScope := *requestScopes[v.Name]
607607
statusScope.Subresource = "status"
608608
statusScope.Namer = handlers.ContextBasedNaming{
609609
SelfLinker: meta.NewAccessor(),
610610
ClusterScoped: clusterScoped,
611611
SelfLinkPathPrefix: selfLinkPrefix,
612612
SelfLinkPathSuffix: "/status",
613613
}
614-
statusScopes[v.Name] = statusScope
614+
statusScopes[v.Name] = &statusScope
615615
}
616616

617617
ret := &crdInfo{

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ import (
4343
utiltrace "k8s.io/utils/trace"
4444
)
4545

46-
func createHandler(r rest.NamedCreater, scope RequestScope, admit admission.Interface, includeName bool) http.HandlerFunc {
46+
func createHandler(r rest.NamedCreater, scope *RequestScope, admit admission.Interface, includeName bool) http.HandlerFunc {
4747
return func(w http.ResponseWriter, req *http.Request) {
4848
// For performance tracking purposes.
4949
trace := utiltrace.New("Create " + req.URL.Path)
@@ -73,7 +73,7 @@ func createHandler(r rest.NamedCreater, scope RequestScope, admit admission.Inte
7373

7474
ctx := req.Context()
7575
ctx = request.WithNamespace(ctx, namespace)
76-
outputMediaType, _, err := negotiation.NegotiateOutputMediaType(req, scope.Serializer, &scope)
76+
outputMediaType, _, err := negotiation.NegotiateOutputMediaType(req, scope.Serializer, scope)
7777
if err != nil {
7878
scope.err(err, w, req)
7979
return
@@ -130,7 +130,7 @@ func createHandler(r rest.NamedCreater, scope RequestScope, admit admission.Inte
130130
userInfo, _ := request.UserFrom(ctx)
131131
admissionAttributes := admission.NewAttributesRecord(obj, nil, scope.Kind, namespace, name, scope.Resource, scope.Subresource, admission.Create, dryrun.IsDryRun(options.DryRun), userInfo)
132132
if mutatingAdmission, ok := admit.(admission.MutationInterface); ok && mutatingAdmission.Handles(admission.Create) {
133-
err = mutatingAdmission.Admit(admissionAttributes, &scope)
133+
err = mutatingAdmission.Admit(admissionAttributes, scope)
134134
if err != nil {
135135
scope.err(err, w, req)
136136
return
@@ -157,7 +157,7 @@ func createHandler(r rest.NamedCreater, scope RequestScope, admit admission.Inte
157157
ctx,
158158
name,
159159
obj,
160-
rest.AdmissionToValidateObjectFunc(admit, admissionAttributes, &scope),
160+
rest.AdmissionToValidateObjectFunc(admit, admissionAttributes, scope),
161161
options,
162162
)
163163
})
@@ -173,18 +173,17 @@ func createHandler(r rest.NamedCreater, scope RequestScope, admit admission.Inte
173173
status.Code = int32(code)
174174
}
175175

176-
scope.Trace = trace
177-
transformResponseObject(ctx, scope, req, w, code, outputMediaType, result)
176+
transformResponseObject(ctx, scope, trace, req, w, code, outputMediaType, result)
178177
}
179178
}
180179

181180
// CreateNamedResource returns a function that will handle a resource creation with name.
182-
func CreateNamedResource(r rest.NamedCreater, scope RequestScope, admission admission.Interface) http.HandlerFunc {
181+
func CreateNamedResource(r rest.NamedCreater, scope *RequestScope, admission admission.Interface) http.HandlerFunc {
183182
return createHandler(r, scope, admission, true)
184183
}
185184

186185
// CreateResource returns a function that will handle a resource creation.
187-
func CreateResource(r rest.Creater, scope RequestScope, admission admission.Interface) http.HandlerFunc {
186+
func CreateResource(r rest.Creater, scope *RequestScope, admission admission.Interface) http.HandlerFunc {
188187
return createHandler(&namedCreaterAdapter{r}, scope, admission, false)
189188
}
190189

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete.go

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ import (
4040

4141
// DeleteResource returns a function that will handle a resource deletion
4242
// TODO admission here becomes solely validating admission
43-
func DeleteResource(r rest.GracefulDeleter, allowsOptions bool, scope RequestScope, admit admission.Interface) http.HandlerFunc {
43+
func DeleteResource(r rest.GracefulDeleter, allowsOptions bool, scope *RequestScope, admit admission.Interface) http.HandlerFunc {
4444
return func(w http.ResponseWriter, req *http.Request) {
4545
// For performance tracking purposes.
4646
trace := utiltrace.New("Delete " + req.URL.Path)
@@ -64,7 +64,7 @@ func DeleteResource(r rest.GracefulDeleter, allowsOptions bool, scope RequestSco
6464
ae := request.AuditEventFrom(ctx)
6565
admit = admission.WithAudit(admit, ae)
6666

67-
outputMediaType, _, err := negotiation.NegotiateOutputMediaType(req, scope.Serializer, &scope)
67+
outputMediaType, _, err := negotiation.NegotiateOutputMediaType(req, scope.Serializer, scope)
6868
if err != nil {
6969
scope.err(err, w, req)
7070
return
@@ -119,13 +119,13 @@ func DeleteResource(r rest.GracefulDeleter, allowsOptions bool, scope RequestSco
119119
userInfo, _ := request.UserFrom(ctx)
120120
attrs := admission.NewAttributesRecord(nil, nil, scope.Kind, namespace, name, scope.Resource, scope.Subresource, admission.Delete, dryrun.IsDryRun(options.DryRun), userInfo)
121121
if mutatingAdmission, ok := admit.(admission.MutationInterface); ok {
122-
if err := mutatingAdmission.Admit(attrs, &scope); err != nil {
122+
if err := mutatingAdmission.Admit(attrs, scope); err != nil {
123123
scope.err(err, w, req)
124124
return
125125
}
126126
}
127127
if validatingAdmission, ok := admit.(admission.ValidationInterface); ok {
128-
if err := validatingAdmission.Validate(attrs, &scope); err != nil {
128+
if err := validatingAdmission.Validate(attrs, scope); err != nil {
129129
scope.err(err, w, req)
130130
return
131131
}
@@ -168,13 +168,12 @@ func DeleteResource(r rest.GracefulDeleter, allowsOptions bool, scope RequestSco
168168
}
169169
}
170170

171-
scope.Trace = trace
172-
transformResponseObject(ctx, scope, req, w, status, outputMediaType, result)
171+
transformResponseObject(ctx, scope, trace, req, w, status, outputMediaType, result)
173172
}
174173
}
175174

176175
// DeleteCollection returns a function that will handle a collection deletion
177-
func DeleteCollection(r rest.CollectionDeleter, checkBody bool, scope RequestScope, admit admission.Interface) http.HandlerFunc {
176+
func DeleteCollection(r rest.CollectionDeleter, checkBody bool, scope *RequestScope, admit admission.Interface) http.HandlerFunc {
178177
return func(w http.ResponseWriter, req *http.Request) {
179178
trace := utiltrace.New("Delete " + req.URL.Path)
180179
defer trace.LogIfLong(500 * time.Millisecond)
@@ -197,7 +196,7 @@ func DeleteCollection(r rest.CollectionDeleter, checkBody bool, scope RequestSco
197196
ctx = request.WithNamespace(ctx, namespace)
198197
ae := request.AuditEventFrom(ctx)
199198

200-
outputMediaType, _, err := negotiation.NegotiateOutputMediaType(req, scope.Serializer, &scope)
199+
outputMediaType, _, err := negotiation.NegotiateOutputMediaType(req, scope.Serializer, scope)
201200
if err != nil {
202201
scope.err(err, w, req)
203202
return
@@ -269,15 +268,15 @@ func DeleteCollection(r rest.CollectionDeleter, checkBody bool, scope RequestSco
269268
userInfo, _ := request.UserFrom(ctx)
270269
attrs := admission.NewAttributesRecord(nil, nil, scope.Kind, namespace, "", scope.Resource, scope.Subresource, admission.Delete, dryrun.IsDryRun(options.DryRun), userInfo)
271270
if mutatingAdmission, ok := admit.(admission.MutationInterface); ok {
272-
err = mutatingAdmission.Admit(attrs, &scope)
271+
err = mutatingAdmission.Admit(attrs, scope)
273272
if err != nil {
274273
scope.err(err, w, req)
275274
return
276275
}
277276
}
278277

279278
if validatingAdmission, ok := admit.(admission.ValidationInterface); ok {
280-
err = validatingAdmission.Validate(attrs, &scope)
279+
err = validatingAdmission.Validate(attrs, scope)
281280
if err != nil {
282281
scope.err(err, w, req)
283282
return
@@ -305,7 +304,6 @@ func DeleteCollection(r rest.CollectionDeleter, checkBody bool, scope RequestSco
305304
}
306305
}
307306

308-
scope.Trace = trace
309-
transformResponseObject(ctx, scope, req, w, http.StatusOK, outputMediaType, result)
307+
transformResponseObject(ctx, scope, trace, req, w, http.StatusOK, outputMediaType, result)
310308
}
311309
}

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/get.go

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ type getterFunc func(ctx context.Context, name string, req *http.Request, trace
4646

4747
// getResourceHandler is an HTTP handler function for get requests. It delegates to the
4848
// passed-in getterFunc to perform the actual get.
49-
func getResourceHandler(scope RequestScope, getter getterFunc) http.HandlerFunc {
49+
func getResourceHandler(scope *RequestScope, getter getterFunc) http.HandlerFunc {
5050
return func(w http.ResponseWriter, req *http.Request) {
5151
trace := utiltrace.New("Get " + req.URL.Path)
5252
defer trace.LogIfLong(500 * time.Millisecond)
@@ -59,7 +59,7 @@ func getResourceHandler(scope RequestScope, getter getterFunc) http.HandlerFunc
5959
ctx := req.Context()
6060
ctx = request.WithNamespace(ctx, namespace)
6161

62-
outputMediaType, _, err := negotiation.NegotiateOutputMediaType(req, scope.Serializer, &scope)
62+
outputMediaType, _, err := negotiation.NegotiateOutputMediaType(req, scope.Serializer, scope)
6363
if err != nil {
6464
scope.err(err, w, req)
6565
return
@@ -72,14 +72,13 @@ func getResourceHandler(scope RequestScope, getter getterFunc) http.HandlerFunc
7272
}
7373

7474
trace.Step("About to write a response")
75-
scope.Trace = trace
76-
transformResponseObject(ctx, scope, req, w, http.StatusOK, outputMediaType, result)
75+
transformResponseObject(ctx, scope, trace, req, w, http.StatusOK, outputMediaType, result)
7776
trace.Step("Transformed response object")
7877
}
7978
}
8079

8180
// GetResource returns a function that handles retrieving a single resource from a rest.Storage object.
82-
func GetResource(r rest.Getter, e rest.Exporter, scope RequestScope) http.HandlerFunc {
81+
func GetResource(r rest.Getter, e rest.Exporter, scope *RequestScope) http.HandlerFunc {
8382
return getResourceHandler(scope,
8483
func(ctx context.Context, name string, req *http.Request, trace *utiltrace.Trace) (runtime.Object, error) {
8584
// check for export
@@ -109,7 +108,7 @@ func GetResource(r rest.Getter, e rest.Exporter, scope RequestScope) http.Handle
109108
}
110109

111110
// GetResourceWithOptions returns a function that handles retrieving a single resource from a rest.Storage object.
112-
func GetResourceWithOptions(r rest.GetterWithOptions, scope RequestScope, isSubresource bool) http.HandlerFunc {
111+
func GetResourceWithOptions(r rest.GetterWithOptions, scope *RequestScope, isSubresource bool) http.HandlerFunc {
113112
return getResourceHandler(scope,
114113
func(ctx context.Context, name string, req *http.Request, trace *utiltrace.Trace) (runtime.Object, error) {
115114
opts, subpath, subpathKey := r.NewGetOptions()
@@ -126,7 +125,7 @@ func GetResourceWithOptions(r rest.GetterWithOptions, scope RequestScope, isSubr
126125
}
127126

128127
// getRequestOptions parses out options and can include path information. The path information shouldn't include the subresource.
129-
func getRequestOptions(req *http.Request, scope RequestScope, into runtime.Object, subpath bool, subpathKey string, isSubresource bool) error {
128+
func getRequestOptions(req *http.Request, scope *RequestScope, into runtime.Object, subpath bool, subpathKey string, isSubresource bool) error {
130129
if into == nil {
131130
return nil
132131
}
@@ -163,7 +162,7 @@ func getRequestOptions(req *http.Request, scope RequestScope, into runtime.Objec
163162
return scope.ParameterCodec.DecodeParameters(query, scope.Kind.GroupVersion(), into)
164163
}
165164

166-
func ListResource(r rest.Lister, rw rest.Watcher, scope RequestScope, forceWatch bool, minRequestTimeout time.Duration) http.HandlerFunc {
165+
func ListResource(r rest.Lister, rw rest.Watcher, scope *RequestScope, forceWatch bool, minRequestTimeout time.Duration) http.HandlerFunc {
167166
return func(w http.ResponseWriter, req *http.Request) {
168167
// For performance tracking purposes.
169168
trace := utiltrace.New("List " + req.URL.Path)
@@ -185,7 +184,7 @@ func ListResource(r rest.Lister, rw rest.Watcher, scope RequestScope, forceWatch
185184
ctx := req.Context()
186185
ctx = request.WithNamespace(ctx, namespace)
187186

188-
outputMediaType, _, err := negotiation.NegotiateOutputMediaType(req, scope.Serializer, &scope)
187+
outputMediaType, _, err := negotiation.NegotiateOutputMediaType(req, scope.Serializer, scope)
189188
if err != nil {
190189
scope.err(err, w, req)
191190
return
@@ -272,8 +271,7 @@ func ListResource(r rest.Lister, rw rest.Watcher, scope RequestScope, forceWatch
272271
}
273272
trace.Step("Listing from storage done")
274273

275-
scope.Trace = trace
276-
transformResponseObject(ctx, scope, req, w, http.StatusOK, outputMediaType, result)
274+
transformResponseObject(ctx, scope, trace, req, w, http.StatusOK, outputMediaType, result)
277275
trace.Step(fmt.Sprintf("Writing http response done (%d items)", meta.LenList(result)))
278276
}
279277
}

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ const (
5555
)
5656

5757
// PatchResource returns a function that will handle a resource patch.
58-
func PatchResource(r rest.Patcher, scope RequestScope, admit admission.Interface, patchTypes []string) http.HandlerFunc {
58+
func PatchResource(r rest.Patcher, scope *RequestScope, admit admission.Interface, patchTypes []string) http.HandlerFunc {
5959
return func(w http.ResponseWriter, req *http.Request) {
6060
// For performance tracking purposes.
6161
trace := utiltrace.New("Patch " + req.URL.Path)
@@ -95,7 +95,7 @@ func PatchResource(r rest.Patcher, scope RequestScope, admit admission.Interface
9595
ctx := req.Context()
9696
ctx = request.WithNamespace(ctx, namespace)
9797

98-
outputMediaType, _, err := negotiation.NegotiateOutputMediaType(req, scope.Serializer, &scope)
98+
outputMediaType, _, err := negotiation.NegotiateOutputMediaType(req, scope.Serializer, scope)
9999
if err != nil {
100100
scope.err(err, w, req)
101101
return
@@ -191,12 +191,12 @@ func PatchResource(r rest.Patcher, scope RequestScope, admit admission.Interface
191191
subresource: scope.Subresource,
192192
dryRun: dryrun.IsDryRun(options.DryRun),
193193

194-
objectInterfaces: &scope,
194+
objectInterfaces: scope,
195195

196196
hubGroupVersion: scope.HubGroupVersion,
197197

198-
createValidation: withAuthorization(rest.AdmissionToValidateObjectFunc(admit, staticCreateAttributes, &scope), scope.Authorizer, createAuthorizerAttributes),
199-
updateValidation: rest.AdmissionToValidateObjectUpdateFunc(admit, staticUpdateAttributes, &scope),
198+
createValidation: withAuthorization(rest.AdmissionToValidateObjectFunc(admit, staticCreateAttributes, scope), scope.Authorizer, createAuthorizerAttributes),
199+
updateValidation: rest.AdmissionToValidateObjectUpdateFunc(admit, staticUpdateAttributes, scope),
200200
admissionCheck: mutatingAdmission,
201201

202202
codec: codec,
@@ -235,8 +235,7 @@ func PatchResource(r rest.Patcher, scope RequestScope, admit admission.Interface
235235
if wasCreated {
236236
status = http.StatusCreated
237237
}
238-
scope.Trace = trace
239-
transformResponseObject(ctx, scope, req, w, status, outputMediaType, result)
238+
transformResponseObject(ctx, scope, trace, req, w, status, outputMediaType, result)
240239
}
241240
}
242241

@@ -517,7 +516,7 @@ func (p *patcher) applyAdmission(ctx context.Context, patchedObject runtime.Obje
517516
}
518517

519518
// patchResource divides PatchResource for easier unit testing
520-
func (p *patcher) patchResource(ctx context.Context, scope RequestScope) (runtime.Object, bool, error) {
519+
func (p *patcher) patchResource(ctx context.Context, scope *RequestScope) (runtime.Object, bool, error) {
521520
p.namespace = request.NamespaceValue(ctx)
522521
switch p.patchType {
523522
case types.JSONPatchType, types.MergePatchType:

0 commit comments

Comments
 (0)