PHILO-HE commented on code in PR #8996:
URL: https://github.com/apache/incubator-gluten/pull/8996#discussion_r1995688840
##########
gluten-core/src/main/scala/org/apache/gluten/initializer/CodedInputStreamClassInitializer.scala:
##########
@@ -16,27 +16,41 @@
*/
package org.apache.gluten.initializer
+import org.apache.spark.internal.Logging
+
import java.lang.reflect.Field
/**
* Pre-load the class instance for CodedInputStream and modify its
defaultRecursionLimit to avoid
* the limit is hit for deeply nested plans. This is based on the fact that
the same class loader is
* used to load this class in the program, then this modification will really
take effect.
*/
-object CodedInputStreamClassInitializer {
- {
+object CodedInputStreamClassInitializer extends Logging {
+ private val newDefaultRecursionLimit = 100000
+
+ def modifyDefaultRecursionLimitUnsafe: Unit = {
try {
// scalastyle:off classforname
- // Use the shaded class name.
val clazz: Class[_] =
-
Class.forName("org.apache.gluten.shaded.com.google.protobuf.CodedInputStream")
+ try {
+ // Use the shaded class name.
+
Class.forName("org.apache.gluten.shaded.com.google.protobuf.CodedInputStream")
+ } catch {
+ // The above class is shaded in final package phase (see
package/pom.xml).
+ // If ClassNotFoundException happens, e.g., in mvn test, load the
original class instead.
+ case _: ClassNotFoundException =>
+ Class.forName("com.google.protobuf.CodedInputStream")
+ }
// scalastyle:on classforname
val field: Field = clazz.getDeclaredField("defaultRecursionLimit")
field.setAccessible(true)
// Enlarge defaultRecursionLimit whose original value is 100.
- field.setInt(null, 100000)
+ field.setInt(null, newDefaultRecursionLimit)
+ logInfo(
+ s"The defaultRecursionLimit in protobuf has been increased to
$newDefaultRecursionLimit")
} catch {
- case e: Exception => e.printStackTrace()
+ case e: Exception =>
+ log.error("Failed to modify the DefaultRecursionLimit in protobuf: " +
e.getMessage)
Review Comment:
@zhztheplayer, if it's not modified, Gluten can still work well for most
cases. This modification is just for avoiding the protobuf representation
failure for deeply nested plans. So we can tolerate the exception for most
cases.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
