Skip to content

[Repo Assist] perf: eliminate redundant GetType() calls in toParam#410

Merged
sergey-tihon merged 3 commits intomasterfrom
repo-assist/perf-toParam-single-gettype-20260428-ab744a16dd4b3adb
Apr 28, 2026
Merged

[Repo Assist] perf: eliminate redundant GetType() calls in toParam#410
sergey-tihon merged 3 commits intomasterfrom
repo-assist/perf-toParam-single-gettype-20260428-ab744a16dd4b3adb

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

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

Summary

toParam in RuntimeHelpers.fs previously dispatched through two private helpers (tryFormatDateOnly, tryFormatTimeOnly) that each called value.GetType() internally. For any common scalar type (string, int, Guid, bool, etc.) this caused up to 3 GetType() calls per invocation — one inside tryFormatDateOnly, one inside tryFormatTimeOnly, and a third in the option-unwrapping branch — before finally reaching obj.ToString().

Change

  • Hoist GetType() once at the top of the catch-all branch in toParam, then dispatch using if/elif chains.
  • Remove the three now-dead private helpers (tryFormatViaMethods, tryFormatDateOnly, tryFormatTimeOnly).
  • Add formatDateOrTimeValue — a focused helper that accepts the pre-computed Type, making the single-call contract explicit at the call site.
  • All other behaviour (DateOnly, TimeOnly, Option unwrapping, fallthrough to obj.ToString()) is identical.

Impact

toParam is called for every HTTP parameter — query strings, form fields, headers, multipart parts. For a typical form-encoded API call with 10–20 fields, this eliminates ~20–40 redundant GetType() calls. While each GetType() is fast, the savings are meaningful for high-throughput or batch-heavy callers (e.g. issue #150, the Stripe 3 MB schema hang, where parameter serialization is on the hot path).

Trade-offs

None. The fallback GetMethod("ToString", ...) path in formatDateOrTimeValue is preserved for forward-compatibility, even though DateOnly and TimeOnly both implement IFormattable on all .NET versions where they exist.

Test Status

✅ Build: dotnet build tests/SwaggerProvider.Tests/SwaggerProvider.Tests.fsproj -c Release — 0 errors, 0 failures

✅ Tests: 327 total, 0 failed, 1 skipped (pre-existing skip)

✅ Format: dotnet fantomas --check passed after formatting

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

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@3de4e604a36b5190a1c7dc4719c7341500ba8a95

Previously, toParam dispatched through tryFormatDateOnly and
tryFormatTimeOnly, each of which called value.GetType() internally.
For any non-DateTime/non-DateTimeOffset scalar type (string, int,
Guid, bool, etc.), this resulted in up to 3 GetType() calls before
falling through to obj.ToString().

Restructure toParam to call GetType() once at the top of the
catch-all branch, then dispatch to a shared formatDateOrTimeValue
helper or the option unwrapping path as needed.

The three private helpers (tryFormatViaMethods, tryFormatDateOnly,
tryFormatTimeOnly) are removed; formatDateOrTimeValue replaces them
with an API that accepts the pre-computed Type, making the single
GetType() call obvious at the call site.

Behavior is identical; all 327 unit tests pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@sergey-tihon sergey-tihon marked this pull request as ready for review April 28, 2026 13:50
Copilot AI review requested due to automatic review settings April 28, 2026 13:50
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 RuntimeHelpers.toParam in the runtime library by reducing redundant reflection work when serializing parameters (query/form/header/multipart), keeping DateOnly/TimeOnly formatting and F# option unwrapping behavior intact.

Changes:

  • Hoists GetType() once in toParam’s fallback path and dispatches via if/elif.
  • Replaces tryFormatDateOnly/tryFormatTimeOnly (and their shared helper) with a single formatDateOrTimeValue helper that accepts a precomputed Type.
  • Removes the now-dead private formatting helpers.

💡 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>
@sergey-tihon sergey-tihon merged commit 973686d into master Apr 28, 2026
2 checks passed
@sergey-tihon sergey-tihon deleted the repo-assist/perf-toParam-single-gettype-20260428-ab744a16dd4b3adb branch April 28, 2026 14:02
Copilot AI added a commit that referenced this pull request Apr 28, 2026
Master (#410) restructured toParam to call GetType() once at the top,
while the enum branch (this PR) added an 'elif ty.IsEnum' arm after the
Option<T> check. The two changes are compatible: keep the single-GetType()
structure from master and preserve the enum serializer dispatch from this PR.

Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com>
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.

2 participants