Unnecessary reference duplicate in backend #23681
Description
Compiler version
3.8.0-RC1 (nightly)
Minimized code
//> using scala 3.nightly def spin = while true do {} class A @main def main = //creating a large object var large: Array[Double] | Null = Array.fill(10_000_000)(1.0) val other = A() //comparing it with another object (on the rhs) println((other == large)) //dropping the reference to the large object large = null //spin to check memory usage spinOutput
The memory grabbed by the large object is not freed, even though there is no variable holding it.
Root cause
I suspect this is because of the current backend behaviour when generating the byte code for an == operation (in BCodeBodyBuilder.scala:genEqEqPrimitive):
val eqEqTempLocal = locals.makeLocal(ObjectRef, nme.EQEQ_LOCAL_VAR.mangledString, defn.ObjectType, r.span)It currently creates a copy of the right-hand side of the "==", so that it can be used later without reevaluating it. However, this copy can create a new GC root, and is a potential memory leak, invisible to the user.
Suggested fix
I suggest to replace this local variable copy by some stack operations, which might also be slightly faster.
current backend code:
val eqEqTempLocal = locals.makeLocal(ObjectRef, nme.EQEQ_LOCAL_VAR.mangledString, defn.ObjectType, r.span) val lNull = new asm.Label val lNonNull = new asm.Label genLoad(l, ObjectRef) // load lhs stack.push(ObjectRef) genLoad(r, ObjectRef) // load rhs stack.pop() locals.store(eqEqTempLocal) // store rhs in a local variable bc dup ObjectRef // duplicate top stack variable (lhs) genCZJUMP(lNull, lNonNull, Primitives.EQ, ObjectRef, targetIfNoJump = lNull) // compare lhs with NULL markProgramPoint(lNull) bc drop ObjectRef // drop top stack variable (lhs) locals.load(eqEqTempLocal) // load rhs then compare with NULL genCZJUMP(success, failure, Primitives.EQ, ObjectRef, targetIfNoJump = lNonNull) markProgramPoint(lNonNull) locals.load(eqEqTempLocal) // load rhs then compare with lhs genCallMethod(defn.Any_equals, InvokeStyle.Virtual) genCZJUMP(success, failure, Primitives.NE, BOOL, targetIfNoJump)I suggest replacing with the following simple stack operations, with the resulting operand stack in comment:
- val eqEqTempLocal = locals.makeLocal(ObjectRef, nme.EQEQ_LOCAL_VAR.mangledString, defn.ObjectType, r.span) val lNull = new asm.Label val lNonNull = new asm.Label - genLoad(l, ObjectRef) + genLoad(r, ObjectRef) // load rhs --> (r) stack.push(ObjectRef) - genLoad(r, ObjectRef) + genLoad(l, ObjectRef) // load lhs --> (l,r) stack.pop() - locals.store(eqEqTempLocal) bc dup ObjectRef // duplicate top stack variable --> (l,l,r) genCZJUMP(lNull, lNonNull, Primitives.EQ, ObjectRef, targetIfNoJump = lNull) // compare lhs with NULL --> (l,r) markProgramPoint(lNull) bc drop ObjectRef // drop top stack variable --> (r) - locals.load(eqEqTempLocal) genCZJUMP(success, failure, Primitives.EQ, ObjectRef, targetIfNoJump = lNonNull) // --> (-) markProgramPoint(lNonNull) - locals.load(eqEqTempLocal) + emit(asm.Opcodes.SWAP) //swap l and r for correct .equals ordering --> (r,l) genCallMethod(defn.Any_equals, InvokeStyle.Virtual) // --> (-) genCZJUMP(success, failure, Primitives.NE, BOOL, targetIfNoJump)The fix is also accessible in this fork.
n.b. in the case where this is accepted, a few tests would need to be changed (for ex. DottyBytecodeTests:patmatControlFlow does not match the bytecode anymore)