Skip to content

Parallelize the Analyzer #4897

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.

Sign up for GitHub

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

Merged
merged 13 commits into from
Aug 28, 2023
Merged

Parallelize the Analyzer #4897

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
0f3ba71
Make class loading thread safe
gzm0 Jul 30, 2023
530a704
Make error collection thread-safe
gzm0 Aug 18, 2023
c857067
Make module dependency collection thread-safe
gzm0 Aug 19, 2023
6e9a0c9
Make topLevelExportInfo collection thread-safe
gzm0 Aug 19, 2023
9450a37
Make MethodInfo management thread-safe
gzm0 Aug 19, 2023
eb8f45b
Make from collection thread-safe
gzm0 Aug 19, 2023
d7d915a
Make jsNativeMember collection thread-safe
gzm0 Aug 19, 2023
4d8ffce
Make field collection thread-safe
gzm0 Aug 19, 2023
f7d6db5
Make reachability flags thread-safe
gzm0 Aug 19, 2023
355ca93
Make method dispatch thread-safe
gzm0 Aug 19, 2023
0da7a54
Parallel analyzer
gzm0 Aug 19, 2023
a28fa78
Rewrite computeMostSpecificProxyMatch
gzm0 Aug 19, 2023
9a0143c
Honor the `parallel` flag in the Analyzer
gzm0 Aug 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Honor the parallel flag in the Analyzer
  • Loading branch information
@gzm0
gzm0 committed Aug 27, 2023
commit 9a0143c655421deac343ef789fda322bbe9a32de
6 changes: 6 additions & 0 deletions linker/js/src/main/scala/org/scalajs/linker/analyzer/Platform.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,13 @@
package org.scalajs.linker.analyzer

import scala.collection.mutable
import scala.concurrent.ExecutionContext

private[analyzer] object Platform {
def emptyThreadSafeMap[K, V]: mutable.Map[K, V] = mutable.Map.empty

def adjustExecutionContextForParallelism(ec: ExecutionContext,
parallel: Boolean): ExecutionContext = {
ec // we're never parallel on JS
}
}
16 changes: 16 additions & 0 deletions linker/jvm/src/main/scala/org/scalajs/linker/analyzer/Platform.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,23 @@ package org.scalajs.linker.analyzer

import scala.collection.mutable
import scala.collection.concurrent.TrieMap
import scala.concurrent.ExecutionContext

import java.util.concurrent.Executors

private[analyzer] object Platform {
def emptyThreadSafeMap[K, V]: mutable.Map[K, V] = TrieMap.empty
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC, the TrieMap will not compact the removed entry, so the mememory usage will be lager than the ConcurrentHashMap, but the ConcurrentHashMap have a different trait.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't remove anything from these maps, so that doesn't seem like a relevant concern.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW: TrieMap supports lock-free, "opportunistic" getOrElseUpdate (might create the value multiple times, but won't if it's already in the map). ConcurrentHashMap doesn't (computeIfAbsent). That was the primary motivator to use TrieMap.

Copy link

@He-Pin He-Pin Aug 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I always use a get to check before invoking the computeIfAbsnet, and the bug you mentioned about ConcurrentHashMap has been fixed in JDK 9.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what you mean with "the bug I mention". The JavaDoc is very clear that computeIfAbsent can be blocking:

The entire method invocation is performed atomically, so the function is applied at most once per key. Some attempted update operations on this map by other threads may be blocked while computation is in progress, so the computation should be short and simple, and must not attempt to update any other mappings of this map.

While this can be desirable sometimes, it's not what we want here: We do not need the guarantee that the method invocation is performed atomically, but we do want the operation to remain lock-free.


def adjustExecutionContextForParallelism(ec: ExecutionContext,
parallel: Boolean): ExecutionContext = {
/* Parallel is the default. Parallelism is disabled (likely for debugging),
* we create our own single thread executor. This is for sure not the most
* efficient, but it is simpler than, say, attempting to build single thread
* execution on top of an arbitrary execution context.
* Further, if parallel is false, we do not expect that speed is the primary
* aim of the execution.
*/
if (parallel) ec
else ExecutionContext.fromExecutorService(Executors.newSingleThreadExecutor())
}
}
10 changes: 9 additions & 1 deletion linker/shared/src/main/scala/org/scalajs/linker/analyzer/Analyzer.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import org.scalajs.linker.standard.ModuleSet.ModuleID

import org.scalajs.logging._

import Platform.emptyThreadSafeMap
import Platform._

import Analysis._
import Infos.{NamespacedMethodName, ReachabilityInfo, ReachabilityInfoInClass}
Expand Down Expand Up @@ -74,6 +74,14 @@ final class Analyzer(config: CommonPhaseConfig, initial: Boolean,
def computeReachability(moduleInitializers: Seq[ModuleInitializer],
symbolRequirements: SymbolRequirement, logger: Logger)(implicit ec: ExecutionContext): Future[Analysis] = {

computeInternal(moduleInitializers, symbolRequirements, logger)(
adjustExecutionContextForParallelism(ec, config.parallel))
}

/** Internal helper to isolate the execution context. */
private def computeInternal(moduleInitializers: Seq[ModuleInitializer],
symbolRequirements: SymbolRequirement, logger: Logger)(implicit ec: ExecutionContext): Future[Analysis] = {

resetState()

infoLoader.update(logger)
Expand Down