Skip to content

Commit c8cc6f2

Browse files
authored
Merge pull request #10310 from TIHan/type-provider-forced-compilation-thread
Sequentialise type provider calls by using a lock rather than requiring the Reactor single threading
2 parents c08f160 + e8607a4 commit c8cc6f2

File tree

8 files changed

+30
-46
lines changed

8 files changed

+30
-46
lines changed

src/fsharp/CompilerConfig.fs

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -238,14 +238,6 @@ type UnresolvedAssemblyReference = UnresolvedAssemblyReference of string * Assem
238238
type ResolvedExtensionReference = ResolvedExtensionReference of string * AssemblyReference list * Tainted<ITypeProvider> list
239239
#endif
240240

241-
/// The thread in which compilation calls will be enqueued and done work on.
242-
/// Note: This is currently only used when disposing of type providers and will be extended to all the other type provider calls when compilations can be done in parallel.
243-
/// Right now all calls in FCS to type providers are single-threaded through use of the reactor thread.
244-
type ICompilationThread =
245-
246-
/// Enqueue work to be done on a compilation thread.
247-
abstract EnqueueWork: (CompilationThreadToken -> unit) -> unit
248-
249241
type ImportedAssembly =
250242
{ ILScopeRef: ILScopeRef
251243
FSharpViewOfMetadata: CcuThunk
@@ -442,7 +434,6 @@ type TcConfigBuilder =
442434
/// show messages about extension type resolution?
443435
mutable showExtensionTypeMessages: bool
444436
#endif
445-
mutable compilationThread: ICompilationThread
446437

447438
/// pause between passes?
448439
mutable pause: bool
@@ -603,9 +594,6 @@ type TcConfigBuilder =
603594
#if !NO_EXTENSIONTYPING
604595
showExtensionTypeMessages = false
605596
#endif
606-
compilationThread =
607-
let ctok = CompilationThreadToken ()
608-
{ new ICompilationThread with member __.EnqueueWork work = work ctok }
609597
pause = false
610598
alwaysCallVirt = true
611599
noDebugData = false
@@ -1000,7 +988,6 @@ type TcConfig private (data: TcConfigBuilder, validate: bool) =
1000988
#if !NO_EXTENSIONTYPING
1001989
member x.showExtensionTypeMessages = data.showExtensionTypeMessages
1002990
#endif
1003-
member x.compilationThread = data.compilationThread
1004991
member x.pause = data.pause
1005992
member x.alwaysCallVirt = data.alwaysCallVirt
1006993
member x.noDebugData = data.noDebugData

src/fsharp/CompilerConfig.fsi

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -92,14 +92,6 @@ type AssemblyReference =
9292

9393
type UnresolvedAssemblyReference = UnresolvedAssemblyReference of string * AssemblyReference list
9494

95-
/// The thread in which compilation calls will be enqueued and done work on.
96-
/// Note: This is currently only used when disposing of type providers and will be extended to all the other type provider calls when compilations can be done in parallel.
97-
/// Right now all calls in FCS to type providers are single-threaded through use of the reactor thread.
98-
type ICompilationThread =
99-
100-
/// Enqueue work to be done on a compilation thread.
101-
abstract EnqueueWork: (CompilationThreadToken -> unit) -> unit
102-
10395
[<RequireQualifiedAccess>]
10496
type CompilerTarget =
10597
| WinExe
@@ -251,7 +243,6 @@ type TcConfigBuilder =
251243
#if !NO_EXTENSIONTYPING
252244
mutable showExtensionTypeMessages: bool
253245
#endif
254-
mutable compilationThread: ICompilationThread
255246
mutable pause: bool
256247
mutable alwaysCallVirt: bool
257248
mutable noDebugData: bool
@@ -429,7 +420,6 @@ type TcConfig =
429420
#if !NO_EXTENSIONTYPING
430421
member showExtensionTypeMessages: bool
431422
#endif
432-
member compilationThread: ICompilationThread
433423
member pause: bool
434424
member alwaysCallVirt: bool
435425
member noDebugData: bool

src/fsharp/CompilerImports.fs

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -695,9 +695,9 @@ type RawFSharpAssemblyDataBackedByFileOnDisk (ilModule: ILModuleDef, ilAssemblyR
695695
type TcImportsSafeDisposal
696696
(disposeActions: ResizeArray<unit -> unit>,
697697
#if !NO_EXTENSIONTYPING
698-
disposeTypeProviderActions: ResizeArray<unit -> unit>,
698+
disposeTypeProviderActions: ResizeArray<unit -> unit>
699699
#endif
700-
compilationThread: ICompilationThread) =
700+
) =
701701

702702
let mutable isDisposed = false
703703

@@ -707,9 +707,7 @@ type TcImportsSafeDisposal
707707
if verbose then
708708
dprintf "disposing of TcImports, %d binaries\n" disposeActions.Count
709709
#if !NO_EXTENSIONTYPING
710-
let actions = disposeTypeProviderActions
711-
if actions.Count > 0 then
712-
compilationThread.EnqueueWork (fun _ -> for action in actions do action())
710+
for action in disposeTypeProviderActions do action()
713711
#endif
714712
for action in disposeActions do action()
715713

@@ -760,8 +758,7 @@ and TcImportsWeakHack (tcImports: WeakReference<TcImports>) =
760758
/// Is a disposable object, but it is recommended not to explicitly call Dispose unless you absolutely know nothing will be using its contents after the disposal.
761759
/// Otherwise, simply allow the GC to collect this and it will properly call Dispose from the finalizer.
762760
and [<Sealed>] TcImports(tcConfigP: TcConfigProvider, initialResolutions: TcAssemblyResolutions, importsBase: TcImports option,
763-
ilGlobalsOpt, compilationThread: ICompilationThread,
764-
dependencyProviderOpt: DependencyProvider option) as this =
761+
ilGlobalsOpt, dependencyProviderOpt: DependencyProvider option) as this =
765762

766763
let mutable resolutions = initialResolutions
767764
let mutable importsBase: TcImports option = importsBase
@@ -786,7 +783,7 @@ and [<Sealed>] TcImports(tcConfigP: TcConfigProvider, initialResolutions: TcAsse
786783
let mutable tcImportsWeak = TcImportsWeakHack (WeakReference<_> this)
787784
#endif
788785

789-
let disposal = new TcImportsSafeDisposal(disposeActions, disposeTypeProviderActions, compilationThread)
786+
let disposal = new TcImportsSafeDisposal(disposeActions, disposeTypeProviderActions)
790787

791788
let CheckDisposed() =
792789
if disposed then assert false
@@ -1257,7 +1254,7 @@ and [<Sealed>] TcImports(tcConfigP: TcConfigProvider, initialResolutions: TcAsse
12571254
for provider in providers do
12581255
tcImportsStrong.AttachDisposeTypeProviderAction(fun () ->
12591256
try
1260-
provider.PUntaintNoFailure(fun x -> x).Dispose()
1257+
provider.PUntaintNoFailure(fun x -> x.Dispose())
12611258
with e ->
12621259
())
12631260

@@ -1667,7 +1664,7 @@ and [<Sealed>] TcImports(tcConfigP: TcConfigProvider, initialResolutions: TcAsse
16671664
let tcResolutions = TcAssemblyResolutions.BuildFromPriorResolutions(ctok, tcConfig, frameworkDLLs, [])
16681665
let tcAltResolutions = TcAssemblyResolutions.BuildFromPriorResolutions(ctok, tcConfig, nonFrameworkDLLs, [])
16691666

1670-
let frameworkTcImports = new TcImports(tcConfigP, tcResolutions, None, None, tcConfig.compilationThread, None)
1667+
let frameworkTcImports = new TcImports(tcConfigP, tcResolutions, None, None, None)
16711668

16721669
// Fetch the primaryAssembly from the referenced assemblies otherwise
16731670
let primaryAssemblyReference =
@@ -1801,7 +1798,7 @@ and [<Sealed>] TcImports(tcConfigP: TcConfigProvider, initialResolutions: TcAsse
18011798
let tcConfig = tcConfigP.Get ctok
18021799
let tcResolutions = TcAssemblyResolutions.BuildFromPriorResolutions(ctok, tcConfig, nonFrameworkReferences, knownUnresolved)
18031800
let references = tcResolutions.GetAssemblyResolutions()
1804-
let tcImports = new TcImports(tcConfigP, tcResolutions, Some baseTcImports, Some tcGlobals.ilg, tcConfig.compilationThread, Some dependencyProvider)
1801+
let tcImports = new TcImports(tcConfigP, tcResolutions, Some baseTcImports, Some tcGlobals.ilg, Some dependencyProvider)
18051802
let! _assemblies = tcImports.RegisterAndImportReferencedAssemblies(ctok, references)
18061803
tcImports.ReportUnresolvedAssemblyReferences knownUnresolved
18071804
return tcImports

src/fsharp/ExtensionTyping.fs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ module internal ExtensionTyping =
171171
tpe.Iter(fun e -> errorR(NumberedError((e.Number, e.ContextualErrorMessage), m)) )
172172
[]
173173

174-
let providers = Tainted<_>.CreateAll providerSpecs
174+
let providers = Tainted<_>.CreateAll(providerSpecs)
175175

176176
providers
177177

src/fsharp/service/IncrementalBuild.fs

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2086,14 +2086,6 @@ type IncrementalBuilder(tcGlobals, frameworkTcImports, nonFrameworkAssemblyInput
20862086
// Never open PDB files for the language service, even if --standalone is specified
20872087
tcConfigB.openDebugInformationForLaterStaticLinking <- false
20882088

2089-
tcConfigB.compilationThread <-
2090-
{ new ICompilationThread with
2091-
member __.EnqueueWork work =
2092-
Reactor.Singleton.EnqueueOp ("Unknown", "ICompilationThread.EnqueueWork", "work", fun ctok ->
2093-
work ctok
2094-
)
2095-
}
2096-
20972089
tcConfigB, sourceFilesNew
20982090

20992091
match loadClosureOpt with

src/fsharp/service/Reactor.fs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ type Reactor() =
4242
let mutable culture = CultureInfo(CultureInfo.CurrentUICulture.Name)
4343

4444
let mutable bgOpCts = new CancellationTokenSource()
45+
4546
/// Mailbox dispatch function.
4647
let builder =
4748
MailboxProcessor<_>.Start <| fun inbox ->
@@ -196,6 +197,7 @@ type Reactor() =
196197
)
197198
return! resultCell.AsyncResult
198199
}
200+
199201
member __.PauseBeforeBackgroundWork with get() = pauseBeforeBackgroundWork and set v = pauseBeforeBackgroundWork <- v
200202

201203
static member Singleton = theReactor

src/fsharp/tainted.fs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,13 @@ open Microsoft.FSharp.Core.CompilerServices
1010
open FSharp.Compiler.AbstractIL.IL
1111
open FSharp.Compiler.AbstractIL.Internal.Library
1212

13+
[<Sealed>]
14+
type internal TypeProviderToken() = interface LockToken
15+
16+
[<Sealed>]
17+
type internal TypeProviderLock() =
18+
inherit Lock<TypeProviderToken>()
19+
1320
type internal TypeProviderError
1421
(
1522
errNum : int,
@@ -69,7 +76,7 @@ type internal TypeProviderError
6976
for msg in errors do
7077
f (new TypeProviderError(errNum, tpDesignation, m, [msg], typeNameContext, methodNameContext))
7178

72-
type TaintedContext = { TypeProvider : ITypeProvider; TypeProviderAssemblyRef : ILScopeRef }
79+
type TaintedContext = { TypeProvider : ITypeProvider; TypeProviderAssemblyRef : ILScopeRef; Lock: TypeProviderLock }
7380

7481
[<NoEquality>][<NoComparison>]
7582
type internal Tainted<'T> (context : TaintedContext, value : 'T) =
@@ -88,7 +95,7 @@ type internal Tainted<'T> (context : TaintedContext, value : 'T) =
8895

8996
member this.Protect f (range:range) =
9097
try
91-
f value
98+
context.Lock.AcquireLock(fun _ -> f value)
9299
with
93100
| :? TypeProviderError -> reraise()
94101
| :? AggregateException as ae ->
@@ -143,7 +150,7 @@ type internal Tainted<'T> (context : TaintedContext, value : 'T) =
143150

144151
static member CreateAll(providerSpecs : (ITypeProvider * ILScopeRef) list) =
145152
[for (tp,nm) in providerSpecs do
146-
yield Tainted<_>({ TypeProvider=tp; TypeProviderAssemblyRef=nm },tp) ]
153+
yield Tainted<_>({ TypeProvider=tp; TypeProviderAssemblyRef=nm; Lock=TypeProviderLock() },tp) ]
147154

148155
member this.OfType<'U> () =
149156
match box value with

src/fsharp/tainted.fsi

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,15 @@ open System.Reflection
1010
open Microsoft.FSharp.Core.CompilerServices
1111
open FSharp.Compiler.Range
1212
open FSharp.Compiler.AbstractIL.IL
13+
open FSharp.Compiler.AbstractIL.Internal.Library
14+
15+
[<Sealed>]
16+
type internal TypeProviderToken =
17+
interface LockToken
18+
19+
[<Sealed;Class>]
20+
type internal TypeProviderLock =
21+
inherit Lock<TypeProviderToken>
1322

1423
/// Stores and transports aggregated list of errors reported by the type provider
1524
type internal TypeProviderError =

0 commit comments

Comments
 (0)