With a single row GpuExplode tries to split the generator array #10131
Conversation
Signed-off-by: Alessandro Bellina <abellina@nvidia.com>
|
build |
|
build |
sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuGenerateExec.scala
Outdated
Show resolved
Hide resolved
sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuGenerateExec.scala
Outdated
Show resolved
Hide resolved
sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuGenerateExec.scala
Outdated
Show resolved
Hide resolved
sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuGenerateExec.scala
Outdated
Show resolved
Hide resolved
| withResource(tbl.getColumn(col).getScalarElement(0)) { sc => | ||
| ColumnVector.fromScalar(sc, 1) | ||
| } |
There was a problem hiding this comment.
Seems like we're getting the first (and only) row of a column and then building a new column from that. Why isn't this just incref of the original table column without copying anything?
| colToReconstitute.reduce( | ||
| ReductionAggregation.collectList(), DType.LIST)) { sc => | ||
| ColumnVector.fromScalar(sc, 1) |
There was a problem hiding this comment.
This seems like a roundabout way to build a list column. We already have the child column, we just need a simple offset vector of two rows, 0 and child column size, and then build a list column from that.
|
One other consideration would be how this change performs wrt to the CPU and what(if) we need to change any recommendations to get close to or better than the CPU perf. |
|
build |
|
build |
| @@ -367,7 +371,7 @@ abstract class GpuExplodeBase extends GpuUnevaluableUnaryExpression with GpuGene | |||
| val inputRows = inputBatch.numRows() | |||
|
|
|||
| // if the number of input rows is 1 or less, cannot split | |||
| if (inputRows <= 1) return Array() | |||
| //if (inputRows <= 1) return Array() | |||
There was a problem hiding this comment.
I would love to see us push inputSplitIndices into the individual generate expressions. But that can be follow on as it really is just tech debt.
Also why is the row commented out?
There was a problem hiding this comment.
Nice catch, that should not be commented out.
I'll look into the inputSplitIndices refactor.
There was a problem hiding this comment.
Filed this for the refactor of inputSplitIndices and more... #10187. I don't want to block this issue further.
| inputTable.explodeOuterPosition(genOffset) | ||
| } else { | ||
| inputTable.explodePosition(genOffset) | ||
| } | ||
| // the position column is at genOffset and the exploded column at genOffset + 1 | ||
| val posColToFix = toFixUp.getColumn(genOffset) |
There was a problem hiding this comment.
nit toFixUp needs to either be under withResource, or at least a closeOnException.
|
build |
|
Note that the integration tests don't exercise the split path because it isn't setup correctly. I am going to punt on split integration tests. I do have unit tests in this PR that exercise the path. Filed this as follow on work: #10185 |
|
@revans2 @jlowe this is ready for another look. @kuhushukla the approach taken here is purely on retry and we are recovering from the cuDF column size limit exception. In order to make this perform better, we can change our size calculation to account for the cuDF column size limit up front. Lets add this as part of a refactor of the generator split code. That code only knows how to split by rows right now and that was a bigger change, so we decided to do the retry way instead. If we can split the input not just by rows, but within rows (before we retry), we can generalize some of this code and avoid several retry attempts. |
sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuGenerateExec.scala
Outdated
Show resolved
Hide resolved
|
build |
Closes #10088
The following PR adds special handling in
GpuGenerateExecwhere a single row (cols + list to split) could be split in case of OOM.The approach is very simple and leads to several retries, we simply split the column we are exploding (this would be a single list), in two halves. These are then attempted again, and following split and retry semantics, they can be split again until we are either successful, or the exploding column has 1 entry (we cannot split a single row with 1 array element).