Completed
Details
Details
Assignee
Alex Miller
Alex MillerReporter
Alex Miller
Alex MillerLabels
Approval
Ok
Patch
Code and Test
Priority
Affects versions
Fix versions
Created August 5, 2015 at 4:56 PM
Updated May 12, 2017 at 6:05 PM
Resolved May 12, 2017 at 6:05 PM
(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:
(time (reduce + 0 (map identity (range 1e8)))) ;; works (time (reduce + 0 (r/map identity (range 1e8)))) ;; oome from holding head of range
Trickier example from CLJ-1250 that doesn't clear `this` in nested loop:
(let [done (atom false) f (future-call (fn inner [] (while (not @done) (loop [found []] (println (conj found 1))))))] (doseq [elem [:a :b :c :done]] (println "queue write " elem)) (reset! done true) @f)
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