-
Notifications
You must be signed in to change notification settings - Fork 824
Use new Cache api in various places #18872
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 4 commits
6b0f7d5
208362a
4925224
59bb141
bd5a72f
8d32412
689112d
13d4dd1
7543b43
859fdac
873a945
03a606d
dedb3ee
e6fc866
4d69ae5
942640b
d3fd87b
aac2a46
57c8f3f
078b49f
e4d45f4
128e847
3d6c92c
d3dbcd1
14a7822
f6c3319
62dc77f
930d871
af601bb
66a5ba7
bd7c24d
8f550ae
c308343
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
|
||
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 | ||
|
@@ -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 | ||
|
@@ -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 | ||
//------------------------------------------------------------------------- | ||
|
@@ -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. | ||
|
@@ -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)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 ( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Previous version had some concurrency problems, because Vlad at some point added Semantics are different. Originally this just always removed the value in 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 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
|
@@ -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 | ||
|
@@ -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 | ||
|
||
// Eliminate let bindings on the way back up | ||
let exprR, adjust = TryEliminateLet cenv env bindR bodyR m | ||
exprR, | ||
|
There was a problem hiding this comment.
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?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
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.