Skip to content

Commit 1410746

Browse files
fangchenliHyukjinKwon
authored andcommitted
[SPARK-55118][CORE] Replace ASM Opcodes wildcard imports
### What changes were proposed in this pull request? Replace wildcard imports with explicit named imports and prefix constants with Opcodes. The same pattern was already used in `StubClassLoader.scala`. ### Why are the changes needed? When compiling Spark with Scala 3 (using coursier), ClosureCleaner.scala and ExecutorClassLoader.scala fail with a cyclic reference error. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Code compiled with Scala3. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Opus 4.5 Closes apache#53889 from fangchenli/scala3-asm-opcodes-fix. Authored-by: Fangchen Li <fangchen.li@outlook.com> Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>
1 parent e17d273 commit 1410746

File tree

2 files changed

+25
-26
lines changed

2 files changed

+25
-26
lines changed

common/utils/src/main/scala/org/apache/spark/util/ClosureCleaner.scala

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,7 @@ import java.lang.reflect.{Field, Modifier}
2424
import scala.collection.mutable.{Map, Queue, Set, Stack}
2525
import scala.jdk.CollectionConverters._
2626

27-
import org.apache.xbean.asm9.{ClassReader, ClassVisitor, Handle, MethodVisitor, Type}
28-
import org.apache.xbean.asm9.Opcodes._
27+
import org.apache.xbean.asm9.{ClassReader, ClassVisitor, Handle, MethodVisitor, Opcodes, Type}
2928
import org.apache.xbean.asm9.tree.{ClassNode, MethodNode}
3029

3130
import org.apache.spark.SparkException
@@ -671,7 +670,7 @@ private[spark] object IndylambdaScalaClosures extends Logging {
671670
* Returns true if both criteria above are met.
672671
*/
673672
def isLambdaBodyCapturingOuter(handle: Handle, ownerInternalName: String): Boolean = {
674-
handle.getTag == H_INVOKESTATIC &&
673+
handle.getTag == Opcodes.H_INVOKESTATIC &&
675674
handle.getName.contains("$anonfun$") &&
676675
handle.getOwner == ownerInternalName &&
677676
handle.getDesc.startsWith(s"(L$ownerInternalName;")
@@ -686,7 +685,7 @@ private[spark] object IndylambdaScalaClosures extends Logging {
686685
*/
687686
def isInnerClassCtorCapturingOuter(
688687
op: Int, owner: String, name: String, desc: String, callerInternalName: String): Boolean = {
689-
op == INVOKESPECIAL && name == "<init>" && desc.startsWith(s"(L$callerInternalName;")
688+
op == Opcodes.INVOKESPECIAL && name == "<init>" && desc.startsWith(s"(L$callerInternalName;")
690689
}
691690

692691
/**
@@ -882,14 +881,14 @@ private[spark] object IndylambdaScalaClosures extends Logging {
882881
addAmmoniteCommandFieldsToTracking(currentClass)
883882
val currentMethodNode = methodNodeById(currentId)
884883
logTrace(s" scanning ${currentId.cls.getName}.${currentId.name}${currentId.desc}")
885-
currentMethodNode.accept(new MethodVisitor(ASM9) {
884+
currentMethodNode.accept(new MethodVisitor(Opcodes.ASM9) {
886885
val currentClassName = currentClass.getName
887886
val currentClassInternalName = currentClassName.replace('.', '/')
888887

889888
// Find and update the accessedFields info. Only fields on known outer classes are tracked.
890889
// This is the FieldAccessFinder equivalent.
891890
override def visitFieldInsn(op: Int, owner: String, name: String, desc: String): Unit = {
892-
if (op == GETFIELD || op == PUTFIELD) {
891+
if (op == Opcodes.GETFIELD || op == Opcodes.PUTFIELD) {
893892
val ownerExternalName = owner.replace('/', '.')
894893
for (cl <- accessedFields.keys if cl.getName == ownerExternalName) {
895894
logTrace(s" found field access $name on $ownerExternalName")
@@ -970,7 +969,7 @@ private[spark] class ReturnStatementInClosureException
970969
extends SparkException("Return statements aren't allowed in Spark closures")
971970

972971
private class ReturnStatementFinder(targetMethodName: Option[String] = None)
973-
extends ClassVisitor(ASM9) {
972+
extends ClassVisitor(Opcodes.ASM9) {
974973
override def visitMethod(access: Int, name: String, desc: String,
975974
sig: String, exceptions: Array[String]): MethodVisitor = {
976975

@@ -984,15 +983,16 @@ private class ReturnStatementFinder(targetMethodName: Option[String] = None)
984983
val isTargetMethod = targetMethodName.isEmpty ||
985984
name == targetMethodName.get || name == targetMethodName.get.stripSuffix("$adapted")
986985

987-
new MethodVisitor(ASM9) {
986+
new MethodVisitor(Opcodes.ASM9) {
988987
override def visitTypeInsn(op: Int, tp: String): Unit = {
989-
if (op == NEW && tp.contains("scala/runtime/NonLocalReturnControl") && isTargetMethod) {
988+
if (op == Opcodes.NEW && tp.contains("scala/runtime/NonLocalReturnControl") &&
989+
isTargetMethod) {
990990
throw new ReturnStatementInClosureException
991991
}
992992
}
993993
}
994994
} else {
995-
new MethodVisitor(ASM9) {}
995+
new MethodVisitor(Opcodes.ASM9) {}
996996
}
997997
}
998998
}
@@ -1016,7 +1016,7 @@ private[util] class FieldAccessFinder(
10161016
findTransitively: Boolean,
10171017
specificMethod: Option[MethodIdentifier[_]] = None,
10181018
visitedMethods: Set[MethodIdentifier[_]] = Set.empty)
1019-
extends ClassVisitor(ASM9) {
1019+
extends ClassVisitor(Opcodes.ASM9) {
10201020

10211021
override def visitMethod(
10221022
access: Int,
@@ -1031,9 +1031,9 @@ private[util] class FieldAccessFinder(
10311031
return null
10321032
}
10331033

1034-
new MethodVisitor(ASM9) {
1034+
new MethodVisitor(Opcodes.ASM9) {
10351035
override def visitFieldInsn(op: Int, owner: String, name: String, desc: String): Unit = {
1036-
if (op == GETFIELD) {
1036+
if (op == Opcodes.GETFIELD) {
10371037
for (cl <- fields.keys if cl.getName == owner.replace('/', '.')) {
10381038
fields(cl) += name
10391039
}
@@ -1045,7 +1045,7 @@ private[util] class FieldAccessFinder(
10451045
for (cl <- fields.keys if cl.getName == owner.replace('/', '.')) {
10461046
// Check for calls a getter method for a variable in an interpreter wrapper object.
10471047
// This means that the corresponding field will be accessed, so we should save it.
1048-
if (op == INVOKEVIRTUAL && owner.endsWith("$iwC") && !name.endsWith("$outer")) {
1048+
if (op == Opcodes.INVOKEVIRTUAL && owner.endsWith("$iwC") && !name.endsWith("$outer")) {
10491049
fields(cl) += name
10501050
}
10511051
// Optionally visit other methods to find fields that are transitively referenced
@@ -1071,7 +1071,7 @@ private[util] class FieldAccessFinder(
10711071
}
10721072
}
10731073

1074-
private class InnerClosureFinder(output: Set[Class[_]]) extends ClassVisitor(ASM9) {
1074+
private class InnerClosureFinder(output: Set[Class[_]]) extends ClassVisitor(Opcodes.ASM9) {
10751075
var myName: String = null
10761076

10771077
// TODO: Recursively find inner closures that we indirectly reference, e.g.
@@ -1086,11 +1086,11 @@ private class InnerClosureFinder(output: Set[Class[_]]) extends ClassVisitor(ASM
10861086

10871087
override def visitMethod(access: Int, name: String, desc: String,
10881088
sig: String, exceptions: Array[String]): MethodVisitor = {
1089-
new MethodVisitor(ASM9) {
1089+
new MethodVisitor(Opcodes.ASM9) {
10901090
override def visitMethodInsn(
10911091
op: Int, owner: String, name: String, desc: String, itf: Boolean): Unit = {
10921092
val argTypes = Type.getArgumentTypes(desc)
1093-
if (op == INVOKESPECIAL && name == "<init>" && argTypes.length > 0
1093+
if (op == Opcodes.INVOKESPECIAL && name == "<init>" && argTypes.length > 0
10941094
&& argTypes(0).toString.startsWith("L") // is it an object?
10951095
&& argTypes(0).getInternalName == myName) {
10961096
output += SparkClassUtils.classForName(

core/src/main/scala/org/apache/spark/executor/ExecutorClassLoader.scala

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,7 @@ import java.nio.charset.StandardCharsets.UTF_8
2525
import scala.util.control.NonFatal
2626

2727
import org.apache.hadoop.fs.{FileSystem, Path}
28-
import org.apache.xbean.asm9._
29-
import org.apache.xbean.asm9.Opcodes._
28+
import org.apache.xbean.asm9.{ClassReader, ClassVisitor, ClassWriter, MethodVisitor, Opcodes}
3029

3130
import org.apache.spark.{SparkConf, SparkEnv}
3231
import org.apache.spark.deploy.SparkHadoopUtil
@@ -238,21 +237,21 @@ class ExecutorClassLoader(
238237
}
239238

240239
class ConstructorCleaner(className: String, cv: ClassVisitor)
241-
extends ClassVisitor(ASM9, cv) {
240+
extends ClassVisitor(Opcodes.ASM9, cv) {
242241
override def visitMethod(access: Int, name: String, desc: String,
243242
sig: String, exceptions: Array[String]): MethodVisitor = {
244243
val mv = cv.visitMethod(access, name, desc, sig, exceptions)
245-
if (name == "<init>" && (access & ACC_STATIC) == 0) {
244+
if (name == "<init>" && (access & Opcodes.ACC_STATIC) == 0) {
246245
// This is the constructor, time to clean it; just output some new
247246
// instructions to mv that create the object and set the static MODULE$
248247
// field in the class to point to it, but do nothing otherwise.
249248
mv.visitCode()
250-
mv.visitVarInsn(ALOAD, 0) // load this
251-
mv.visitMethodInsn(INVOKESPECIAL, "java/lang/Object", "<init>", "()V", false)
252-
mv.visitVarInsn(ALOAD, 0) // load this
249+
mv.visitVarInsn(Opcodes.ALOAD, 0) // load this
250+
mv.visitMethodInsn(Opcodes.INVOKESPECIAL, "java/lang/Object", "<init>", "()V", false)
251+
mv.visitVarInsn(Opcodes.ALOAD, 0) // load this
253252
// val classType = className.replace('.', '/')
254-
// mv.visitFieldInsn(PUTSTATIC, classType, "MODULE$", "L" + classType + ";")
255-
mv.visitInsn(RETURN)
253+
// mv.visitFieldInsn(Opcodes.PUTSTATIC, classType, "MODULE$", "L" + classType + ";")
254+
mv.visitInsn(Opcodes.RETURN)
256255
mv.visitMaxs(-1, -1) // stack size and local vars will be auto-computed
257256
mv.visitEnd()
258257
null

0 commit comments

Comments
 (0)