Skip to content
Draft
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
6b0f7d5
optimizer
majocha Sep 3, 2025
208362a
argInfo + implicit yield
majocha Sep 3, 2025
4925224
fix comment
majocha Sep 4, 2025
59bb141
Merge branch 'main' into cache-2
majocha Sep 4, 2025
bd5a72f
Merge branch 'main' into cache-2
majocha Sep 4, 2025
8d32412
see if this helps
majocha Sep 4, 2025
689112d
wip
majocha Sep 5, 2025
13d4dd1
Revert "wip"
majocha Sep 5, 2025
7543b43
log min threads
majocha Sep 5, 2025
859fdac
?
majocha Sep 5, 2025
873a945
force immediate eviction in some test runs
majocha Sep 5, 2025
03a606d
.
majocha Sep 5, 2025
dedb3ee
Merge branch 'main' into cache-2
majocha Sep 5, 2025
e6fc866
.
majocha Sep 5, 2025
4d69ae5
format
majocha Sep 5, 2025
942640b
.
majocha Sep 5, 2025
d3fd87b
Merge branch 'main' into cache-2
majocha Sep 8, 2025
aac2a46
add a test
majocha Sep 9, 2025
57c8f3f
restore optimizer
majocha Sep 9, 2025
078b49f
additional type rel caches
majocha Sep 10, 2025
e4d45f4
Merge branch 'main' into cache-2
majocha Sep 11, 2025
128e847
while loop
majocha Sep 14, 2025
3d6c92c
Merge branch 'main' into cache-2
majocha Sep 15, 2025
d3dbcd1
Merge branch 'main' into cache-2
majocha Sep 16, 2025
14a7822
Merge branch 'main' into cache-2
majocha Sep 17, 2025
f6c3319
don't unload ad
majocha Sep 17, 2025
62dc77f
server gc in netcore tests
majocha Sep 17, 2025
930d871
collect stats better
majocha Sep 17, 2025
af601bb
show stats in vs output pane
majocha Sep 17, 2025
66a5ba7
fix method missing at runtime - ns2.0 haha
majocha Sep 17, 2025
bd7c24d
format
majocha Sep 17, 2025
8f550ae
Merge branch 'main' into cache-2
majocha Sep 18, 2025
c308343
better formatting of stats
majocha Sep 18, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 1 addition & 8 deletions src/Compiler/Checking/CheckBasics.fs
Original file line number Diff line number Diff line change
Expand Up @@ -243,11 +243,7 @@ type TcEnv =
eLambdaArgInfos: ArgReprInfo list list

// Do we lay down an implicit debug point?
eIsControlFlow: bool

// In order to avoid checking implicit-yield expressions multiple times, we cache the resulting checked expressions.
// This avoids exponential behavior in the type checker when nesting implicit-yield expressions.
eCachedImplicitYieldExpressions : HashMultiMap<range, SynExpr * TType * Expr>
eIsControlFlow: bool
}

member tenv.DisplayEnv = tenv.eNameResEnv.DisplayEnv
Expand Down Expand Up @@ -319,8 +315,6 @@ type TcFileState =

diagnosticOptions: FSharpDiagnosticOptions

argInfoCache: ConcurrentDictionary<(string * range), ArgReprInfo>

// forward call
TcPat: WarnOnUpperFlag -> TcFileState -> TcEnv -> PrelimValReprInfo option -> TcPatValFlags -> TcPatLinearEnv -> TType -> SynPat -> (TcPatPhase2Input -> Pattern) * TcPatLinearEnv

Expand Down Expand Up @@ -370,7 +364,6 @@ type TcFileState =
conditionalDefines = conditionalDefines
isInternalTestSpanStackReferring = isInternalTestSpanStackReferring
diagnosticOptions = diagnosticOptions
argInfoCache = ConcurrentDictionary()
TcPat = tcPat
TcSimplePats = tcSimplePats
TcSequenceExpressionEntry = tcSequenceExpressionEntry
Expand Down
8 changes: 0 additions & 8 deletions src/Compiler/Checking/CheckBasics.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -130,9 +130,6 @@ type TcEnv =

eIsControlFlow: bool

// In order to avoid checking implicit-yield expressions multiple times, we cache the resulting checked expressions.
// This avoids exponential behavior in the type checker when nesting implicit-yield expressions.
eCachedImplicitYieldExpressions: HashMultiMap<range, SynExpr * TType * Expr>
}

member DisplayEnv: DisplayEnv
Expand Down Expand Up @@ -269,11 +266,6 @@ type TcFileState =

diagnosticOptions: FSharpDiagnosticOptions

/// A cache for ArgReprInfos which get created multiple times for the same values
/// Since they need to be later mutated with updates from signature files this should make sure
/// we're always dealing with the same instance and the updates don't get lost
argInfoCache: ConcurrentDictionary<(string * range), ArgReprInfo>

// forward call
TcPat:
WarnOnUpperFlag
Expand Down
3 changes: 1 addition & 2 deletions src/Compiler/Checking/CheckDeclarations.fs
Original file line number Diff line number Diff line change
Expand Up @@ -5620,8 +5620,7 @@ let emptyTcEnv g =
eCtorInfo = None
eCallerMemberName = None
eLambdaArgInfos = []
eIsControlFlow = false
eCachedImplicitYieldExpressions = HashMultiMap(HashIdentity.Structural, useConcurrentDictionary = true) }
eIsControlFlow = false }

let CreateInitialTcEnv(g, amap, scopem, assemblyName, ccus) =
(emptyTcEnv g, ccus) ||> List.collectFold (fun env (ccu, autoOpens, internalsVisible) ->
Expand Down
47 changes: 22 additions & 25 deletions src/Compiler/Checking/Expressions/CheckExpressions.fs
Original file line number Diff line number Diff line change
Expand Up @@ -955,8 +955,12 @@ let AdjustValSynInfoInSignature g ty (SynValInfo(argsData, retData) as sigMD) =
| _ ->
sigMD

let getArgInfoCache =
let options = Caches.CacheOptions.getDefault() |> Caches.CacheOptions.withNoEviction
let factory _ = new Caches.Cache<_, ArgReprInfo>(options, "argInfoCache")
WeakMap.getOrCreate factory
Comment on lines +958 to +961
Copy link
Member

Choose a reason for hiding this comment

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

Are these caches going to be cleared when asking FCS to clear its caches?

Copy link
Contributor Author

@majocha majocha Aug 29, 2025

Choose a reason for hiding this comment

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

Currently not, I think since they're weakly attached to higher lever stuff, they'll just get GC'd with it. I captured some telemetry and the caches get disposed quite often during the test run.


let TranslateTopArgSynInfo (cenv: cenv) isArg m tcAttributes (SynArgInfo(Attributes attrs, isOpt, nm)) =
let TranslateTopArgSynInfo cenv isArg m tcAttributes (SynArgInfo(Attributes attrs, isOpt, nm)) =
// Synthesize an artificial "OptionalArgument" attribute for the parameter
let optAttrs =
if isOpt then
Expand All @@ -977,20 +981,14 @@ let TranslateTopArgSynInfo (cenv: cenv) isArg m tcAttributes (SynArgInfo(Attribu
// Call the attribute checking function
let attribs = tcAttributes (optAttrs@attrs)

let key = nm |> Option.map (fun id -> id.idText, id.idRange)
let key = nm |> Option.map (fun id -> (id.idText, id.idRange))

let mkDefaultArgInfo _ : ArgReprInfo = { Attribs = attribs; Name = nm; OtherRange = None }

let argInfo =
key
|> Option.map cenv.argInfoCache.TryGetValue
|> Option.bind (fun (found, info) ->
if found then
Some info
else None)
|> Option.defaultValue ({ Attribs = attribs; Name = nm; OtherRange = None }: ArgReprInfo)

match key with
| Some k -> cenv.argInfoCache.[k] <- argInfo
| None -> ()
match key with
| Some key -> (getArgInfoCache cenv).GetOrAdd(key, mkDefaultArgInfo)
| _ -> mkDefaultArgInfo ()

// Set freshly computed attribs in case they are different in the cache
argInfo.Attribs <- attribs
Expand Down Expand Up @@ -4051,6 +4049,13 @@ type ImplicitlyBoundTyparsAllowed =
| NewTyparsOK
| NoNewTypars

// In order to avoid checking implicit-yield expressions multiple times, we cache the resulting checked expressions.
// This avoids exponential behavior in the type checker when nesting implicit-yield expressions.
let getImplicitYieldExpressionsCache =
let options = Caches.CacheOptions.getReferenceIdentity() |> Caches.CacheOptions.withNoEviction
let factory _ = new Caches.Cache<SynExpr, _>(options, "implicitYieldExpressions")
WeakMap.getOrCreate factory

//-------------------------------------------------------------------------
// Checking types and type constraints
//-------------------------------------------------------------------------
Expand Down Expand Up @@ -5503,19 +5508,12 @@ and CheckForAdjacentListExpression (cenv: cenv) synExpr hpa isInfix delayed (arg
and TcExprThen (cenv: cenv) overallTy env tpenv isArg synExpr delayed =
let g = cenv.g

let cachedExpression =
env.eCachedImplicitYieldExpressions.FindAll synExpr.Range
|> List.tryPick (fun (se, ty, e) ->
if obj.ReferenceEquals(se, synExpr) then Some (ty, e) else None
)

match cachedExpression with
| Some (ty, expr) ->
match (getImplicitYieldExpressionsCache cenv).TryGetValue synExpr with
| true, (ty, expr) ->
UnifyOverallType cenv env synExpr.Range overallTy ty
expr, tpenv
| _ ->


match synExpr with

// A.
Expand Down Expand Up @@ -6378,9 +6376,8 @@ and TcExprSequentialOrImplicitYield (cenv: cenv) overallTy env tpenv (sp, synExp
| Expr.DebugPoint(_,e) -> e
| _ -> expr1

env.eCachedImplicitYieldExpressions.Add(synExpr1.Range, (synExpr1, expr1Ty, cachedExpr))
try TcExpr cenv overallTy env tpenv otherExpr
finally env.eCachedImplicitYieldExpressions.Remove synExpr1.Range
(getImplicitYieldExpressionsCache cenv).AddOrUpdate(synExpr1, (expr1Ty, cachedExpr))
Copy link
Member

Choose a reason for hiding this comment

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

The semantics w.r.t. to error handling is different now, aren't they?

Also it feels like the previous version had a race condition (FindAll and later Add), possibly the Multi map was chosen to workaround it by storing a list and not a single value?

Copy link
Contributor Author

@majocha majocha Sep 4, 2025

Choose a reason for hiding this comment

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

Previous version had some concurrency problems, because Vlad at some point added ConcurrentDictionary as HashMultiMap backing store to fix them.

Semantics are different. Originally this just always removed the value in finally. I have to revisit this, I remember the change made sense to me but I forgot why, oh lol. I guess my thinking was eviction will be sufficient here. Now I also notice this was attached to env, not cenv.

This is not really tested in the test suite but there is a benchmark. I'll run it later to see if this still works.

TcExpr cenv overallTy env tpenv otherExpr

and TcExprStaticOptimization (cenv: cenv) overallTy env tpenv (constraints, synExpr2, expr3, m) =
let constraintsR, tpenv = List.mapFold (TcStaticOptimizationConstraint cenv env) tpenv constraints
Expand Down
10 changes: 8 additions & 2 deletions src/Compiler/Optimize/Optimizer.fs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ open Internal.Utilities.Collections
open Internal.Utilities.Library
open Internal.Utilities.Library.Extras
open FSharp.Compiler
open FSharp.Compiler.Caches
open FSharp.Compiler.AbstractIL.Diagnostics
open FSharp.Compiler.AbstractIL.IL
open FSharp.Compiler.AttributeChecking
Expand Down Expand Up @@ -36,6 +37,10 @@ open System.Collections.ObjectModel

let OptimizerStackGuardDepth = GetEnvInteger "FSHARP_Optimizer" 50

let getFreeLocalsCache =
let options = CacheOptions.getReferenceIdentity() |> CacheOptions.withNoEviction
WeakMap.getOrCreate <| fun _ -> new Cache<_, _>(options, "freeLocalsCache")

let i_ldlen = [ I_ldlen; (AI_conv DT_I4) ]

/// size of a function call
Expand Down Expand Up @@ -2898,10 +2903,11 @@ and OptimizeLinearExpr cenv env expr contf =

let (bindR, bindingInfo), env = OptimizeBinding cenv false env bind

OptimizeLinearExpr cenv env body (contf << (fun (bodyR, bodyInfo) ->
OptimizeLinearExpr cenv env body (contf << (fun (bodyR, bodyInfo) ->
// PERF: This call to ValueIsUsedOrHasEffect/freeInExpr amounts to 9% of all optimization time.
// Is it quadratic or quasi-quadratic?
if ValueIsUsedOrHasEffect cenv (fun () -> (freeInExpr (CollectLocalsWithStackGuard()) bodyR).FreeLocals) (bindR, bindingInfo) then
let collect expr = (freeInExpr (CollectLocalsWithStackGuard()) expr).FreeLocals
if ValueIsUsedOrHasEffect cenv (fun () -> (getFreeLocalsCache cenv).GetOrAdd(bodyR, collect)) (bindR, bindingInfo) then
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This gives a modest 20% hit ratio in tests. May or may not help somewhat IRL but needs benchmarking.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looking at Optimization times when building FCS I don't see any speedup.

// Eliminate let bindings on the way back up
let exprR, adjust = TryEliminateLet cenv env bindR bodyR m
exprR,
Expand Down
2 changes: 1 addition & 1 deletion src/Compiler/Utilities/Caches.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ module internal CacheMetrics =
/// Set FSHARP_OTEL_EXPORT environment variable to enable OpenTelemetry export to external collectors in tests.
val Meter: Meter

[<Class>]
/// A local listener that can be created for a specific Cache instance to get its metrics. For testing purposes only.
[<Class>]
type internal CacheMetricsListener =
member GetStats: unit -> Map<string, float>
member GetTotals: unit -> Map<string, int64>
Expand Down
Loading