Skip to content
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

[WIP] ShimLoader.updateSparkClassLoader fails with openjdk Java11 #5356

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
32 changes: 13 additions & 19 deletions sql-plugin/src/main/scala/com/nvidia/spark/rapids/ShimLoader.scala
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import org.apache.spark.internal.Logging
import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan
import org.apache.spark.sql.catalyst.rules.Rule
import org.apache.spark.sql.execution.{ColumnarRule, SparkPlan}
import org.apache.spark.util.MutableURLClassLoader
import org.apache.spark.util.{MutableURLClassLoader, ParentClassLoader}

/*
Plugin jar uses non-standard class file layout. It consists of three types of areas,
Expand Down Expand Up @@ -209,24 +209,18 @@ object ShimLoader extends Logging {
private def updateSparkClassLoader(): Unit = {
// TODO propose a proper addClassPathURL API to Spark similar to addJar but
// accepting non-file-based URI
serializerClassloader().foreach { urlAddable =>
ekrivokonmapr marked this conversation as resolved.
Show resolved Hide resolved
logInfo(s"Updating spark classloader $urlAddable with the URLs: " +
urlsForSparkClassLoader.mkString(", "))
urlsForSparkClassLoader.foreach { url =>
MethodUtils.invokeMethod(urlAddable, true, "addURL", url)
}
logInfo(s"Spark classLoader $urlAddable updated successfully")
urlAddable match {
case urlCl: java.net.URLClassLoader =>
if (!urlCl.getURLs.contains(shimCommonURL)) {
// infeasible, defensive diagnostics
logWarning(s"Didn't find expected URL $shimCommonURL in the spark " +
s"classloader $urlCl although addURL succeeded, maybe pushed up to the " +
s"parent classloader ${urlCl.getParent}")
}
case _ => ()
}
pluginClassLoader = urlAddable
val contextClassLoader = Thread.currentThread().getContextClassLoader
Option(contextClassLoader).collect {
case mutable: MutableURLClassLoader => mutable
case replCL if replCL.getClass.getName == "org.apache.spark.repl.ExecutorClassLoader" =>
val parentLoaderField = replCL.getClass.getDeclaredMethod("parentLoader")
val parentLoader = parentLoaderField.invoke(replCL).asInstanceOf[ParentClassLoader]
parentLoader.getParent.asInstanceOf[MutableURLClassLoader]
}.foreach { mutable =>
// MutableURLClassloader dedupes for us
pluginClassLoader = contextClassLoader
mutable.addURL(shimURL)
mutable.addURL(shimCommonURL)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ object RapidsShuffleTestHelper extends MockitoSugar with Arm {
}

def withMockContiguousTable[T](numRows: Long)(body: ContiguousTable => T): T = {
val rows: Seq[Integer] = (0 until numRows.toInt).map(new Integer(_))
val rows: Seq[Integer] = (0 until numRows.toInt).map(Integer.valueOf)
ekrivokonmapr marked this conversation as resolved.
Show resolved Hide resolved
withResource(ColumnVector.fromBoxedInts(rows:_*)) { cvBase =>
cvBase.incRefCount()
val gpuCv = GpuColumnVector.from(cvBase, IntegerType)
Expand Down