Skip to content

Commit

Permalink
Revert TcSymbolUseData cleanup per dotnet#6084 (dotnet#6089)
Browse files Browse the repository at this point in the history
  • Loading branch information
auduchinok committed Jun 9, 2020
1 parent 8867dd9 commit 39ccbd1
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 66 deletions.
49 changes: 3 additions & 46 deletions src/absil/illib.fs
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,6 @@ 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 be 80k to account for larger pointer sizes for when F# is running 64-bit.
let LOH_SIZE_THRESHOLD_BYTES = 80_000

//---------------------------------------------------------------------
// Library: ReportTime
//---------------------------------------------------------------------
Expand Down Expand Up @@ -415,48 +411,9 @@ module List =

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.
module ValueOptionInternal =

let inline ofOption x = match x with Some x -> ValueSome x | None -> ValueNone
Expand Down
21 changes: 7 additions & 14 deletions src/fsharp/NameResolution.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1403,7 +1403,7 @@ let GetNestedTypesOfType (ad, ncenv: NameResolver, optFilter, staticResInfo, che
//-------------------------------------------------------------------------

/// Represents the kind of the occurrence when reporting a name in name resolution
[<RequireQualifiedAccess; Struct>]
[<RequireQualifiedAccess>]
type ItemOccurence =
/// This is a binding / declaration of the item
| Binding
Expand Down Expand Up @@ -1701,24 +1701,17 @@ 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 =
capturedNameResolutions
|> ResizeArray.mapToSmallArrayChunks (fun cnr -> { Item=cnr.Item; ItemOccurence=cnr.ItemOccurence; DisplayEnv=cnr.DisplayEnv; Range=cnr.Range })

let allUsesOfSymbols = [| for cnr in capturedNameResolutions -> { 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 =
// 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 |]
[| for symbolUse in allUsesOfSymbols 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 @@ -282,7 +282,7 @@ type TypeNameResolutionInfo =
static member ResolveToTypeRefs : TypeNameResolutionStaticArgsInfo -> TypeNameResolutionInfo

/// Represents the kind of the occurrence when reporting a name in name resolution
[<RequireQualifiedAccess; Struct>]
[<RequireQualifiedAccess>]
type internal ItemOccurence =
| Binding
| Use
Expand Down Expand Up @@ -363,7 +363,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: 2 additions & 4 deletions src/fsharp/service/FSharpCheckerResults.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1861,8 +1861,7 @@ type FSharpCheckFileResults
(fun () -> [| |])
(fun scope ->
let cenv = scope.SymbolEnv
[| for symbolUseChunk in scope.ScopeSymbolUses.AllUsesOfSymbols do
for symbolUse in symbolUseChunk do
[| for symbolUse in scope.ScopeSymbolUses.AllUsesOfSymbols 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 Expand Up @@ -2087,8 +2086,7 @@ type FSharpCheckProjectResults
let cenv = SymbolEnv(tcGlobals, thisCcu, Some ccuSig, tcImports)

[| for r in tcSymbolUses do
for symbolUseChunk in r.AllUsesOfSymbols do
for symbolUse in symbolUseChunk do
for symbolUse in r.AllUsesOfSymbols 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

0 comments on commit 39ccbd1

Please sign in to comment.