Check that dotty is reentrant #708
Conversation
|
Succeeds running 8 threads on my machine in 4 min 15 sec, sequential time was 7 min 20 sec. |
odersky
force-pushed
the
add/check-reentrant
branch
from
1e322e4 to
f7ac8df
Compare
| */ | ||
| class CtxLazy[T](expr: Context => T) { | ||
| private var myValue: T = _ | ||
| private var forced = false |
|
@odersky, is this PR ready for review? |
|
Yes, please review. On Fri, Jul 3, 2015 at 11:31 AM, Dmitry Petrashko [email protected]
Martin Odersky |
| @@ -200,44 +203,21 @@ object Names { | |||
| private final val fillFactor = 0.7 | |||
|
|
|||
| /** Memory to store all names sequentially. */ | |||
| @sharable // because it's only mutated in synchronized block of termName | |||
There was a problem hiding this comment.
Keeping track and maintaining such annotation could be a burden.
Instead of annotating explicitly, could we infer that private fields that are modified in synchronized(this) blocks are sharable?
There was a problem hiding this comment.
Maybe at some point. But right now that's probably too much of a detour. Let's see first whether @sharable thing becomes a feature of general utility. Even now it serves a purpose: If we want to figure out data races, it's good to scrutinize members with @sharable annotations. For the others we can run -Ycheck:reentrant to rule them out (well, almost, modulo to the shortcomings mentioned in the comment).
|
Otherwise LGTM. |
New miniphase CheckRentrant verifies that compiled program is without vars accessible through global roots if -Ycheck-reentrant option is set. Known shortcoming: Array elements are currently not considered as vars. This is because in many programs arrays are used as an efficient container for immutable fields.
Add @sharable annotation for classes and vals that are presumed to be safely sharable between threads. Also: Document CtxLazy.
Some global roots were pointing to shared mutable state but were never used.
Some globally accessible vars were never updated; should be vals.
The resident compiler is not supposed to be called from multiple threads; mark as unshared to avoid spurious re-entrancy errors.
_nextId is used to set Symbol's id fields. That field is actually used for more than priunting. In LambdaLift, it determines Symbol ordering when constructing (tree-) sets of symbols. Instead of a thread-unsafe global counter, we not use existing infrastructure in ConetxtState.
Also, some code movements in Names to make it more obvious that mutating operations are only called from synchronized blocks.
and use ctx.log for other output.
The thread-shared value NoDenotation has a validity with run-id that got updated to the run-id of the current thread. This caused a partest failure with a "demanding denotation outside its range" error. We now treat NoDenotation specially in current, hoping this will avoid the condition.
Avoids the need of an @Unshared annotation. For now I leave the annotation in, maybe it will be useful later.
odersky
force-pushed
the
add/check-reentrant
branch
from
90e4d4f to
c7cc6d8
Compare
|
Rebased to master. |
Not quite done yet. But the basic checking is in place and a lot of false positives are eliminated. One true possible datarace involving Symbols.nextId was eliminated. partest afterwards runs OK in parallel on my machine.