rednaxelafx commented on code in PR #42995:
URL: https://github.com/apache/spark/pull/42995#discussion_r1332183182
##########
common/utils/src/main/scala/org/apache/spark/util/ClosureCleaner.scala:
##########
@@ -46,11 +42,24 @@ private[spark] object ClosureCleaner extends Logging {
null
} else {
val baos = new ByteArrayOutputStream(128)
- Utils.copyStream(resourceStream, baos, true)
+
+ SparkStreamUtils.copyStream(resourceStream, baos, closeStreams = true)
new ClassReader(new ByteArrayInputStream(baos.toByteArray))
}
}
+ private[util] def isAmmoniteCommandOrHelper(clazz: Class[_]): Boolean =
clazz.getName.matches(
+ "^ammonite\\.\\$sess\\.cmd[0-9]*(\\$Helper\\$?)?")
+
+ private[util] def isDefinedInAmmonite(clazz: Class[_]): Boolean =
clazz.getName.matches(
+ "^ammonite\\.\\$sess\\.cmd[0-9]*.*")
Review Comment:
Ditto
##########
common/utils/src/main/scala/org/apache/spark/util/ClosureCleaner.scala:
##########
@@ -893,10 +1101,12 @@ private class InnerClosureFinder(output: Set[Class[_]])
extends ClassVisitor(ASM
op: Int, owner: String, name: String, desc: String, itf: Boolean):
Unit = {
val argTypes = Type.getArgumentTypes(desc)
if (op == INVOKESPECIAL && name == "<init>" && argTypes.length > 0
- && argTypes(0).toString.startsWith("L") // is it an object?
- && argTypes(0).getInternalName == myName) {
- output += Utils.classForName(owner.replace('/', '.'),
- initialize = false, noSparkClassLoader = true)
+ && argTypes(0).toString.startsWith("L") // is it an object?
+ && argTypes(0).getInternalName == myName) {
Review Comment:
Is this format change necessary? I like the original better...
(If this is from scalafmt then I have no problem with it 🤷 )
##########
common/utils/src/main/scala/org/apache/spark/util/ClosureCleaner.scala:
##########
@@ -46,11 +42,24 @@ private[spark] object ClosureCleaner extends Logging {
null
} else {
val baos = new ByteArrayOutputStream(128)
- Utils.copyStream(resourceStream, baos, true)
+
+ SparkStreamUtils.copyStream(resourceStream, baos, closeStreams = true)
new ClassReader(new ByteArrayInputStream(baos.toByteArray))
}
}
+ private[util] def isAmmoniteCommandOrHelper(clazz: Class[_]): Boolean =
clazz.getName.matches(
+ "^ammonite\\.\\$sess\\.cmd[0-9]*(\\$Helper\\$?)?")
Review Comment:
Can we use Scala's triple quote string literal here to avoid the need for
the escaping?
i.e.
```scala
"""^ammonite\.\$sess\.cmd[0-9]*(\$Helper\$?)?"""
```
##########
common/utils/src/main/scala/org/apache/spark/util/ClosureCleaner.scala:
##########
@@ -893,10 +1101,12 @@ private class InnerClosureFinder(output: Set[Class[_]])
extends ClassVisitor(ASM
op: Int, owner: String, name: String, desc: String, itf: Boolean):
Unit = {
val argTypes = Type.getArgumentTypes(desc)
if (op == INVOKESPECIAL && name == "<init>" && argTypes.length > 0
- && argTypes(0).toString.startsWith("L") // is it an object?
- && argTypes(0).getInternalName == myName) {
- output += Utils.classForName(owner.replace('/', '.'),
- initialize = false, noSparkClassLoader = true)
+ && argTypes(0).toString.startsWith("L") // is it an object?
+ && argTypes(0).getInternalName == myName) {
Review Comment:
Is this format change necessary? I like the original better...
(If this is from scalafmt then I have no problem with it 🤷 )
##########
common/utils/src/main/scala/org/apache/spark/util/ClosureCleaner.scala:
##########
@@ -678,18 +851,45 @@ private[spark] object IndylambdaScalaClosures extends
Logging {
// Depth-first search for inner closures and track the fields that were
accessed in them.
// Start from the lambda body's implementation method, follow method
invocations
val visited = Set.empty[MethodIdentifier[_]]
- val stack = Stack[MethodIdentifier[_]](implMethodId)
+ val queue = Queue[MethodIdentifier[_]](implMethodId)
Review Comment:
Is this intended? What's the rationale for changing from stack => queue? Is
breadth-first search preferred here?
--
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]
