-
Notifications
You must be signed in to change notification settings - Fork 833
All parallel optimizations enabled #18998
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
❗ Release notes required
|
aed8407 to
ec081eb
Compare
Can I ignore the other three?
This one is fully green now.
Can I ignore the other three? This one is fully green now.
Yes, we can continue with just this one.
System.Exception : EVIL_PROVIDER_NamespaceName_Exception.err EVIL_PROVIDER_NamespaceName_Exception.bsl differ; "diff between [D:\a_work1円\s\artifacts\Temp\FSharp.Test.Utilities72306301円6278円ff13\typeProviders\negTests\EVIL_PROVIDER_NamespaceName_Exception.bsl] and [D:\a_work1円\s\artifacts\Temp\FSharp.Test.Utilities72306301円6278円ff13\typeProviders\negTests\EVIL_PROVIDER_NamespaceName_Exception.err]
diff at line 4
+
Unhandled Exception: System.NullReferenceException: Object reference not set to an instance of an object.
at FSharp.Compiler.CompilerImports.clo@1108-481.Invoke(TcImportsLockToken tcitok)
at Internal.Utilities.Library.Lock1.AcquireLock[a](FSharpFunc2 f)
at FSharp.Compiler.CompilerImports.TcImportsSafeDisposal.Finalize()
"
Type providers don't like parallel imports, another random fail in evil provider.
Type providers don't like parallel imports, another random fail in evil provider.
I wonder if it because of the evil provider, or if the compiler should not synchronizing access to type provider code for all of them. (I assume that at time of TP creation, nothing in the compiler was parallel and access to TP libraries was always sequential).
Type providers don't like parallel imports, another random fail in evil provider.
I wonder if it because of the
evilprovider, or if the compiler should not synchronizing access to type provider code for all of them. (I assume that at time of TP creation, nothing in the compiler was parallel and access to TP libraries was always sequential).
I suspect it was because of one missing lock 6b96083
There was a comment that this requires a lock, but the lock was not there.
Type providers don't like parallel imports, another random fail in evil provider.
I wonder if it because of the
evilprovider, or if the compiler should not synchronizing access to type provider code for all of them. (I assume that at time of TP creation, nothing in the compiler was parallel and access to TP libraries was always sequential).
Yes, the oldest thing I remember was the Reactor, that made everything sequential. But then there's this #10310
Ok this makes sense, all access to TP synchronized.
We cannot tell from the outside if TP internals are thread safe or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those are legit - the signature has them, and implementation does not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Luckily all of this is a no-op of sigFiles is empty (= most projects).
It does O(#sigFiles * #implFiles) new string allocations (for the interpolated string).
Do we need to be that liberate for the order?
I would have assumed that a sigFile can just look at idx+1, idx-1 in the overall files array;; and then compare spans of characters. Or are there test cases with more relaxed rules on the ordering?
We should aim to be
- no-op if no sig files
- linear if sig files present but correctly ordered
- and only worse for degenerate orderings.
(for regular compilation it does not matter, but design time scenario via transparent compiler is IIRC making use of the graph)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I'm not sure why it was like this instead of just comparing QualifiedNameOfFile.Id.
But this covers one edge case I was not aware of: fsi file does not need to directly precede the implementation, it just needs to come before. I can't remember out of my head but there are some public projects in the wild that do it that way.
EDIT because QualifiedNameOfFile is useless here.
I think Transparent Compiler caches the graphs, too. So the impact is not that bad.
Uh oh!
There was an error while loading. Please reload this page.
See tracking issue #18989
Merged #18985 #18986 #18990 to see if it all together still works.
TODO:
Some il baselines changed because of reordered methods.
This also mostly does not work with
deterministiccompilation.