Skip to content

Commit

Permalink
TcSymbolUseData cleanup per dotnet#6084 (dotnet#6089)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
baronfel authored and TIHan committed Jan 17, 2019
1 parent ce59d48 commit 7584974
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 12 deletions.
49 changes: 48 additions & 1 deletion src/absil/illib.fs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ let inline isNonNull x = not (isNull x)
let inline nonNull msg x = if isNull x then failwith ("null: " + msg) else x
let inline (===) x y = LanguagePrimitives.PhysicalEquality x y

/// 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
/// We set the limit to slightly under that to allow for some 'slop'
let LOH_SIZE_THRESHOLD_BYTES = 84_900

//---------------------------------------------------------------------
// Library: ReportTime
//---------------------------------------------------------------------
Expand Down Expand Up @@ -91,7 +95,7 @@ module Order =
let toFunction (pxOrder: IComparer<'U>) x y = pxOrder.Compare(x,y)

//-------------------------------------------------------------------------
// Library: arrays,lists,options
// Library: arrays,lists,options,resizearrays
//-------------------------------------------------------------------------

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

module ResizeArray =

/// Split a ResizeArray into an array of smaller chunks.
/// This requires `items/chunkSize` Array copies of length `chunkSize` if `items/chunkSize % 0 = 0`,
/// otherwise `items/chunkSize + 1` Array copies.
let chunkBySize chunkSize f (items: ResizeArray<'t>) =
// we could use Seq.chunkBySize here, but that would involve many enumerator.MoveNext() calls that we can sidestep with a bit of math
let itemCount = items.Count
if itemCount = 0
then [||]
else
let chunksCount =
match itemCount / chunkSize with
| n when itemCount % chunkSize = 0 -> n
| n -> n + 1 // any remainder means we need an additional chunk to store it

[| for index in 0..chunksCount-1 do
let startIndex = index * chunkSize
let takeCount = min (itemCount - startIndex) chunkSize

let holder = Array.zeroCreate takeCount
// we take a bounds-check hit here on each access.
// other alternatives here include
// * iterating across an IEnumerator (incurs MoveNext penalty)
// * doing a block copy using `List.CopyTo(index, array, index, count)` (requires more copies to do the mapping)
// none are significantly better.
for i in 0 .. takeCount - 1 do
holder.[i] <- f items.[i]
yield holder |]

/// Split a large ResizeArray into a series of array chunks that are each under the Large Object Heap limit.
/// This is done to help prevent a stop-the-world collection of the single large array, instead allowing for a greater
/// probability of smaller collections. Stop-the-world is still possible, just less likely.
let mapToSmallArrayChunks f (inp: ResizeArray<'t>) =
let itemSizeBytes = sizeof<'t>
// rounding down here is good because it ensures we don't go over
let maxArrayItemCount = LOH_SIZE_THRESHOLD_BYTES / itemSizeBytes

/// chunk the provided input into arrays that are smaller than the LOH limit
/// in order to prevent long-term storage of those values
chunkBySize maxArrayItemCount f inp


/// Because FSharp.Compiler.Service is a library that will target FSharp.Core 4.5.2 for the forseeable future,
/// we need to stick these functions in this module rather than using the module functions for ValueOption
/// that come after FSharp.Core 4.5.2.
Expand Down
21 changes: 14 additions & 7 deletions src/fsharp/NameResolution.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1212,7 +1212,7 @@ let GetNestedTypesOfType (ad, ncenv:NameResolver, optFilter, staticResInfo, chec
//-------------------------------------------------------------------------

/// Represents the kind of the occurrence when reporting a name in name resolution
[<RequireQualifiedAccess>]
[<RequireQualifiedAccess; Struct>]
type ItemOccurence =
/// This is a binding / declaration of the item
| Binding
Expand Down Expand Up @@ -1496,17 +1496,24 @@ type TcSymbolUseData =
/// This is a memory-critical data structure - allocations of this data structure and its immediate contents
/// is one of the highest memory long-lived data structures in typical uses of IDEs. Not many of these objects
/// are allocated (one per file), but they are large because the allUsesOfAllSymbols array is large.
type TcSymbolUses(g, capturedNameResolutions : ResizeArray<CapturedNameResolution>, formatSpecifierLocations: (range * int)[]) =
type TcSymbolUses(g, capturedNameResolutions : ResizeArray<CapturedNameResolution>, formatSpecifierLocations: (range * int)[]) =

// Make sure we only capture the information we really need to report symbol uses
let allUsesOfSymbols = [| for cnr in capturedNameResolutions -> { Item=cnr.Item; ItemOccurence=cnr.ItemOccurence; DisplayEnv=cnr.DisplayEnv; Range=cnr.Range } |]
let allUsesOfSymbols =
capturedNameResolutions
|> ResizeArray.mapToSmallArrayChunks (fun cnr -> { Item=cnr.Item; ItemOccurence=cnr.ItemOccurence; DisplayEnv=cnr.DisplayEnv; Range=cnr.Range })

let capturedNameResolutions = ()
do ignore capturedNameResolutions // don't capture this!

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

member this.AllUsesOfSymbols = allUsesOfSymbols

Expand Down
4 changes: 2 additions & 2 deletions src/fsharp/NameResolution.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ type TypeNameResolutionInfo =
static member ResolveToTypeRefs : TypeNameResolutionStaticArgsInfo -> TypeNameResolutionInfo

/// Represents the kind of the occurrence when reporting a name in name resolution
[<RequireQualifiedAccess>]
[<RequireQualifiedAccess; Struct>]
type internal ItemOccurence =
| Binding
| Use
Expand Down Expand Up @@ -319,7 +319,7 @@ type internal TcSymbolUses =
member GetUsesOfSymbol : Item -> TcSymbolUseData[]

/// All the uses of all items within the file
member AllUsesOfSymbols : TcSymbolUseData[]
member AllUsesOfSymbols : TcSymbolUseData[][]

/// Get the locations of all the printf format specifiers in the file
member GetFormatSpecifierLocationsAndArity : unit -> (range * int)[]
Expand Down
6 changes: 4 additions & 2 deletions src/fsharp/service/service.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1917,7 +1917,8 @@ type FSharpCheckProjectResults(projectFileName:string, tcConfigOption, keepAssem
let cenv = SymbolEnv(tcGlobals, thisCcu, Some ccuSig, tcImports)

[| for r in tcSymbolUses do
for symbolUse in r.AllUsesOfSymbols do
for symbolUseChunk in r.AllUsesOfSymbols do
for symbolUse in symbolUseChunk do
if symbolUse.ItemOccurence <> ItemOccurence.RelatedText then
let symbol = FSharpSymbol.Create(cenv, symbolUse.Item)
yield FSharpSymbolUse(tcGlobals, symbolUse.DisplayEnv, symbol, symbolUse.ItemOccurence, symbolUse.Range) |]
Expand Down Expand Up @@ -2111,7 +2112,8 @@ type FSharpCheckFileResults(filename: string, errors: FSharpErrorInfo[], scopeOp
(fun () -> [| |])
(fun scope ->
let cenv = scope.SymbolEnv
[| for symbolUse in scope.ScopeSymbolUses.AllUsesOfSymbols do
[| for symbolUseChunk in scope.ScopeSymbolUses.AllUsesOfSymbols do
for symbolUse in symbolUseChunk do
if symbolUse.ItemOccurence <> ItemOccurence.RelatedText then
let symbol = FSharpSymbol.Create(cenv, symbolUse.Item)
yield FSharpSymbolUse(scope.TcGlobals, symbolUse.DisplayEnv, symbol, symbolUse.ItemOccurence, symbolUse.Range) |])
Expand Down

0 comments on commit 7584974

Please sign in to comment.