Github user kiszk commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19687#discussion_r149630182
  
    --- Diff: 
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/encoders/ExpressionEncoderSuite.scala
 ---
    @@ -441,4 +443,28 @@ class ExpressionEncoderSuite extends PlanTest with 
AnalysisTest {
           }
         }
       }
    +
    +  /**
    +   * Verify the size of scala.reflect.runtime.JavaUniverse.undoLog before 
and after `func` to
    +   * ensure we don't leak Scala reflection garbage.
    +   *
    +   * @see 
org.apache.spark.sql.catalyst.ScalaReflection.cleanUpReflectionObjects
    +   */
    +  private def verifyNotLeakingReflectionObjects[T](func: => T): T = {
    +    def undoLogSize: Int = {
    +      import scala.reflect.runtime.{JavaUniverse, universe}
    +      universe.asInstanceOf[JavaUniverse].undoLog.log.size
    +    }
    +
    +    val previousUndoLogSize = undoLogSize
    +    val r = func
    +    assert(previousUndoLogSize == undoLogSize)
    +    r
    +  }
    +
    +  private def testAndVerifyNotLeakingReflectionObjects(testName: 
String)(testFun: => Any) {
    +    test(testName) {
    +      verifyNotLeakingReflectionObjects(testFun)
    --- End diff --
    
    Do we have to call `verifyNotLeakingReflectionObjects` twice? One is to 
call `implicit`. The other is here.


---

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

Reply via email to