Minor perf opt by Thorium · Pull Request #417 · fsprojects/FSharp.TypeProviders.SDK · GitHub
Skip to content

Minor perf opt - #417

Merged
sergey-tihon merged 1 commit into
fsprojects:masterfrom
Thorium:designtime-perf-opt
May 13, 2025
Merged

sergey-tihon merged 1 commit into
fsprojects:masterfrom
Thorium:designtime-perf-opt

Conversation

@Thorium

@Thorium Thorium commented Apr 15, 2025 •

Copy link
Copy Markdown
Member
  • isNull is inline also in FSharp.Core. It's called in many places.
  • Lazy load of ILAssemblyRefs: There is no point in spending so much time initially to construct ilAssemblyRefs which is only used by GetReferencedAssemblies() call
  • Avoid looping 120 libraries through on most cases: Often convTypeRef is called to get some very basic base-type like a "System.Object" or "System.Int32" thus going through the dependency assemblies one-by-one can be a fall-back method rather than default implementation.

I would love to have some benchmark tests for ProvidedTypes.fs but the issue is that it's mostly used by FSharp.Compiler via a heavy IDE, and current unit-tests are not covering the majority of the process, so I don't have any idea how that could be done. Right now the best way seems to be to attach a VS to VS and open some type-provider and see what is going on.

- isNull is inline also in FSharp.Core
- Faster eqTypes
- Lazy load of ILAssemblyRefs
- Avoid looping 120 libraries through on most cases
@Thorium
Thorium force-pushed the designtime-perf-opt branch from 44dec43 to 1f9af4a Compare May 10, 2025 14:32
@Thorium

Thorium commented May 10, 2025

Copy link
Copy Markdown
Member Author

Copilot AI left a comment

Copy link
Copy Markdown

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 applies minor performance optimizations to the type provider code by reducing unnecessary computations and delaying resource-intensive operations until needed.

  • Changed utility functions (isNull, isNil, isEmpty) to be inline.
  • Introduced lazy initialization for ILAssemblyRefs to defer their construction.
  • Modified ProvidedTypes lookup logic to short-circuit basic type translations while reusing the ProvidedTypeSymbol check.
Comments suppressed due to low confidence (1)

src/ProvidedTypes.fs:6256

  • Ensure that the lazy initialization is thread-safe in contexts where ILAssemblyRefs may be accessed concurrently. If concurrent access is possible, consider specifying an explicit thread-safety mode or verifying that the default Lazy behavior meets the requirements.
let ilAssemblyRefs = lazy [ for i in 1 .. getNumRows ILTableNames.AssemblyRef do yield seekReadAssemblyRef i ]

Comment thread src/ProvidedTypes.fs
@Thorium

Thorium commented May 13, 2025

Copy link
Copy Markdown
Member Author
  • Ensure that the lazy initialization is thread-safe in contexts where ILAssemblyRefs may be accessed concurrently. If concurrent access is possible, consider specifying an explicit thread-safety mode or verifying that the default Lazy behavior meets the requirements.

The type ILModuleReader in ProvidedTypes.fs uses a lot of lazy keyword without lazy (lock (System.Object ()) (fun () ->-pattern. For example, see the seekReadTopExportedTypes and seekReadNestedExportedTypes just above this call. Thus, I expect the type is not shared with multiple threads but rather initialised multiple times if VisualStudio decides to use multiple threads to call this.

@sergey-tihon
sergey-tihon merged commit 3b22420 into fsprojects:master May 13, 2025
@sergey-tihon

Copy link
Copy Markdown
Member

github-actions Bot added a commit that referenced this pull request Feb 24, 2026
Update RELEASE_NOTES.md with changes since 8.1.0:
- #417: Performance optimizations for design-time loading
- Reference assembly loading fixes
- #404: Build fix for FX_NO_LOCAL_FILESYSTEM

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants