Use new Cache api in various places #18872
Conversation
❗ Release notes requiredCaution No release notes found for the changed paths (see table below). Please make sure to add an entry with an informative description of the change as well as link to this pull request, issue and language suggestion if applicable. Release notes for this repository are based on Keep A Changelog format. The following format is recommended for this repository:
If you believe that release notes are not necessary for this PR, please add NO_RELEASE_NOTES label to the pull request. You can open this PR in browser to add release notes: open in github.dev
|
| // 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 |
There was a problem hiding this comment.
This gives a modest 20% hit ratio in tests. May or may not help somewhat IRL but needs benchmarking.
| /// Usage: | ||
| /// let getValueFor = WeakMap.getOrCreate (fun () -> expensiveInit()) | ||
| /// let v = getValueFor someKey | ||
| let getOrCreate valueFactory = |
There was a problem hiding this comment.
Allows to quickly and and-hoc attach a cache instance in various places. Currently we thread various caches along with context objects / records which is not that convenient when testing things out.
| let getArgInfoCache = | ||
| let options = Caches.CacheOptions.getDefault() |> Caches.CacheOptions.withNoEviction | ||
| let factory _ = new Caches.Cache<_, ArgReprInfo>(options, "argInfoCache") | ||
| WeakMap.getOrCreate factory |
There was a problem hiding this comment.
Are these caches going to be cleared when asking FCS to clear its caches?
There was a problem hiding this comment.
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.
|
Ahh, there's that memory leak in vs tests, again 😔 |
Also add no-eviction to type subsumption cache conditional on
OneOffcompilation mode.