Skip to content

[Repo Assist] perf/refactor: unify canBind* helpers; skip Array.filter on bindAll path#509

Merged
dsyme merged 2 commits intomasterfrom
repo-assist/perf-canBind-helpers-20260425-49d046a5ffb3a8de
May 1, 2026
Merged

[Repo Assist] perf/refactor: unify canBind* helpers; skip Array.filter on bindAll path#509
dsyme merged 2 commits intomasterfrom
repo-assist/perf-canBind-helpers-20260425-49d046a5ffb3a8de

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot commented Apr 25, 2026

🤖 This is an automated PR from Repo Assist.

Summary

Two closely-related improvements to ProvidedTypes.fs (Tasks 8 & 5):

Task 5 — Coding improvement: unify canBind* helpers

The five visibility-filter helpers (canBindConstructor, canBindMethod, canBindProperty, canBindField, canBindEvent) had identical bodies:

hasFlag bindingFlags BindingFlags.Public && c.IsPublic
|| hasFlag bindingFlags BindingFlags.NonPublic && not c.IsPublic

These are now expressed via a shared canBindByVisibility inline helper, reducing duplication and making the pattern more explicit.

Task 8 — Performance improvement: skip Array.filter on the bindAll path

In TargetTypeDefinition.GetMethods(bindingFlags) (and GetConstructors, GetFields, GetEvents, GetProperties, GetNestedTypes), every call previously did:

methDefs.Force() |> Array.filter (canBindMethod bindingFlags)

methDefs.Force() is cached (Lazy), but Array.filter allocates a new array on every call. When both Public and NonPublic are in bindingFlags every member passes the filter — the allocation is wasted.

The new isVisibilityBindAll helper detects this case and returns the cached array directly:

let arr = methDefs.Force()
if isVisibilityBindAll bindingFlags then arr else arr |> Array.filter (canBindMethod bindingFlags)
```

The F# compiler uses `bindAll = DeclaredOnly ||| Public ||| NonPublic ||| Static ||| Instance` for all member-enumeration calls, so this eliminates a redundant O(n) allocation on every `Get*` call against every `TargetTypeDefinition` (the target-assembly-backed type representation used by all generative type providers).

The same optimisation is applied to `TypeSymbol.Get*` for `TargetGeneric` instantiations.

## Test Status

```
Passed! - Failed: 0, Passed: 157, Skipped: 0, Total: 157

All 157 tests pass with no changes to test code.

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

@github-actions
perf/refactor: unify canBind* helpers; skip Array.filter on bindAll p…
734c971 
…ath in TargetTypeDefinition and TypeSymbol

- Extract canBindByVisibility inline helper shared by all five canBind* functions
  (canBindConstructor / canBindMethod / canBindProperty / canBindField / canBindEvent),
  removing five nearly-identical bodies.
- Add isVisibilityBindAll helper: returns true when both Public and NonPublic
  BindingFlags are set, meaning every member passes the visibility filter.
- Use isVisibilityBindAll in TargetTypeDefinition.GetConstructors/GetMethods/
  GetFields/GetEvents/GetProperties/GetNestedTypes: return the already-computed
  lazy array directly instead of calling Array.filter and allocating a fresh copy.
  The F# compiler always calls these with bindAll (Public|NonPublic|Static|Instance|
  DeclaredOnly), so this eliminates a redundant O(n) allocation on every member
  enumeration of every TargetTypeDefinition.
- Apply the same short-circuit in TypeSymbol.Get* for TargetGeneric instantiations.

157/157 tests pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This was referenced Apr 27, 2026
Closed
Closed
Open
@dsyme dsyme marked this pull request as ready for review May 1, 2026 16:24
@dsyme dsyme merged commit 88587a5 into master May 1, 2026
2 checks passed
dsyme pushed a commit that referenced this pull request May 1, 2026
@github-actions
[Repo Assist] perf: cache GetParameters/PropertyType/FieldType/EventH…
2505f60 
…andlerType in target-model wrappers (#513)

🤖 *This is an automated PR from Repo Assist.*

## Summary

Caches computed values in the target-model member wrappers
(`txILConstructorDef`, `txILMethodDef`, `txILPropertyDef`,
`txILEventDef`, `txILFieldDef`) inside `TargetTypeDefinition` to avoid
repeated type-resolution and array allocations on every call.

## What was happening

Each wrapper function creates a `new XxxInfo()` object expression.
Before this change, several frequently-called members recomputed their
results from scratch on every access:

| Wrapper | Member | Was doing |
|---------|--------|-----------|
| `txILConstructorDef` | `GetParameters()` | `Array.map (txILParameter
...)` every call |
| `txILMethodDef` | `GetParameters()` | `Array.map (txILParameter ...)`
every call |
| `txILPropertyDef` | `PropertyType` | `txILType` resolution every call
|
| `txILPropertyDef` | `GetIndexParameters()` | `Array.map (txILParameter
...)` every call |
| `txILEventDef` | `EventHandlerType` | `txILType` resolution every call
|
| `txILFieldDef` | `FieldType` | `txILType` resolution every call |

## Why this matters

`TargetTypeDefinition` already caches its member wrapper arrays via
`lazy` (`ctorDefs`, `methDefs`, `fieldDefs`, `propDefs`, `eventDefs`).
This means the **same wrapper objects are reused** across calls. Without
result caching inside the wrappers, every access to `GetParameters()`,
`PropertyType`, etc. allocates a new array and re-runs IL type
resolution — which the F# compiler does repeatedly during type inference
against a generative type provider's generated assembly.

## Fix

Added `lazy` bindings before each object expression to cache the
computed results:

````fsharp
// txILMethodDef (before)
override __.GetParameters() = inp.Parameters |> Array.map (txILParameter (gps, gps2))

// txILMethodDef (after)
let parametersCache = lazy (inp.Parameters |> Array.map (txILParameter (gps, gps2)))
// ...
override __.GetParameters() = parametersCache.Value
```

Same pattern applied to `txILConstructorDef`, `txILPropertyDef`,
`txILEventDef`, and `txILFieldDef`.

Note: This is independent of and complementary to PR #509 (which
optimises `Array.filter (canBindXxx)` on the bind-all path).

## Test Status

```
Passed! - Failed: 0, Passed: 157, Skipped: 0, Total: 157
````
All 157 tests pass.

> Generated by 🌈 Repo Assist, see [workflow
run](https://github.com/fsprojects/FSharp.TypeProviders.SDK/actions/runs/25196752728).




> Generated by 🌈 Repo Assist, see [workflow
run](https://github.com/fsprojects/FSharp.TypeProviders.SDK/actions/runs/25196752728).
[Learn
more](https://github.com/githubnext/agentics/blob/main/docs/repo-assist.md).
>
> To install this [agentic
workflow](https://github.com/githubnext/agentics/blob/96b9d4c39aa22359c0b38265927eadb31dcf4e2a/workflows/repo-assist.md),
run
> ```
> gh aw add
githubnext/agentics@96b9d4c
> ```

<!-- gh-aw-agentic-workflow: Repo Assist, engine: copilot, model: auto,
id: 25196752728, workflow_id: repo-assist, run:
https://github.com/fsprojects/FSharp.TypeProviders.SDK/actions/runs/25196752728
-->

<!-- gh-aw-workflow-id: repo-assist -->

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dsyme dsyme dsyme approved these changes

Assignees

No one assigned

Labels

automation repo-assist

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@dsyme