Reducer instances hold onto the head of seqs

Description

(This ticket started life as CLJ-1250, was committed in 1.8.0-alpha2, pulled out after alpha4, and this is the new version that fixes the logic about whether in a tail call as well as addresses direct linking added in 1.8.0-alpha3.)

Problem: Original example was with reducers holding onto the head of a lazy seq:

Trickier example from CLJ-1250 that doesn't clear `this` in nested loop:

Problem: #'reducer closes over a collection in order to reify CollReduce, and the closed-over collection reference is never cleared. When code attempts to reduce over this anonymous transformed collection, it will realize the tail while the head is stored in the closed-over.

Approach: When invoking a method in a tail call, clear 'this' prior to invoking.

The criteria for when a tail call is a safe point to clear 'this':

1) Must be in return position
2) Not in a try block (might need 'this' during catch/finally)
3) Not direct linked

Return position (#1) isn't simply (context == C.RETURN) because loop bodies are always parsed in C.RETURN context

A new dynvar METHOD_RETURN_CONTEXT tracks whether an InvokeExpr in tail position can directly leave the body of the compiled java method. It is set to RT.T in the outermost parsing of a method body and invalidated (set to null) when a loop body is being parsed where the context for the loop expression is not RETURN parsed. Added clear in StaticInvokeExpr as that is now a thing with direct linking again.

Removes calls to emitClearLocals(), which were a no-op.

Patch: clj-1793-3.patch

Screened by: Alex Miller

Environment

1.8.0-alpha2 - 1.8.0-alpha4

Activity

Show:
Ghadi Shayban
August 24, 2015, 4:45 PM

Rich the original context is in CLJ-1250 which was a defect/problem. It was merged and revert because of a problem in the impl. This ticket is the continuation of the previous one, but unfortunately the title lost the context and became approach-oriented and not problem-oriented. Blame Alex. (I kid, it's an artifact of the mutable approach to issue management.)

Nicola Mometto
March 23, 2016, 1:34 PM

Just a note that the original ticket for this issue had 10 votes

Nicola Mometto
March 30, 2016, 2:50 PM

The following code currently eventually causes an OOM to happen, the patch in this ticket correctly helps not holding onto the collection and doesn't cause memory to run infinitely

Before patch:

After patch:

Alex Miller
September 8, 2016, 11:14 PM

Refreshed patch to apply to master. No semantic changes, attribution retained.

Alex Miller
March 14, 2017, 5:42 PM

Looks like this one was missed when applying patches for release

Completed

Assignee

Alex Miller

Reporter

Alex Miller

Labels

Approval

Ok

Patch

Code and Test

Fix versions

Affects versions

Priority

Critical
Configure