Skip to content

FCS: fix races that made the background builder repeat work - #20481

Open
auduchinok wants to merge 2 commits into
dotnet:mainfrom
auduchinok:bg-builder-races
Open

FCS: fix races that made the background builder repeat work#20481
auduchinok wants to merge 2 commits into
dotnet:mainfrom
auduchinok:bg-builder-races

Conversation

@auduchinok

Copy link
Copy Markdown
Member
  • IncrementalBuilder: read and replace the state under the semaphore, so concurrent requests after a file change no longer each build their own chain of bound models, and concurrent NotifyFileChanged calls no longer lose each other's notification.
  • BackgroundCompiler: replace the builder node only if nobody else already did, so invalidated references create one builder instead of one per concurrent caller.
  • BackgroundCompiler: memoize parse results as a node created under the cache lock, so concurrent parses of the same source share one parse.
  • Add a stress test: 50 concurrent project checks after a file change type-checked every file 10 times before, once now.

- IncrementalBuilder: read and replace the state under the semaphore, so concurrent
  requests after a file change no longer each build their own chain of bound models,
  and concurrent NotifyFileChanged calls no longer lose each other's notification.
- BackgroundCompiler: replace the builder node only if nobody else already did, so
  invalidated references create one builder instead of one per concurrent caller.
- BackgroundCompiler: memoize parse results as a node created under the cache lock,
  so concurrent parses of the same source share one parse.
- Add a stress test: 50 concurrent project checks after a file change type-checked
  every file 10 times before, once now.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

❗ Release notes required

You can open this PR in browser to add release notes: open in github.dev


✅ Found changes and release notes in following paths:

Change path Release notes path Description
`src/Compiler` docs/release-notes/.FSharp.Compiler.Service/11.0.100.md

@github-actions github-actions Bot added the AI-Tooling-Check-Scanned-Clean Tooling check: diff analyzed, no interesting infrastructure files label Sep 7, 2026
// STATIC ROOT: FSharpLanguageServiceTestable.FSharpChecker.parseFileInProjectCache. Most recently used cache for parsing files.
let parseFileCache =
MruCache<ParseCacheLockToken, _ * SourceTextHash * _, _>(
MruCache<ParseCacheLockToken, _ * SourceTextHash * _, GraphNode<FSharpParseFileResults>>(

@T-Gro T-Gro Sep 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🤖 🕵️ [P2] Unchanged files reparse after GC even while their parse results are retained. With FCS_ParseFileCacheSize=2 set before launch, this prints 0; true on base and 1; false on head: the weak entry targets a GraphNode that the retained result does not keep alive. Preserve weak reuse of completed results alongside in-flight sharing.

open System
open System.IO
open FSharp.Compiler.CodeAnalysis
open FSharp.Compiler.Text

let repro () =
    let checker = FSharpChecker.Create(useTransparentCompiler = false)
    let files = Array.init 3 (fun i -> Path.GetFullPath $"CacheProbe{i}.fs")
    let options = { FSharpParsingOptions.Default with SourceFiles = files }
    let source = SourceText.ofString "module CacheProbe\nlet value = 1"
    let parse i = checker.ParseFile(files[i], source, options, cache = true) |> Async.RunSynchronously
    let retained = Array.init 3 parse
    GC.Collect()
    GC.WaitForPendingFinalizers()
    GC.Collect()
    let before = FSharpChecker.ActualParseFileCount
    let again = parse 0
    printfn "%d; %b" (FSharpChecker.ActualParseFileCount - before)
        (Object.ReferenceEquals(retained[0], again))
    GC.KeepAlive retained

repro ()

@T-Gro T-Gro added the AI-reviewed PR reviewed by AI review council label Sep 9, 2026
@T-Gro
T-Gro self-requested a review September 9, 2026 09:18
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI-reviewed PR reviewed by AI review council AI-Tooling-Check-Scanned-Clean Tooling check: diff analyzed, no interesting infrastructure files

Projects

Status: New

Development

Successfully merging this pull request may close these issues.

2 participants