Skip to content

[Repo Assist] perf: avoid FSharpValue.GetUnionFields in toParam via cached tag reader#399

Merged
sergey-tihon merged 4 commits intomasterfrom
repo-assist/perf-cache-option-tag-reader-20260424-d7ce09bd9fdc5bbc
Apr 25, 2026
Merged

[Repo Assist] perf: avoid FSharpValue.GetUnionFields in toParam via cached tag reader#399
sergey-tihon merged 4 commits intomasterfrom
repo-assist/perf-cache-option-tag-reader-20260424-d7ce09bd9fdc5bbc

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

🤖 This PR was created by Repo Assist, an automated AI assistant.

Summary

Replaces the FSharpValue.GetUnionFields call in RuntimeHelpers.toParam with a precomputed, cached union tag reader.

Problem

When serializing optional query/form parameters, toParam unwrapped F# option<T> values using FSharpValue.GetUnionFields. On every call this allocates:

  • a UnionCaseInfo object
  • an obj[] of field values

This overhead accumulates for APIs that send many optional parameters, especially in tight loops or high-frequency endpoints.

Fix

  • Add optionTagReaderCache: maps each option<T> type to a precomputed FSharpValue.PreComputeUnionTagReader result. After the first call per type, tag reading is allocation-free.
  • Add optionValueCache: maps each option<T> to its cached Value PropertyInfo. Shared with unwrapFSharpOption, eliminating the now-redundant optionValuePropCache.
  • Check tag = 1 (Some) then access .Value via the cached property.

Before / After

Before After (2nd+ call)
Tag check GetUnionFields → allocates UnionCaseInfo + obj[] Precomputed reader, no allocation
Value access values.[0] from the obj[] Cached PropertyInfo.GetValue

Test Status

✅ All 327 unit tests pass (dotnet tests/SwaggerProvider.Tests/bin/Release/net10.0/SwaggerProvider.Tests.dll)
✅ Fantomas format check passes

Generated by 🌈 Repo Assist, see workflow run. Learn more.

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@96b9d4c39aa22359c0b38265927eadb31dcf4e2a

Replace the FSharpValue.GetUnionFields call in toParam (used when
unwrapping F# option values for query/form parameters) with a
precomputed union-tag reader.

GetUnionFields allocates a UnionCaseInfo object and an obj[] on every
call. The new approach:
- Caches FSharpValue.PreComputeUnionTagReader per option type (O(1) tag
  check with no allocation after first call)
- Caches the Value PropertyInfo per option type (shared with
  unwrapFSharpOption)
- Removes the now-redundant private optionValuePropCache, consolidating
  both caches in one place

This matters for APIs that send many optional query/form parameters;
the improvement is visible when profiling clients that call high-
frequency endpoints with optional fields.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@sergey-tihon sergey-tihon marked this pull request as ready for review April 25, 2026 05:19
Copilot AI review requested due to automatic review settings April 25, 2026 05:19
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR optimizes runtime parameter serialization by reducing allocations when unwrapping F# option<'T> values in RuntimeHelpers.toParam, replacing FSharpValue.GetUnionFields with a cached, precomputed union tag reader.

Changes:

  • Added a cached PreComputeUnionTagReader per concrete option<'T> type to avoid GetUnionFields allocations on repeated calls.
  • Consolidated/cached reflection for the Value property into a shared optionValueCache used by both toParam and unwrapFSharpOption.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/SwaggerProvider.Runtime/RuntimeHelpers.fs Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/SwaggerProvider.Runtime/RuntimeHelpers.fs Outdated
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@sergey-tihon sergey-tihon merged commit 85499ff into master Apr 25, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants