Skip to content

Commit 978e60e

Browse files
committed
Address maropu's new comments
1 parent b559a30 commit 978e60e

File tree

1 file changed

+22
-6
lines changed

1 file changed

+22
-6
lines changed

core/src/main/scala/org/apache/spark/util/ClosureCleaner.scala

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -552,13 +552,20 @@ private[spark] object IndylambdaScalaClosures extends Logging {
552552
// produces the following trace-level logs:
553553
// (slightly simplified:
554554
// - omitting the "ignoring ..." lines;
555-
// - "$iw" is actually "$line16.$read$$iw$$iw$$iw$$iw$$iw$$iw$$iw$$iw";
555+
// - "$iw" is actually "$line14.$read$$iw$$iw$$iw$$iw$$iw$$iw$$iw$$iw";
556556
// - "invokedynamic" lines are simplified to just show the name+desc, omitting the bsm info)
557557
// Cleaning indylambda closure: $anonfun$closure$1$adapted
558558
// scanning $iw.$anonfun$closure$1$adapted(L$iw;Ljava/lang/Object;)Lscala/collection/immutable/IndexedSeq;
559559
// found intra class call to $iw.$anonfun$closure$1(L$iw;I)Lscala/collection/immutable/IndexedSeq;
560560
// scanning $iw.$anonfun$closure$1(L$iw;I)Lscala/collection/immutable/IndexedSeq;
561561
// found inner class $iw$InnerFoo$1
562+
// found method innerClosure()Lscala/Function1;
563+
// found method $anonfun$innerClosure$2(L$iw$InnerFoo$1;I)Ljava/lang/String;
564+
// found method $anonfun$innerClosure$1(L$iw$InnerFoo$1;I)Lscala/collection/immutable/IndexedSeq;
565+
// found method <init>(L$iw;)V
566+
// found method $anonfun$innerClosure$2$adapted(L$iw$InnerFoo$1;Ljava/lang/Object;)Ljava/lang/String;
567+
// found method $anonfun$innerClosure$1$adapted(L$iw$InnerFoo$1;Ljava/lang/Object;)Lscala/collection/immutable/IndexedSeq;
568+
// found method $deserializeLambda$(Ljava/lang/invoke/SerializedLambda;)Ljava/lang/Object;
562569
// found call to outer $iw$InnerFoo$1.innerClosure()Lscala/Function1;
563570
// scanning $iw$InnerFoo$1.innerClosure()Lscala/Function1;
564571
// scanning $iw$InnerFoo$1.$deserializeLambda$(Ljava/lang/invoke/SerializedLambda;)Ljava/lang/Object;
@@ -627,10 +634,18 @@ private[spark] object IndylambdaScalaClosures extends Logging {
627634
val implMethodId = MethodIdentifier(
628635
implClass, lambdaProxy.getImplMethodName, lambdaProxy.getImplMethodSignature)
629636

630-
// The set of classes that we would consider following the calls into.
637+
// The set internal names of classes that we would consider following the calls into.
631638
// Candidates are: known outer class which happens to be the starting closure's impl class,
632639
// and all inner classes discovered below.
633-
val trackedClassesByInternalName = Map[String, Class[_]](implClassInternalName -> implClass)
640+
// Note that code in an inner class can make calls to methods in any of its enclosing classes,
641+
// e.g.
642+
// starting closure (in class T)
643+
// inner class A
644+
// inner class B
645+
// inner closure
646+
// we need to track calls from "inner closure" to outer classes relative to it (class T, A, B)
647+
// to better find and track field accesses.
648+
val trackedClassInternalNames = Set[String](implClassInternalName)
634649

635650
// Depth-first search for inner closures and track the fields that were accessed in them.
636651
// Start from the lambda body's implementation method, follow method invocations
@@ -677,16 +692,17 @@ private[spark] object IndylambdaScalaClosures extends Logging {
677692
// Discover inner classes.
678693
// This this the InnerClassFinder equivalent for inner classes, which still use the
679694
// `$outer` chain. So this is NOT controlled by the `findTransitively` flag.
680-
logTrace(s" found inner class $ownerExternalName")
695+
logDebug(s" found inner class $ownerExternalName")
681696
val innerClassInfo = getOrUpdateClassInfo(owner)
682697
val innerClass = innerClassInfo._1
683698
val innerClassNode = innerClassInfo._2
684-
trackedClassesByInternalName(owner) = innerClass
699+
trackedClassInternalNames += owner
685700
// We need to visit all methods on the inner class so that we don't missing anything.
686701
for (m <- innerClassNode.methods.asScala) {
702+
logTrace(s" found method ${m.name}${m.desc}")
687703
pushIfNotVisited(MethodIdentifier(innerClass, m.name, m.desc))
688704
}
689-
} else if (findTransitively && trackedClassesByInternalName.contains(owner)) {
705+
} else if (findTransitively && trackedClassInternalNames.contains(owner)) {
690706
logTrace(s" found call to outer $ownerExternalName.$name$desc")
691707
val (calleeClass, _) = getOrUpdateClassInfo(owner) // make sure MethodNodes are cached
692708
pushIfNotVisited(MethodIdentifier(calleeClass, name, desc))

0 commit comments

Comments
 (0)