[GitHub] [spark] srowen commented on a change in pull request #24085: [SPARK-26555][SQL] make ScalaReflection subtype checking thread safe

[GitBox <gi...@apache.org>]

srowen commented on a change in pull request #24085: [SPARK-26555][SQL] make ScalaReflection subtype checking thread safe
URL: https://github.com/apache/spark/pull/24085#discussion_r265727789
 
 

 ##########
 File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/ScalaReflection.scala
 ##########
 @@ -66,19 +66,25 @@ object ScalaReflection extends ScalaReflection {
    */
   def dataTypeFor[T : TypeTag]: DataType = dataTypeFor(localTypeOf[T])
 
+  private[catalyst] def isSubtype(tpe1: `Type`, tpe2: `Type`): Boolean = {
+    this.synchronized {
+      tpe1 <:< tpe2
 
 Review comment:
   Yeah, that's the problem, I would bet this creates some thread contention bottlenecks. It's a problem although somewhat niche w.r.t. Spark as I'm not sure you'd perform operations in parallel, usually, that called this code. It's more of a Scala bug; lightweight workarounds on the Spark end are OK but I'm unsure if this, even if it's the whole fix, is worth the contention. That said it's only an educated guess.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

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