You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/11/08 06:42:05 UTC
[GitHub] [spark] zsxwing opened a new pull request, #38556: [SPARK-41045][SQL]Pre-compute to eliminate ScalaReflection calls after deserializer is created
zsxwing opened a new pull request, #38556:
URL: https://github.com/apache/spark/pull/38556
### What changes were proposed in this pull request?
Currently when `ScalaReflection` returns a deserializer, for a few complex types, such as array, map, udt, etc, it creates functions that may still touch `ScalaReflection` after the deserializer is created.
`ScalaReflection` is a performance bottleneck for multiple threads as it holds multiple global locks. We can refactor `ScalaReflection.deserializerFor` to pre-compute everything that needs to touch `ScalaReflection` before creating the deserializer. After this, once the deserializer is created, it can be reused by multiple threads without touching `ScalaReflection.deserializerFor` any more and it will be much faster.
### Why are the changes needed?
Optimize `ScalaReflection.deserializerFor` to make deserializers faster under multiple threads.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
This is refactoring `deserializerFor` to optimize the code. Existing tests should already cover the correctness.
--
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
[GitHub] [spark] zsxwing commented on pull request #38556: [SPARK-41045][SQL]Pre-compute to eliminate ScalaReflection calls after deserializer is created
Posted by GitBox <gi...@apache.org>.
zsxwing commented on PR #38556:
URL: https://github.com/apache/spark/pull/38556#issuecomment-1307475937
Thanks! Merging to master
--
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
[GitHub] [spark] asfgit closed pull request #38556: [SPARK-41045][SQL]Pre-compute to eliminate ScalaReflection calls after deserializer is created
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #38556: [SPARK-41045][SQL]Pre-compute to eliminate ScalaReflection calls after deserializer is created
URL: https://github.com/apache/spark/pull/38556
--
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
[GitHub] [spark] cloud-fan commented on a diff in pull request #38556: [SPARK-41045][SQL]Pre-compute to eliminate ScalaReflection calls after deserializer is created
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on code in PR #38556:
URL: https://github.com/apache/spark/pull/38556#discussion_r1016217473
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/JavaTypeInference.scala:
##########
@@ -309,7 +307,7 @@ object JavaTypeInference {
dataType,
nullable = elementNullable,
newTypePath,
- (casted, typePath) => deserializerFor(et, casted, typePath))
+ deserializerFor(et, _, newTypePath))
Review Comment:
I think the changes in this file are just code cleanup. I'm fine with it as they are also related.
--
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
[GitHub] [spark] zsxwing commented on a diff in pull request #38556: [SPARK-41045][SQL]Pre-compute to eliminate ScalaReflection calls after deserializer is created
Posted by GitBox <gi...@apache.org>.
zsxwing commented on code in PR #38556:
URL: https://github.com/apache/spark/pull/38556#discussion_r1016847017
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/JavaTypeInference.scala:
##########
@@ -309,7 +307,7 @@ object JavaTypeInference {
dataType,
nullable = elementNullable,
newTypePath,
- (casted, typePath) => deserializerFor(et, casted, typePath))
+ deserializerFor(et, _, newTypePath))
Review Comment:
Yep. They are cleanup due to the change of `deserializerForWithNullSafetyAndUpcast` in `DeserializerBuildHelper.scala`
--
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