Skip to content

Commit b721d8d

Browse files
[Repo Assist] refactor: add save-based caching to GetField/GetEvent/GetNestedType; use Dictionary in ILNestedExportedTypesAndForwarders (#498)
🤖 *This is an automated PR from Repo Assist.* ## Summary Two complementary consistency/performance improvements to `ProvidedTypeDefinition` and `ILNestedExportedTypesAndForwarders`. ### 1. `save`-based caching for `GetField`, `GetEvent`, `GetNestedType` **Before**: these three overrides used `Array.filter` / `Array.tryFind` for O(n) linear scans on every call: ```fsharp override this.GetField(name, bindingFlags) = let xs = this.GetFields bindingFlags |> Array.filter (fun m -> m.Name = name) if xs.Length > 0 then xs.[0] else null ``` **After**: they now use the same `save`-based dictionary cache that `GetMethodImpl` and `GetPropertyImpl` already use — building a `name → member` dictionary once per `bindingFlags` combination and doing O(1) lookups thereafter: ```fsharp override this.GetField(name, bindingFlags) = let table = save (bindingFlags ||| BindingFlags.GetField) (fun () -> this.GetFields bindingFlags |> Seq.map (fun f -> f.Name, f) |> dict) match table.TryGetValue name with | true, f -> f | false, _ -> null ``` Discriminator bits chosen to avoid cache-key collisions with existing `save` calls: | Member | Discriminator bit | |--------|-------------------| | `GetMethodImpl` | `BindingFlags.InvokeMethod` (0x100) — existing | | `GetPropertyImpl` | `BindingFlags.GetProperty` (0x1000) — existing | | `GetField` | `BindingFlags.GetField` (0x400) — new | | `GetEvent` | `BindingFlags.SetField` (0x800) — new | | `GetNestedType` | `BindingFlags.SetProperty` (0x2000) — new | The stale `//save ("field1", ...)` / `//save ("event1", ...)` / `//save ("nested1", ...)` comments (from a previous refactor attempt) are also removed to reduce noise. ### 2. `ILNestedExportedTypesAndForwarders`: `Map` → `Dictionary` **Before**: used a functional `Map` (balanced BST, O(log n) lookup) built via `Array.fold`: ```fsharp let lmap = lazy ((Map.empty, larr.Force()) ||> Array.fold (fun m x -> m.Add(x.Name, x))) member __.TryFindByName nm = lmap.Force().TryFind nm ``` **After**: consistent with the adjacent `ILTypeDefs` and `ILExportedTypesAndForwarders` types which already use `Dictionary`: ```fsharp let lmap = lazy ( let m = Dictionary() for x in larr.Force() do m.[x.Name] <- x m) member __.TryFindByName nm = match lmap.Force().TryGetValue nm with true, v -> Some v | false, _ -> None ``` ## Test Status All **126/126** tests pass (`dotnet test tests/FSharp.TypeProviders.SDK.Tests.fsproj -c Release`). > Generated by 🌈 Repo Assist, see [workflow run](https://github.com/fsprojects/FSharp.TypeProviders.SDK/actions/runs/23990843787). [Learn more](https://github.com/githubnext/agentics/blob/main/docs/repo-assist.md). > > To install this [agentic workflow](https://github.com/githubnext/agentics/blob/7ee2b60744abf71b985bead4599640f165edcd93/workflows/repo-assist.md), run > ``` > gh aw add githubnext/agentics@7ee2b60 > ``` <!-- gh-aw-agentic-workflow: Repo Assist, engine: copilot, model: auto, id: 23990843787, workflow_id: repo-assist, run: https://github.com/fsprojects/FSharp.TypeProviders.SDK/actions/runs/23990843787 --> <!-- 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>
1 parent 41c3167 commit b721d8d

1 file changed

Lines changed: 32 additions & 31 deletions

File tree

src/ProvidedTypes.fs

Lines changed: 32 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1619,43 +1619,41 @@ and ProvidedTypeDefinition(isTgt: bool, container:TypeContainer, className: stri
16191619
if xs.Length > 0 then xs.[0] else null
16201620

16211621
override __.GetMethodImpl(name, bindingFlags, _binderBinder, _callConvention, _types, _modifiers): MethodInfo =
1622-
(//save ("methimpl", bindingFlags, Some name) (fun () ->
1623-
// This is performance critical for large spaces of provided methods and properties
1624-
// Save a table of the methods grouped by name
1625-
let table =
1626-
save (bindingFlags ||| BindingFlags.InvokeMethod) (fun () ->
1627-
let methods = this.GetMethods bindingFlags
1628-
methods |> Seq.groupBy (fun m -> m.Name) |> Seq.map (fun (k, v) -> k, Seq.toArray v) |> dict)
1629-
1630-
let xs = match table.TryGetValue name with | true, tn -> tn | false, _ -> [| |]
1631-
//let xs = this.GetMethods bindingFlags |> Array.filter (fun m -> m.Name = name)
1632-
if xs.Length > 1 then failwithf "GetMethodImpl. not support overloads, name = '%s', methods - '%A', callstack = '%A'" name xs Environment.StackTrace
1633-
if xs.Length > 0 then xs.[0] else null)
1622+
// This is performance critical for large spaces of provided methods and properties.
1623+
// Build a name→methods table once per bindingFlags combination and cache it.
1624+
let table =
1625+
save (bindingFlags ||| BindingFlags.InvokeMethod) (fun () ->
1626+
let methods = this.GetMethods bindingFlags
1627+
methods |> Seq.groupBy (fun m -> m.Name) |> Seq.map (fun (k, v) -> k, Seq.toArray v) |> dict)
1628+
let xs = match table.TryGetValue name with | true, tn -> tn | false, _ -> [| |]
1629+
if xs.Length > 1 then failwithf "GetMethodImpl. not support overloads, name = '%s', methods - '%A', callstack = '%A'" name xs Environment.StackTrace
1630+
if xs.Length > 0 then xs.[0] else null
16341631

16351632
override this.GetField(name, bindingFlags) =
1636-
(//save ("field1", bindingFlags, Some name) (fun () ->
1637-
let xs = this.GetFields bindingFlags |> Array.filter (fun m -> m.Name = name)
1638-
if xs.Length > 0 then xs.[0] else null)
1633+
let table =
1634+
save (bindingFlags ||| BindingFlags.GetField) (fun () ->
1635+
this.GetFields bindingFlags |> Seq.map (fun f -> f.Name, f) |> dict)
1636+
match table.TryGetValue name with | true, f -> f | false, _ -> null
16391637

16401638
override __.GetPropertyImpl(name, bindingFlags, _binder, _returnType, _types, _modifiers) =
1641-
(//save ("prop1", bindingFlags, Some name) (fun () ->
1642-
let table =
1643-
save (bindingFlags ||| BindingFlags.GetProperty) (fun () ->
1644-
let methods = this.GetProperties bindingFlags
1645-
methods |> Seq.groupBy (fun m -> m.Name) |> Seq.map (fun (k, v) -> k, Seq.toArray v) |> dict)
1646-
let xs = match table.TryGetValue name with | true, tn -> tn | false, _ -> [| |]
1647-
//let xs = this.GetProperties bindingFlags |> Array.filter (fun m -> m.Name = name)
1648-
if xs.Length > 0 then xs.[0] else null)
1639+
let table =
1640+
save (bindingFlags ||| BindingFlags.GetProperty) (fun () ->
1641+
let methods = this.GetProperties bindingFlags
1642+
methods |> Seq.groupBy (fun m -> m.Name) |> Seq.map (fun (k, v) -> k, Seq.toArray v) |> dict)
1643+
let xs = match table.TryGetValue name with | true, tn -> tn | false, _ -> [| |]
1644+
if xs.Length > 0 then xs.[0] else null
16491645

16501646
override __.GetEvent(name, bindingFlags) =
1651-
(//save ("event1", bindingFlags, Some name) (fun () ->
1652-
let xs = this.GetEvents bindingFlags |> Array.filter (fun m -> m.Name = name)
1653-
if xs.Length > 0 then xs.[0] else null)
1647+
let table =
1648+
save (bindingFlags ||| BindingFlags.SetField) (fun () ->
1649+
this.GetEvents bindingFlags |> Seq.map (fun e -> e.Name, e) |> dict)
1650+
match table.TryGetValue name with | true, e -> e | false, _ -> null
16541651

16551652
override __.GetNestedType(name, bindingFlags) =
1656-
(//save ("nested1", bindingFlags, Some name) (fun () ->
1657-
let xs = this.GetNestedTypes bindingFlags |> Array.filter (fun m -> m.Name = name)
1658-
if xs.Length > 0 then xs.[0] else null)
1653+
let table =
1654+
save (bindingFlags ||| BindingFlags.SetProperty) (fun () ->
1655+
this.GetNestedTypes bindingFlags |> Seq.map (fun t -> t.Name, t) |> dict)
1656+
match table.TryGetValue name with | true, t -> t | false, _ -> null
16591657

16601658
override __.GetInterface(name, ignoreCase) =
16611659
let sc = if ignoreCase then StringComparison.OrdinalIgnoreCase else StringComparison.Ordinal
@@ -3133,9 +3131,12 @@ module internal AssemblyReader =
31333131
override x.ToString() = "nested fwd " + x.Name
31343132

31353133
and ILNestedExportedTypesAndForwarders(larr:Lazy<ILNestedExportedType[]>) =
3136-
let lmap = lazy ((Map.empty, larr.Force()) ||> Array.fold (fun m x -> m.Add(x.Name, x)))
3134+
let lmap = lazy (
3135+
let m = Dictionary()
3136+
for x in larr.Force() do m.[x.Name] <- x
3137+
m)
31373138
member __.Entries = larr.Force()
3138-
member __.TryFindByName nm = lmap.Force().TryFind nm
3139+
member __.TryFindByName nm = match lmap.Force().TryGetValue nm with true, v -> Some v | false, _ -> None
31393140

31403141
and [<NoComparison; NoEquality>]
31413142
ILExportedTypeOrForwarder =

0 commit comments

Comments
 (0)