Skip to content

Commit 7584974

Browse files
baronfelTIHan
authored andcommitted
TcSymbolUseData cleanup per #6084 (#6089)
* chunkify TcSymbolUseData * move LOH size out to a constant * do chunking and mapping together to reduce allocations * clarify comment around GC impacts * add comment informing others of the potential for LOH allocations
1 parent ce59d48 commit 7584974

File tree

4 files changed

+68
-12
lines changed

4 files changed

+68
-12
lines changed

src/absil/illib.fs

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,10 @@ let inline isNonNull x = not (isNull x)
4141
let inline nonNull msg x = if isNull x then failwith ("null: " + msg) else x
4242
let inline (===) x y = LanguagePrimitives.PhysicalEquality x y
4343

44+
/// Per the docs the threshold for the Large Object Heap is 85000 bytes: https://docs.microsoft.com/en-us/dotnet/standard/garbage-collection/large-object-heap#how-an-object-ends-up-on-the-large-object-heap-and-how-gc-handles-them
45+
/// We set the limit to slightly under that to allow for some 'slop'
46+
let LOH_SIZE_THRESHOLD_BYTES = 84_900
47+
4448
//---------------------------------------------------------------------
4549
// Library: ReportTime
4650
//---------------------------------------------------------------------
@@ -91,7 +95,7 @@ module Order =
9195
let toFunction (pxOrder: IComparer<'U>) x y = pxOrder.Compare(x,y)
9296

9397
//-------------------------------------------------------------------------
94-
// Library: arrays,lists,options
98+
// Library: arrays,lists,options,resizearrays
9599
//-------------------------------------------------------------------------
96100

97101
module Array =
@@ -432,6 +436,49 @@ module List =
432436
let existsSquared f xss = xss |> List.exists (fun xs -> xs |> List.exists (fun x -> f x))
433437
let mapiFoldSquared f z xss = mapFoldSquared f z (xss |> mapiSquared (fun i j x -> (i,j,x)))
434438

439+
module ResizeArray =
440+
441+
/// Split a ResizeArray into an array of smaller chunks.
442+
/// This requires `items/chunkSize` Array copies of length `chunkSize` if `items/chunkSize % 0 = 0`,
443+
/// otherwise `items/chunkSize + 1` Array copies.
444+
let chunkBySize chunkSize f (items: ResizeArray<'t>) =
445+
// we could use Seq.chunkBySize here, but that would involve many enumerator.MoveNext() calls that we can sidestep with a bit of math
446+
let itemCount = items.Count
447+
if itemCount = 0
448+
then [||]
449+
else
450+
let chunksCount =
451+
match itemCount / chunkSize with
452+
| n when itemCount % chunkSize = 0 -> n
453+
| n -> n + 1 // any remainder means we need an additional chunk to store it
454+
455+
[| for index in 0..chunksCount-1 do
456+
let startIndex = index * chunkSize
457+
let takeCount = min (itemCount - startIndex) chunkSize
458+
459+
let holder = Array.zeroCreate takeCount
460+
// we take a bounds-check hit here on each access.
461+
// other alternatives here include
462+
// * iterating across an IEnumerator (incurs MoveNext penalty)
463+
// * doing a block copy using `List.CopyTo(index, array, index, count)` (requires more copies to do the mapping)
464+
// none are significantly better.
465+
for i in 0 .. takeCount - 1 do
466+
holder.[i] <- f items.[i]
467+
yield holder |]
468+
469+
/// Split a large ResizeArray into a series of array chunks that are each under the Large Object Heap limit.
470+
/// This is done to help prevent a stop-the-world collection of the single large array, instead allowing for a greater
471+
/// probability of smaller collections. Stop-the-world is still possible, just less likely.
472+
let mapToSmallArrayChunks f (inp: ResizeArray<'t>) =
473+
let itemSizeBytes = sizeof<'t>
474+
// rounding down here is good because it ensures we don't go over
475+
let maxArrayItemCount = LOH_SIZE_THRESHOLD_BYTES / itemSizeBytes
476+
477+
/// chunk the provided input into arrays that are smaller than the LOH limit
478+
/// in order to prevent long-term storage of those values
479+
chunkBySize maxArrayItemCount f inp
480+
481+
435482
/// Because FSharp.Compiler.Service is a library that will target FSharp.Core 4.5.2 for the forseeable future,
436483
/// we need to stick these functions in this module rather than using the module functions for ValueOption
437484
/// that come after FSharp.Core 4.5.2.

src/fsharp/NameResolution.fs

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1212,7 +1212,7 @@ let GetNestedTypesOfType (ad, ncenv:NameResolver, optFilter, staticResInfo, chec
12121212
//-------------------------------------------------------------------------
12131213

12141214
/// Represents the kind of the occurrence when reporting a name in name resolution
1215-
[<RequireQualifiedAccess>]
1215+
[<RequireQualifiedAccess; Struct>]
12161216
type ItemOccurence =
12171217
/// This is a binding / declaration of the item
12181218
| Binding
@@ -1496,17 +1496,24 @@ type TcSymbolUseData =
14961496
/// This is a memory-critical data structure - allocations of this data structure and its immediate contents
14971497
/// is one of the highest memory long-lived data structures in typical uses of IDEs. Not many of these objects
14981498
/// are allocated (one per file), but they are large because the allUsesOfAllSymbols array is large.
1499-
type TcSymbolUses(g, capturedNameResolutions : ResizeArray<CapturedNameResolution>, formatSpecifierLocations: (range * int)[]) =
1500-
1499+
type TcSymbolUses(g, capturedNameResolutions : ResizeArray<CapturedNameResolution>, formatSpecifierLocations: (range * int)[]) =
1500+
15011501
// Make sure we only capture the information we really need to report symbol uses
1502-
let allUsesOfSymbols = [| for cnr in capturedNameResolutions -> { Item=cnr.Item; ItemOccurence=cnr.ItemOccurence; DisplayEnv=cnr.DisplayEnv; Range=cnr.Range } |]
1502+
let allUsesOfSymbols =
1503+
capturedNameResolutions
1504+
|> ResizeArray.mapToSmallArrayChunks (fun cnr -> { Item=cnr.Item; ItemOccurence=cnr.ItemOccurence; DisplayEnv=cnr.DisplayEnv; Range=cnr.Range })
1505+
15031506
let capturedNameResolutions = ()
15041507
do ignore capturedNameResolutions // don't capture this!
15051508

15061509
member this.GetUsesOfSymbol(item) =
1507-
[| for symbolUse in allUsesOfSymbols do
1508-
if protectAssemblyExploration false (fun () -> ItemsAreEffectivelyEqual g item symbolUse.Item) then
1509-
yield symbolUse |]
1510+
// This member returns what is potentially a very large array, which may approach the size constraints of the Large Object Heap.
1511+
// This is unlikely in practice, though, because we filter down the set of all symbol uses to those specifically for the given `item`.
1512+
// Consequently we have a much lesser chance of ending up with an array large enough to be promoted to the LOH.
1513+
[| for symbolUseChunk in allUsesOfSymbols do
1514+
for symbolUse in symbolUseChunk do
1515+
if protectAssemblyExploration false (fun () -> ItemsAreEffectivelyEqual g item symbolUse.Item) then
1516+
yield symbolUse |]
15101517

15111518
member this.AllUsesOfSymbols = allUsesOfSymbols
15121519

src/fsharp/NameResolution.fsi

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ type TypeNameResolutionInfo =
240240
static member ResolveToTypeRefs : TypeNameResolutionStaticArgsInfo -> TypeNameResolutionInfo
241241

242242
/// Represents the kind of the occurrence when reporting a name in name resolution
243-
[<RequireQualifiedAccess>]
243+
[<RequireQualifiedAccess; Struct>]
244244
type internal ItemOccurence =
245245
| Binding
246246
| Use
@@ -319,7 +319,7 @@ type internal TcSymbolUses =
319319
member GetUsesOfSymbol : Item -> TcSymbolUseData[]
320320

321321
/// All the uses of all items within the file
322-
member AllUsesOfSymbols : TcSymbolUseData[]
322+
member AllUsesOfSymbols : TcSymbolUseData[][]
323323

324324
/// Get the locations of all the printf format specifiers in the file
325325
member GetFormatSpecifierLocationsAndArity : unit -> (range * int)[]

src/fsharp/service/service.fs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1917,7 +1917,8 @@ type FSharpCheckProjectResults(projectFileName:string, tcConfigOption, keepAssem
19171917
let cenv = SymbolEnv(tcGlobals, thisCcu, Some ccuSig, tcImports)
19181918

19191919
[| for r in tcSymbolUses do
1920-
for symbolUse in r.AllUsesOfSymbols do
1920+
for symbolUseChunk in r.AllUsesOfSymbols do
1921+
for symbolUse in symbolUseChunk do
19211922
if symbolUse.ItemOccurence <> ItemOccurence.RelatedText then
19221923
let symbol = FSharpSymbol.Create(cenv, symbolUse.Item)
19231924
yield FSharpSymbolUse(tcGlobals, symbolUse.DisplayEnv, symbol, symbolUse.ItemOccurence, symbolUse.Range) |]
@@ -2111,7 +2112,8 @@ type FSharpCheckFileResults(filename: string, errors: FSharpErrorInfo[], scopeOp
21112112
(fun () -> [| |])
21122113
(fun scope ->
21132114
let cenv = scope.SymbolEnv
2114-
[| for symbolUse in scope.ScopeSymbolUses.AllUsesOfSymbols do
2115+
[| for symbolUseChunk in scope.ScopeSymbolUses.AllUsesOfSymbols do
2116+
for symbolUse in symbolUseChunk do
21152117
if symbolUse.ItemOccurence <> ItemOccurence.RelatedText then
21162118
let symbol = FSharpSymbol.Create(cenv, symbolUse.Item)
21172119
yield FSharpSymbolUse(scope.TcGlobals, symbolUse.DisplayEnv, symbol, symbolUse.ItemOccurence, symbolUse.Range) |])

0 commit comments

Comments
 (0)