Skip to content

Commit

Permalink
addressed review comments
Browse files Browse the repository at this point in the history
Signed-off-by: Raza Jafri <[email protected]>
  • Loading branch information
razajafri committed Feb 25, 2022
1 parent 980eba3 commit a2a6b7e
Show file tree
Hide file tree
Showing 17 changed files with 37 additions and 93 deletions.
2 changes: 1 addition & 1 deletion jenkins/spark-premerge-build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ rapids_shuffle_smoke_test() {
PYSP_TEST_spark_cores_max=2 \
PYSP_TEST_spark_executor_cores=1 \
SPARK_SUBMIT_FLAGS="--conf spark.executorEnv.UCX_ERROR_SIGNALS=" \
PYSP_TEST_spark_shuffle_manager=com.nvidia.spark.rapids.shims.RapidsShuffleManager \
PYSP_TEST_spark_shuffle_manager=com.nvidia.spark.rapids.RapidsShuffleManager \
PYSP_TEST_spark_rapids_memory_gpu_minAllocFraction=0 \
PYSP_TEST_spark_rapids_memory_gpu_maxAllocFraction=0.1 \
PYSP_TEST_spark_rapids_memory_gpu_allocFraction=0.1 \
Expand Down
20 changes: 12 additions & 8 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@
<source>${project.basedir}/src/main/301+-nondb/scala</source>
<source>${project.basedir}/src/main/301/scala</source>
<source>${project.basedir}/src/main/301until304/scala</source>
<source>${project.basedir}/src/main/30X-32X-33X/scala</source>
<source>${project.basedir}/src/main/30X/scala</source>
<source>${project.basedir}/src/main/301until310-all/scala</source>
<source>${project.basedir}/src/main/301until310-nondb/scala</source>
<source>${project.basedir}/src/main/301until320-all/scala</source>
Expand Down Expand Up @@ -167,7 +167,7 @@
<source>${project.basedir}/src/main/301+-nondb/scala</source>
<source>${project.basedir}/src/main/302/scala</source>
<source>${project.basedir}/src/main/301until304/scala</source>
<source>${project.basedir}/src/main/30X-32X-33X/scala</source>
<source>${project.basedir}/src/main/30X/scala</source>
<source>${project.basedir}/src/main/301until310-all/scala</source>
<source>${project.basedir}/src/main/301until310-nondb/scala</source>
<source>${project.basedir}/src/main/301until320-all/scala</source>
Expand Down Expand Up @@ -227,7 +227,7 @@
<source>${project.basedir}/src/main/301+-nondb/scala</source>
<source>${project.basedir}/src/main/303/scala</source>
<source>${project.basedir}/src/main/301until304/scala</source>
<source>${project.basedir}/src/main/30X-32X-33X/scala</source>
<source>${project.basedir}/src/main/30X/scala</source>
<source>${project.basedir}/src/main/301until310-all/scala</source>
<source>${project.basedir}/src/main/301until310-nondb/scala</source>
<source>${project.basedir}/src/main/301until320-all/scala</source>
Expand Down Expand Up @@ -281,7 +281,7 @@
<sources>
<source>${project.basedir}/src/main/301+-nondb/scala</source>
<source>${project.basedir}/src/main/304/scala</source>
<source>${project.basedir}/src/main/30X-32X-33X/scala</source>
<source>${project.basedir}/src/main/30X/scala</source>
<source>${project.basedir}/src/main/301until310-all/scala</source>
<source>${project.basedir}/src/main/301until310-nondb/scala</source>
<source>${project.basedir}/src/main/301until320-all/scala</source>
Expand Down Expand Up @@ -335,6 +335,7 @@
<sources>
<source>${project.basedir}/src/main/301+-nondb/scala</source>
<source>${project.basedir}/src/main/311-nondb/scala</source>
<source>${project.basedir}/src/main/31X-33X/scala</source>
<source>${project.basedir}/src/main/301until320-all/scala</source>
<source>${project.basedir}/src/main/301until320-noncdh/scala</source>
<source>${project.basedir}/src/main/301until320-nondb/scala</source>
Expand Down Expand Up @@ -517,6 +518,7 @@
<sources>
<source>${project.basedir}/src/main/301+-nondb/scala</source>
<source>${project.basedir}/src/main/312-nondb/scala</source>
<source>${project.basedir}/src/main/31X-33X/scala</source>
<source>${project.basedir}/src/main/301until320-all/scala</source>
<source>${project.basedir}/src/main/301until320-noncdh/scala</source>
<source>${project.basedir}/src/main/301until320-nondb/scala</source>
Expand Down Expand Up @@ -575,6 +577,7 @@
<sources>
<source>${project.basedir}/src/main/301+-nondb/scala</source>
<source>${project.basedir}/src/main/313/scala</source>
<source>${project.basedir}/src/main/31X-33X/scala</source>
<source>${project.basedir}/src/main/301until320-all/scala</source>
<source>${project.basedir}/src/main/301until320-noncdh/scala</source>
<source>${project.basedir}/src/main/301until320-nondb/scala</source>
Expand Down Expand Up @@ -632,7 +635,7 @@
<configuration>
<sources>
<source>${project.basedir}/src/main/301+-nondb/scala</source>
<source>${project.basedir}/src/main/30X-32X-33X/scala</source>
<source>${project.basedir}/src/main/31X-33X/scala</source>
<source>${project.basedir}/src/main/320/scala</source>
<source>${project.basedir}/src/main/301until330-all/scala</source>
<source>${project.basedir}/src/main/311+-all/scala</source>
Expand Down Expand Up @@ -696,7 +699,7 @@
<configuration>
<sources>
<source>${project.basedir}/src/main/301+-nondb/scala</source>
<source>${project.basedir}/src/main/30X-32X-33X/scala</source>
<source>${project.basedir}/src/main/31X-33X/scala</source>
<source>${project.basedir}/src/main/321/scala</source>
<source>${project.basedir}/src/main/301until330-all/scala</source>
<source>${project.basedir}/src/main/311+-all/scala</source>
Expand Down Expand Up @@ -759,7 +762,7 @@
<configuration>
<sources>
<source>${project.basedir}/src/main/301+-nondb/scala</source>
<source>${project.basedir}/src/main/30X-32X-33X/scala</source>
<source>${project.basedir}/src/main/31X-33X/scala</source>
<source>${project.basedir}/src/main/322/scala</source>
<source>${project.basedir}/src/main/301until330-all/scala</source>
<source>${project.basedir}/src/main/311+-all/scala</source>
Expand Down Expand Up @@ -824,7 +827,7 @@
<configuration>
<sources>
<source>${project.basedir}/src/main/301+-nondb/scala</source>
<source>${project.basedir}/src/main/30X-32X-33X/scala</source>
<source>${project.basedir}/src/main/31X-33X/scala</source>
<source>${project.basedir}/src/main/330/scala</source>
<source>${project.basedir}/src/main/311+-all/scala</source>
<source>${project.basedir}/src/main/311+-nondb/scala</source>
Expand Down Expand Up @@ -888,6 +891,7 @@
<configuration>
<sources>
<source>${project.basedir}/src/main/301+-nondb/scala</source>
<source>${project.basedir}/src/main/31X-33X/scala</source>
<source>${project.basedir}/src/main/311-nondb/scala</source>
<source>${project.basedir}/src/main/311cdh/scala</source>
<source>${project.basedir}/src/main/301until320-all/scala</source>
Expand Down
6 changes: 3 additions & 3 deletions scripts/spark2diffs/GpuJoinUtils.diff
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
16,18d15
< package com.nvidia.spark.rapids.shims
< package com.nvidia.spark.rapids.shims.v2
<
< import com.nvidia.spark.rapids.shims._
< import com.nvidia.spark.rapids.shims.v2._
20,26c17
< import org.apache.spark.sql.execution.joins.{BuildLeft, BuildRight, BuildSide}
<
Expand All @@ -11,7 +11,7 @@
< */
< sealed abstract class GpuBuildSide
---
> package com.nvidia.spark.rapids.shims
> package com.nvidia.spark.rapids.shims.v2
28c19
< case object GpuBuildRight extends GpuBuildSide
---
Expand Down
6 changes: 3 additions & 3 deletions scripts/spark2diffs/GpuOverrides.diff
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
24a25
> import ai.rapids.cudf.DType
26c27
< import com.nvidia.spark.rapids.shims._
< import com.nvidia.spark.rapids.shims.v2._
---
> import com.nvidia.spark.rapids.shims.{AQEUtils, GpuHashPartitioning, GpuSpecifiedWindowFrameMeta, GpuWindowExpressionMeta, OffsetWindowFunctionMeta}
> import com.nvidia.spark.rapids.shims.v2.{AQEUtils, GpuHashPartitioning, GpuSpecifiedWindowFrameMeta, GpuWindowExpressionMeta, OffsetWindowFunctionMeta}
31a33,35
> import org.apache.spark.sql.catalyst.expressions.rapids.TimeStamp
> import org.apache.spark.sql.catalyst.json.rapids.GpuJsonScan
Expand All @@ -32,7 +32,7 @@
> import org.apache.spark.sql.rapids.catalyst.expressions.GpuRand
50a62,63
> import org.apache.spark.sql.rapids.execution.python._
> import org.apache.spark.sql.rapids.shims.GpuTimeAdd
> import org.apache.spark.sql.rapids.shims.v2.GpuTimeAdd
63c76
< abstract class ReplacementRule[INPUT <: BASE, BASE, WRAP_TYPE <: RapidsMeta[INPUT, BASE]](
---
Expand Down
4 changes: 2 additions & 2 deletions scripts/spark2diffs/GpuSortMergeJoinMeta.diff
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@
---
> * Copyright (c) 2019-2022, NVIDIA CORPORATION.
17,20c17
< package com.nvidia.spark.rapids.shims
< package com.nvidia.spark.rapids.shims.v2
<
< import com.nvidia.spark.rapids._
< import com.nvidia.spark.rapids.shims._
< import com.nvidia.spark.rapids.shims.v2._
---
> package com.nvidia.spark.rapids
29c26
Expand Down
2 changes: 1 addition & 1 deletion scripts/spark2diffs/aggregate.diff
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
> import ai.rapids.cudf.NvtxColor
> import com.nvidia.spark.rapids.GpuMetric._
> import com.nvidia.spark.rapids.RapidsPluginImplicits._
> import com.nvidia.spark.rapids.shims.ShimUnaryExecNode
> import com.nvidia.spark.rapids.shims.v2.ShimUnaryExecNode
>
> import org.apache.spark.TaskContext
> import org.apache.spark.internal.Logging
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@

package com.nvidia.spark.rapids.shims.spark302

import com.nvidia.spark.rapids.{SparkShims, SparkShimVersion}
import com.nvidia.spark.rapids.shims.SparkShimImpl
import com.nvidia.spark.rapids.SparkShimVersion

object SparkShimServiceProvider {
val VERSION = SparkShimVersion(3, 0, 2)
Expand All @@ -30,5 +29,4 @@ class SparkShimServiceProvider extends com.nvidia.spark.rapids.SparkShimServiceP
def matchesVersion(version: String): Boolean = {
SparkShimServiceProvider.VERSIONNAMES.contains(version)
}

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020-2022, NVIDIA CORPORATION.
* Copyright (c) 2021-2022, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ object SparkShimServiceProvider {

class SparkShimServiceProvider extends com.nvidia.spark.rapids.SparkShimServiceProvider {

override def getShimVersion: SparkShimVersion = SparkShimServiceProvider.VERSION

def matchesVersion(version: String): Boolean = {
SparkShimServiceProvider.VERSIONNAMES.contains(version)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

package com.nvidia.spark.rapids.shims.spark311cdh

import com.nvidia.spark.rapids.{ClouderaShimVersion, SparkShimVersion}
import com.nvidia.spark.rapids.ClouderaShimVersion

object SparkShimServiceProvider {
val VERSION = ClouderaShimVersion(3, 1, 1, "3.1.7270")
Expand All @@ -26,7 +26,7 @@ object SparkShimServiceProvider {

class SparkShimServiceProvider extends com.nvidia.spark.rapids.SparkShimServiceProvider {

override def getShimVersion: SparkShimVersion = SparkShimServiceProvider.VERSION
override def getShimVersion: ClouderaShimVersion = SparkShimServiceProvider.VERSION

def matchesVersion(version: String): Boolean = {
version.contains(SparkShimServiceProvider.CDH_BASE_VERSION)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020-2022, NVIDIA CORPORATION.
* Copyright (c) 2021-2022, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019-2022, NVIDIA CORPORATION.
* Copyright (c) 2021-2022, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
package com.nvidia.spark.rapids.shims

import com.nvidia.spark.rapids._
import com.nvidia.spark.rapids.shims.spark321.SparkShimServiceProvider

import org.apache.spark.rdd.RDD
import org.apache.spark.sql.SparkSession
Expand All @@ -27,7 +26,8 @@ import org.apache.spark.sql.execution.datasources.{FilePartition, FileScanRDD, P
import org.apache.spark.sql.types.StructType

object SparkShimImpl extends Spark321PlusShims with Spark30Xuntil33XShims {
override def getSparkShimVersion: ShimVersion = SparkShimServiceProvider.VERSION

override def getSparkShimVersion: ShimVersion = ShimLoader.getShimVersion

override def getFileScanRDD(
sparkSession: SparkSession,
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,13 @@ import org.apache.spark.util.MutableURLClassLoader
E.g., Spark 3.2.0 Shim will use
jar:file:/home/spark/rapids-4-spark_2.12-22.02.0.jar!/spark3xx-common/
jar:file:/home/spark/rapids-4-spark_2.12-22.02.0.jar!/spark320/
jar:file:/home/spark/rapids-4-spark_2.12-22.04.0.jar!/spark3xx-common/
jar:file:/home/spark/rapids-4-spark_2.12-22.04.0.jar!/spark320/
Spark 3.1.1 will use
jar:file:/home/spark/rapids-4-spark_2.12-22.02.0.jar!/spark3xx-common/
jar:file:/home/spark/rapids-4-spark_2.12-22.02.0.jar!/spark311/
jar:file:/home/spark/rapids-4-spark_2.12-22.04.0.jar!/spark3xx-common/
jar:file:/home/spark/rapids-4-spark_2.12-22.04.0.jar!/spark311/
Using these Jar URL's allows referencing different bytecode produced from identical sources
by incompatible Scala / Spark dependencies.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,6 @@ package com.nvidia.spark.rapids
* A Spark version shim layer interface.
*/
trait SparkShimServiceProvider {
def getShimVersion: SparkShimVersion
def getShimVersion: ShimVersion
def matchesVersion(version:String): Boolean
}

0 comments on commit a2a6b7e

Please sign in to comment.