[
https://issues.apache.org/jira/browse/GROOVY-10657?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18140919#comment-18140919
]
ASF GitHub Bot commented on GROOVY-10657:
-----------------------------------------
daniellansun opened a new pull request, #3283:
URL: https://github.com/apache/groovy/pull/3283
Backport of https://github.com/apache/groovy/pull/3280 to `GROOVY_6_0_X`.
Follow-up to https://github.com/apache/groovy/pull/3226 (already on this
branch).
## Review follow-up
The array-store duplicate lived on `BinaryExpressionMultiTypeDispatcher` as
`duplicateTopBeneath(operandStack, operandsBelow)`. The only caller passed `2`.
That helper now sits on `OperandStack` next to `dup` and `swap`, as
`dupUnderTwo()`.
An array store has already cast the index to `int`, so the two operands
under the value are one word each (array reference, int index). The copy is
`DUP_X2`, or `DUP2_X2` when the value is a `long` or a `double`. `DUP_X1` and
`DUP2_X1` are not part of this operation.
## What still uses a temporary
A plain array or `putAt` statement evaluates receiver, index and right-hand
side as the store arguments. No temporary is allocated.
When the assignment expression's value is required:
- The array-store fast path duplicates the value under those two operands
before the component cast. The store consumes the original. The expression
value is the pre-cast right-hand side, not the value returned by `putAt`.
- A `putAt` call cannot share that duplicate. The classic call site pushes a
`CallSite` under the receiver and the index, three words below the value, and
there is no `DUP_X3`. Inserting the copy only under the receiver and the index
would make it the `invokeinterface` receiver. Invokedynamic has no call-site
operand, but the copy would have to be emitted inside that backend after the
arguments are on the stack. Both backends keep one temporary across the call
and clear a reference slot immediately afterwards.
Compound `+=`, Elvis `?=` / `??=`, and postfix `a[i]++` evaluate the
receiver and the index once because each is used for the read and the write.
The operation result is also stored, because `putAt` consumes it and the
expression value is the operation result. Those slots are required. Reference
slots are cleared once the value has been reloaded. `removeVar` only forgets
the slot; it does not clear it.
## Guarantees
- Evaluation order for a plain subscript assignment stays receiver, then
index, then right-hand side. A safe subscript still evaluates the right-hand
side before the null check and skips the index when the receiver is null.
- The value of an assignment expression remains the right-hand side (or the
compound-operation result), not the value returned by `putAt`.
- A reference temporary that subscript assignment still needs does not stay
reachable for the rest of the enclosing method.
## Tests
- `bugs.Groovy10657` covers statement and expression `putAt`, array
assignment, Elvis subscript, compound `+=`, and postfix `a[i]++`, including
that a large right-hand side or receiver can be collected while the frame is
still live.
- `StaticCompilationTest` keeps the `@CompileStatic` store sequences
(`intArraySet`, `objectArraySet`, `DUP_X2`, `DUP2_X2`).
- `BinaryOperationsTest` checks the same store sequences without
`@CompileStatic` on the classic backend, and the invokedynamic `putAt` shape: a
statement allocates no right-hand slot, and an expression clears the one
temporary it keeps.
> Generated code for overloaded assignment operator prevents garbage collection
> -----------------------------------------------------------------------------
>
> Key: GROOVY-10657
> URL: https://issues.apache.org/jira/browse/GROOVY-10657
> Project: Groovy
> Issue Type: Bug
> Components: Compiler
> Affects Versions: 4.0.3
> Reporter: Jonathan Baxter
> Assignee: Daniel Sun
> Priority: Major
> Fix For: 7.0.0-alpha-1, 6.0.1
>
>
> The groovy code
> {code:java}
> obj["lhs"] = "rhs"{code}
> ostensibly converts to
> {code:java}
> obj.putAt("lhs","rhs")
> {code}
> if {{obj}} has the appropriate {{putAt}} method.
> However , in the process of tracking down a memory leak, I discovered that
> assignment is not exactly equivalent to invocation of the {{putAt}} method.
> Specifically, the code generated by the compiler is equivalent to creating an
> additional temporary variable to hold the rhs of the assignment, and then
> invoking the {{putAt}} method with that temporary variable as its second
> argument:
> {code:java}
> String string = "rhs"
> obj.putAt("lhs",string){code}
> The problem with this is that since {{"rhs"}} now has a live reference
> ({{{}string{}}}), it is not eligible for garbage collection.
> In my particular use case, {{"rhs"}} is a very large object that I would like
> to be automatically garbage collected after the assignment is executed, but
> before the script has finished. But that means I have to use the underlying
> {{putAt}} method rather than the assignment operator, which rather negates
> the benefits of overloading the assignment operator.
> One potential fix would be to simply null out any temporary variables
> constructed by the compiler, so that
> {code:java}
> obj["lhs"] = "rhs"
> {code}
> becomes
> {code:java}
> String string = "rhs"
> obj.putAt("lhs",string)
> string = null <----- "rhs" is now eligible for garbage-collection{code}
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
