You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "ueshin (via GitHub)" <gi...@apache.org> on 2023/10/10 23:10:51 UTC

[PR] [SPARK-45494][PYTHON] Introduce read/write a byte array util functions for PythonWorkerUtils [spark]

ueshin opened a new pull request, #43321:
URL: https://github.com/apache/spark/pull/43321

   ### What changes were proposed in this pull request?
   
   Introduce read/write a byte array util functions for `PythonWorkerUtils`.
   
   The following util functions will be added:
   
   - `writeBytes`
   - `readBytes`
   
   ### Why are the changes needed?
   
   To read/write a byte array also happens often.
   
   ### Does this PR introduce _any_ user-facing change?
   
   No.
   
   ### How was this patch tested?
   
   The existing tests.
   
   ### Was this patch authored or co-authored using generative AI tooling?
   
   No.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

Re: [PR] [SPARK-45494][PYTHON] Introduce util functions to read/write a byte array for PythonWorkerUtils [spark]

Posted by "allisonwang-db (via GitHub)" <gi...@apache.org>.
allisonwang-db commented on code in PR #43321:
URL: https://github.com/apache/spark/pull/43321#discussion_r1353752650


##########
core/src/main/scala/org/apache/spark/api/python/PythonRunner.scala:
##########
@@ -845,9 +843,7 @@ private[spark] class PythonRunner(
         try {
           stream.readInt() match {
             case length if length > 0 =>
-              val obj = new Array[Byte](length)
-              stream.readFully(obj)
-              obj
+              PythonWorkerUtils.readBytes(length, stream)
             case 0 => Array.emptyByteArray

Review Comment:
   Can we use `PythonWorkerUtils.readBytes` if we have `length=0` or we need to handle that separately?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

Re: [PR] [SPARK-45494][PYTHON] Introduce read/write a byte array util functions for PythonWorkerUtils [spark]

Posted by "ueshin (via GitHub)" <gi...@apache.org>.
ueshin commented on PR #43321:
URL: https://github.com/apache/spark/pull/43321#issuecomment-1756423607

   Also cc @dtenedor @WweiL


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

Re: [PR] [SPARK-45494][PYTHON] Introduce util functions to read/write a byte array for PythonWorkerUtils [spark]

Posted by "ueshin (via GitHub)" <gi...@apache.org>.
ueshin commented on code in PR #43321:
URL: https://github.com/apache/spark/pull/43321#discussion_r1353781026


##########
core/src/main/scala/org/apache/spark/api/python/PythonRunner.scala:
##########
@@ -845,9 +843,7 @@ private[spark] class PythonRunner(
         try {
           stream.readInt() match {
             case length if length > 0 =>
-              val obj = new Array[Byte](length)
-              stream.readFully(obj)
-              obj
+              PythonWorkerUtils.readBytes(length, stream)
             case 0 => Array.emptyByteArray

Review Comment:
   That's a good point. I think we can include this in `readBytes` to always avoid creating a new empty array.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

Re: [PR] [SPARK-45494][CORE][PYTHON] Introduce util functions to read/write a byte array for PythonWorkerUtils [spark]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun closed pull request #43321: [SPARK-45494][CORE][PYTHON] Introduce util functions to read/write a byte array for PythonWorkerUtils
URL: https://github.com/apache/spark/pull/43321


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org