[QST] deserializeStream() will acquire gpu semaphore，but why serializeStream() do not release the gpu semaphore explicitly ?

[JustPlay]

spark-rapids/sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuColumnarBatchSerializer.scala

Line 49 in 1a2b17e

override def serializeStream(out: OutputStream): SerializationStream = new SerializationStream {

spark-rapids/sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuColumnarBatchSerializer.scala

Line 132 in 1a2b17e

override def deserializeStream(in: InputStream): DeserializationStream = {

deserializeStream() will acquire gpu semaphore，but why serializeStream() do not release the gpu semaphore explicitly ?

I think release the gpu semaphore explicitly will help increase cpu concurrenty for loading the next batches.

@jlowe @revans2

[revans2]

That is because it is already released.

spark-rapids/sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuPartitioning.scala

Line 85 in 1a2b17e

GpuSemaphore.releaseIfNecessary(TaskContext.get())

We should document it better and if you want to turn this into a documentation bug I would be happy to fix it. The shuffle code went through a number of iterations to get it to be performant. Sadly there is a lot of coupling between the serializer and the shuffle code and we need to document it better.

spark-rapids/sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuColumnarBatchSerializer.scala

Lines 73 to 76 in 1a2b17e

	case gpu: GpuColumnVector =>
	val cpu = gpu.copyToHost()
	toClose += cpu
	columns(i) = cpu.getBase

is the only code in the serializer that works with data still on the GPU. That code is likely dead code. I believe I left it in there just as a precaution, but it should be cleaned up a lot.

I hope that this helps.