Skip to content

Commit 00d85c0

Browse files
authored
perf: reduce allocations & reflection in generated operation code (#455)
* perf: reduce allocations & reflection in generated operation code Compile-time (OperationCompiler): - Hoist toParam / toQueryParams / TaskExtensions.cast / AsyncExtensions.cast MethodInfo resolution to the OperationCompiler instance (resolved once per provider instead of per generated operation). - Hoist string-list union-case reflection in Provider.OpenApiClient. - Build a paramName -> IOpenApiParameter map once per operation and use it for O(1) lookups in invokeCode (was Seq.tryFind per ShapeVar). - Build the fixed Content-Type/Accept headers list at provider-generation time instead of emitting a quotation that re-evaluates per call. - Make response quotations (object / stream / string / unit) lazy so only the branch actually used for the operation's return type is built. Generated code (runtime): - Emit RuntimeHelpers.createHttpRequestFromQueryLists instead of chained List.append / List.concat for query parameter assembly. - Emit RuntimeHelpers.fillHeadersAndCookies instead of an inline Seq.filter / Seq.map / String.concat cookie-building quotation. - For typed JSON responses, emit a direct generic TaskExtensions.cast<'T> / AsyncExtensions.cast<'T> call via ProvidedTypeBuilder.MakeGenericMethod, avoiding MethodInfo.Invoke on every API call. - Avoid the previous shared-quotation Var hazard that caused FS3033 'An item with the same key has already been added' for operations with multiple path/query parameters. Runtime helpers (RuntimeHelpers): - Add createHttpRequestFromQueryLists: flattens seq<#seq<string*string>> into a ResizeArray, filtering nulls, and delegates to createHttpRequest. - Add fillHeadersAndCookies: calls fillHeaders, then builds the Cookie header imperatively with StringBuilder using the standards-compliant '; ' separator and TryAddWithoutValidation. Tests: - Add regression tests in Schema.OperationCompilationTests that assert generated invokeCode does not reuse the same Quotations.Var instance across path/query parameters. Validated: - dotnet fantomas --check src/ tests/ - Unit tests: 479 total, 0 failed, 1 skipped - Provider integration tests (Swashbuckle server): 127 total, 0 failed - Stripe spec3.json (414 paths, 1422 schemas, 587 operations) compiles successfully; generated code contains 0 reflective casts, 0 List.append and 0 List.concat calls. * test: cover createHttpRequestFromQueryLists and fillHeadersAndCookies Address Copilot review feedback on PR #455 by adding focused RuntimeHelpersTests for the two new helpers: createHttpRequestFromQueryLists: - flattens multiple query lists into the request URI - strips leading slash when all lists are empty - strips leading slash when all inner lists are empty - skips null-valued pairs across multiple lists - produces no query string when all values are null - preserves the HTTP method fillHeadersAndCookies: - emits Cookie header with canonical '; ' separator - single cookie has no separator - skips null cookie values - omits Cookie header when all values are null - omits Cookie header when cookie list is empty - still adds normal headers via fillHeaders - still skips null-value headers
1 parent a208c61 commit 00d85c0

5 files changed

Lines changed: 332 additions & 78 deletions

File tree

src/SwaggerProvider.DesignTime/OperationCompiler.fs

Lines changed: 97 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ namespace SwaggerProvider.Internal.Compilers
33
open System
44
open System.Collections.Generic
55
open System.Net.Http
6+
open System.Reflection
67
open System.Text.Json
78

89
open Microsoft.FSharp.Quotations
@@ -60,6 +61,44 @@ type PayloadType =
6061

6162
/// Object for compiling operations.
6263
type OperationCompiler(schema: OpenApiDocument, defCompiler: DefinitionCompiler, ignoreControllerPrefix, ignoreOperationId, asAsync: bool) =
64+
let toParamMethod =
65+
match <@@ RuntimeHelpers.toParam(null) @@> with
66+
| Call(None, m, _) -> m
67+
| _ -> failwith "Cannot extract toParam MethodInfo"
68+
69+
let toQueryParamsMethod =
70+
match <@@ RuntimeHelpers.toQueryParams "" null Unchecked.defaultof<ProvidedApiClientBase> @@> with
71+
| Call(None, m, _) -> m
72+
| _ -> failwith "Cannot extract toQueryParams MethodInfo"
73+
74+
let resolveCastMethod(ownerType: Type) =
75+
ownerType.GetMethods(BindingFlags.Public ||| BindingFlags.Static)
76+
|> Array.tryFind(fun m ->
77+
m.Name = "cast"
78+
&& m.IsGenericMethodDefinition
79+
&& m.GetGenericArguments().Length = 1
80+
&& m.GetParameters().Length = 1)
81+
|> Option.defaultWith(fun () -> failwithf "Cannot extract %s.cast<'T> MethodInfo" ownerType.FullName)
82+
83+
let taskCastMethod = resolveCastMethod typeof<TaskExtensions>
84+
let asyncCastMethod = resolveCastMethod typeof<AsyncExtensions>
85+
86+
let stringPairListExpr(items: (string * string) list) : Expr<(string * string) list> =
87+
let empty = <@ [] @>
88+
89+
(empty, List.rev items)
90+
||> List.fold(fun acc (name, value) ->
91+
let nameExpr = Expr.Value(name, typeof<string>) |> Expr.Cast<string>
92+
let valueExpr = Expr.Value(value, typeof<string>) |> Expr.Cast<string>
93+
94+
<@ (%nameExpr, %valueExpr) :: %acc @>)
95+
96+
let typedListExpr(items: Expr<'T> list) : Expr<'T list> =
97+
let empty = <@ [] @>
98+
99+
(empty, List.rev items)
100+
||> List.fold(fun acc item -> <@ %item :: %acc @>)
101+
63102
let compileOperation (providedMethodName: string) (apiCall: ApiCall) =
64103
let path, pathItem, opTy = apiCall
65104
let operation = pathItem.Operations[opTy]
@@ -96,7 +135,7 @@ type OperationCompiler(schema: OpenApiDocument, defCompiler: DefinitionCompiler,
96135
let (|NoMediaType|_|)(content: IDictionary<string, OpenApiMediaType>) =
97136
if isNull content || content.Count = 0 then Some() else None
98137

99-
let payloadTy, payloadMime, parameters, ctArgIndex =
138+
let payloadTy, payloadMime, parameters, ctArgIndex, apiParamByProvidedName =
100139
/// handles de-duplicating Swagger parameter names if the same parameter name
101140
/// appears in multiple locations in a given operation definition.
102141
let uniqueParamName usedNames (param: IOpenApiParameter) =
@@ -156,8 +195,8 @@ type OperationCompiler(schema: OpenApiDocument, defCompiler: DefinitionCompiler,
156195
|> List.partition(_.Required)
157196

158197
let buildProvidedParameters usedNames (paramList: IOpenApiParameter list) =
159-
((usedNames, []), paramList)
160-
||> List.fold(fun (names, parameters) current ->
198+
((usedNames, [], []), paramList)
199+
||> List.fold(fun (names, parameters, lookup) current ->
161200
let names, paramName = uniqueParamName names current
162201

163202
let paramType =
@@ -170,15 +209,20 @@ type OperationCompiler(schema: OpenApiDocument, defCompiler: DefinitionCompiler,
170209
let paramDefaultValue = defCompiler.GetDefaultValue paramType
171210
ProvidedParameter(paramName, paramType, false, paramDefaultValue)
172211

173-
(names, providedParam :: parameters))
174-
|> fun (finalNames, ps) -> finalNames, List.rev ps
212+
(names, providedParam :: parameters, (paramName, current) :: lookup))
213+
|> fun (finalNames, ps, lookup) -> finalNames, List.rev ps, List.rev lookup
175214

176-
let namesAfterRequired, requiredProvidedParams =
215+
let namesAfterRequired, requiredProvidedParams, requiredLookup =
177216
buildProvidedParameters Set.empty requiredOpenApiParams
178217

179-
let _, optionalProvidedParams =
218+
let _, optionalProvidedParams, optionalLookup =
180219
buildProvidedParameters namesAfterRequired optionalOpenApiParams
181220

221+
let apiParamByProvidedName =
222+
requiredLookup @ optionalLookup
223+
|> List.choose(fun (paramName, param) -> if param.In.HasValue then Some(paramName, param) else None)
224+
|> Map.ofList
225+
182226
let ctArgIndex, parameters =
183227
let scope = UniqueNameGenerator()
184228

@@ -196,7 +240,7 @@ type OperationCompiler(schema: OpenApiDocument, defCompiler: DefinitionCompiler,
196240

197241
ctArgIndex, requiredProvidedParams @ optionalProvidedParams @ [ ctParam ]
198242

199-
payloadTy, payloadTy.ToMediaType(), parameters, ctArgIndex
243+
payloadTy, payloadTy.ToMediaType(), parameters, ctArgIndex, apiParamByProvidedName
200244

201245
// find the inner type value
202246
let okResponse =
@@ -258,6 +302,12 @@ type OperationCompiler(schema: OpenApiDocument, defCompiler: DefinitionCompiler,
258302
|> Seq.toArray
259303
|> Array.unzip
260304

305+
let fixedHeaders =
306+
[ if not(isNull payloadMime) then
307+
"Content-Type", payloadMime
308+
if not(isNull retMime) then
309+
"Accept", retMime ]
310+
261311
let m =
262312
ProvidedMethod(
263313
providedMethodName,
@@ -271,13 +321,7 @@ type OperationCompiler(schema: OpenApiDocument, defCompiler: DefinitionCompiler,
271321

272322
let httpMethod = opTy.ToString()
273323

274-
let headers =
275-
<@
276-
[ if not(isNull payloadMime) then
277-
"Content-Type", payloadMime
278-
if not(isNull retMime) then
279-
"Accept", retMime ]
280-
@>
324+
let headers = stringPairListExpr fixedHeaders
281325

282326
// Locates parameters matching the arguments
283327
let mutable payloadExp = None
@@ -298,14 +342,7 @@ type OperationCompiler(schema: OpenApiDocument, defCompiler: DefinitionCompiler,
298342
apiArgs
299343
|> List.choose (function
300344
| ShapeVar sVar as expr ->
301-
let param =
302-
openApiParameters
303-
|> Seq.tryFind(fun x ->
304-
// pain point: we have to make sure that the set of names we search for here are the same as the set of names generated when we make `parameters` above
305-
let baseName = niceCamelName x.Name
306-
baseName = sVar.Name || (unambiguousName x) = sVar.Name)
307-
308-
match param with
345+
match apiParamByProvidedName |> Map.tryFind sVar.Name with
309346
| Some(par) -> Some(par, expr)
310347
| _ ->
311348
let payloadType = PayloadType.Parse sVar.Name
@@ -324,31 +361,21 @@ type OperationCompiler(schema: OpenApiDocument, defCompiler: DefinitionCompiler,
324361
// object across all calls, causing "duplicate key" exceptions in ProvidedTypes
325362
// when the same helper is called for multiple parameters in one operation.
326363
// Instead, build the call expression directly without an intermediate binding.
327-
let toParamMethod =
328-
match <@@ RuntimeHelpers.toParam(null) @@> with
329-
| Call(None, m, _) -> m
330-
| _ -> failwith "Cannot extract toParam MethodInfo"
331-
332364
let coerceString exp =
333365
let obj = Expr.Coerce(exp, typeof<obj>)
334366
Expr.Call(toParamMethod, [ obj ]) |> Expr.Cast<string>
335367

336-
let toQueryParamsMethod =
337-
match <@@ RuntimeHelpers.toQueryParams "" null (%this) @@> with
338-
| Call(None, m, _) -> m
339-
| _ -> failwith "Cannot extract toQueryParams MethodInfo"
340-
341368
let rec coerceQueryString name expr =
342369
let obj = Expr.Coerce(expr, typeof<obj>)
343370

344371
Expr.Call(toQueryParamsMethod, [ Expr.Value name; obj; this ])
345372
|> Expr.Cast<(string * string) list>
346373

347374
// Partitions arguments based on their locations
348-
let path, queryParams, headers =
349-
let path, queryParams, headers, cookies =
350-
((<@ path @>, <@ [] @>, headers, <@ [] @>), parameters)
351-
||> List.fold(fun (path, query, headers, cookies) (param: IOpenApiParameter, valueExpr) ->
375+
let path, queryParamLists, headers, cookies =
376+
let path, queryParamLists, headers, cookies =
377+
((<@ path @>, [], headers, <@ [] @>), parameters)
378+
||> List.fold(fun (path, queryParamLists, headers, cookies) (param: IOpenApiParameter, valueExpr) ->
352379
if param.In.HasValue then
353380
let name = param.Name
354381

@@ -357,44 +384,32 @@ type OperationCompiler(schema: OpenApiDocument, defCompiler: DefinitionCompiler,
357384
let value = coerceString valueExpr
358385
let pattern = $"{{%s{name}}}"
359386
let path' = <@ (%path).Replace(pattern, %value) @>
360-
(path', query, headers, cookies)
387+
(path', queryParamLists, headers, cookies)
361388
| ParameterLocation.Query ->
362389
let listValues = coerceQueryString name valueExpr
363-
let query' = <@ List.append %query %listValues @>
364-
(path, query', headers, cookies)
390+
(path, listValues :: queryParamLists, headers, cookies)
365391
| ParameterLocation.Header ->
366392
let value = coerceString valueExpr
367393
let headers' = <@ (name, %value) :: (%headers) @>
368-
(path, query, headers', cookies)
394+
(path, queryParamLists, headers', cookies)
369395
| ParameterLocation.Cookie ->
370396
let value = coerceString valueExpr
371397
let cookies' = <@ (name, %value) :: (%cookies) @>
372-
(path, query, headers, cookies')
398+
(path, queryParamLists, headers, cookies')
373399
| x -> failwithf $"Unsupported parameter location '%A{x}'"
374400
else
375401
failwithf "This should not happen, payload expression is already parsed")
376402

377-
let headers' =
378-
<@
379-
let cookieHeader =
380-
%cookies
381-
|> Seq.filter(snd >> isNull >> not)
382-
|> Seq.map(fun (name, value) -> $"{name}={value}")
383-
|> String.concat ";"
384-
385-
if String.IsNullOrEmpty cookieHeader then
386-
%headers
387-
else
388-
("Cookie", cookieHeader) :: (%headers)
389-
@>
390-
391-
(path, queryParams, headers')
403+
path, List.rev queryParamLists, headers, cookies
392404

405+
let queryParamLists = typedListExpr queryParamLists
393406

394407
let httpRequestMessage =
395408
<@
396-
let msg = RuntimeHelpers.createHttpRequest httpMethod %path %queryParams
397-
RuntimeHelpers.fillHeaders msg %headers
409+
let msg =
410+
RuntimeHelpers.createHttpRequestFromQueryLists httpMethod %path %queryParamLists
411+
412+
RuntimeHelpers.fillHeadersAndCookies msg %headers %cookies
398413
msg
399414
@>
400415

@@ -441,7 +456,7 @@ type OperationCompiler(schema: OpenApiDocument, defCompiler: DefinitionCompiler,
441456
let action =
442457
<@ (%this).CallAsync(%httpRequestMessageWithPayload, errorCodes, errorDescriptions, %ct) @>
443458

444-
let responseObj =
459+
let responseObj() =
445460
let innerReturnType = defaultArg retTy null
446461

447462
<@
@@ -455,7 +470,7 @@ type OperationCompiler(schema: OpenApiDocument, defCompiler: DefinitionCompiler,
455470
}
456471
@>
457472

458-
let responseStream =
473+
let responseStream() =
459474
<@
460475
let x = %action
461476
let ct = %ct
@@ -467,7 +482,7 @@ type OperationCompiler(schema: OpenApiDocument, defCompiler: DefinitionCompiler,
467482
}
468483
@>
469484

470-
let responseString =
485+
let responseString() =
471486
<@
472487
let x = %action
473488
let ct = %ct
@@ -479,7 +494,7 @@ type OperationCompiler(schema: OpenApiDocument, defCompiler: DefinitionCompiler,
479494
}
480495
@>
481496

482-
let responseUnit =
497+
let responseUnit() =
483498
<@
484499
let x = %action
485500

@@ -489,27 +504,36 @@ type OperationCompiler(schema: OpenApiDocument, defCompiler: DefinitionCompiler,
489504
}
490505
@>
491506

492-
// if we're an async method, then we can just return the above, coerced to the overallReturnType.
493-
// if we're not async, then run that^ through Async.RunSynchronously before doing the coercion.
507+
// Build only the response quotation needed for this operation's return shape.
508+
// For typed JSON responses, emit direct generic cast calls so generated clients
509+
// do not pay MethodInfo.Invoke costs on every API call.
494510
if not asAsync then
495511
match retTy with
496-
| None -> responseUnit.Raw
497-
| Some t when t = typeof<IO.Stream> -> <@ %responseStream @>.Raw
512+
| None -> (responseUnit()).Raw
513+
| Some t when t = typeof<IO.Stream> -> <@ %(responseStream()) @>.Raw
498514
| Some t ->
499515
match retMime with
500-
| TextReturn _ -> <@ %responseString @>.Raw
501-
| _ -> Expr.Coerce(<@ RuntimeHelpers.taskCast t %responseObj @>, overallReturnType)
516+
| TextReturn _ -> <@ %(responseString()) @>.Raw
517+
| _ ->
518+
let castMethod = ProvidedTypeBuilder.MakeGenericMethod(taskCastMethod, [ t ])
519+
520+
Expr.Call(castMethod, [ responseObj() ])
521+
|> fun e -> Expr.Coerce(e, overallReturnType)
502522
else
503523
let awaitTask t =
504524
<@ Async.AwaitTask(%t) @>
505525

506526
match retTy with
507-
| None -> (awaitTask responseUnit).Raw
508-
| Some t when t = typeof<IO.Stream> -> <@ %(awaitTask responseStream) @>.Raw
527+
| None -> (awaitTask(responseUnit())).Raw
528+
| Some t when t = typeof<IO.Stream> -> <@ %(awaitTask(responseStream())) @>.Raw
509529
| Some t ->
510530
match retMime with
511-
| TextReturn _ -> <@ %(awaitTask responseString) @>.Raw
512-
| _ -> Expr.Coerce(<@ RuntimeHelpers.asyncCast t %(awaitTask responseObj) @>, overallReturnType)
531+
| TextReturn _ -> <@ %(awaitTask(responseString())) @>.Raw
532+
| _ ->
533+
let castMethod = ProvidedTypeBuilder.MakeGenericMethod(asyncCastMethod, [ t ])
534+
535+
Expr.Call(castMethod, [ awaitTask(responseObj()) ])
536+
|> fun e -> Expr.Coerce(e, overallReturnType)
513537
)
514538

515539
let xmlDoc =

src/SwaggerProvider.DesignTime/Provider.OpenApiClient.fs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,14 @@ type public OpenApiClientTypeProvider(cfg: TypeProviderConfig) as this =
3030
// check we contain a copy of runtime files, and are not referencing the runtime DLL
3131
do assert (typeof<ProvidedApiClientBase>.Assembly.GetName().Name = asm.GetName().Name)
3232

33-
let buildStringListExpr(items: string list) : Expr =
33+
let stringListNilCase, stringListConsCase =
3434
let cases = FSharpType.GetUnionCases typeof<string list>
35-
let nilCase = cases |> Array.find(fun c -> c.Name = "Empty")
36-
let consCase = cases |> Array.find(fun c -> c.Name = "Cons")
37-
let nil = Expr.NewUnionCase(nilCase, [])
35+
cases |> Array.find(fun c -> c.Name = "Empty"), cases |> Array.find(fun c -> c.Name = "Cons")
36+
37+
let buildStringListExpr(items: string list) : Expr =
38+
let nil = Expr.NewUnionCase(stringListNilCase, [])
3839

39-
List.foldBack (fun (s: string) acc -> Expr.NewUnionCase(consCase, [ Expr.Value(s, typeof<string>); acc ])) items nil
40+
List.foldBack (fun (s: string) acc -> Expr.NewUnionCase(stringListConsCase, [ Expr.Value(s, typeof<string>); acc ])) items nil
4041

4142
let myParamType =
4243
let t =

0 commit comments

Comments
 (0)