go compilation holding onto head of closed over seq
Description
Environment
Attachments
clones
Activity
Kevin Downey July 18, 2022 at 10:54 PM
updated the patch due to a missing word in the commit message
Kevin Downey July 18, 2022 at 9:55 PM
I’ve deleted the existing patch files and uploaded a single patch file with a single commit that contains everything (including what I was previously calling the optional patch 4) since that seems more inline with the patch guidelines ( ).
Kevin Downey July 17, 2022 at 10:18 PM
tweaks to patch 3 and 4 around optimizing the traversal order of blocks to limit the amount of recomputation required. with these changes a new setting of timings (loading the clojure.core.async namespace in a fresh repl)
Code | relative load time | absolute load time avg (ms) |
|---|---|---|
master | 100% | 5797.25 |
patch 3 | 107% | 6221.94 |
patch 4 | 104% | 5967.38 |
I am not sure why 3 and 4 have a difference in the timings, could just be noise.
Kevin Downey July 15, 2022 at 5:09 PM
timings for loading clojure.core.async averaged of 100 runs each:
Code | relative load time | absolute load time(ms) |
|---|---|---|
master | 100% | 5240.49 |
patch 1-3 | 111% | 5847.36 |
patch 4 | 111% | 5835.85 |
Kevin Downey July 15, 2022 at 1:59 AM
While patches 1-3 fix this bug and add locals clearing to go blocks, I’ve added an optional 4th patch in the series. Patch #4 is a larger change, but seems like a good improvement.
Patch #4
subsumes the existing analysis in the new data flow analysis and gets rid of the block index
removes the crossing env stuff
adds the once annotation to the emitted state machine function.
The down side to patch #4 is it seems to lead to larger state arrays. I haven’t dug into why.
Code | Avg State Array Size loading clojure.core.async |
|---|---|
master | 17.0 |
patches 1-3 | 17.05 |
patch 4 | 17.57 |
Details
Assignee
Kevin DowneyKevin DowneyReporter
Alex MillerAlex MillerPatch
CodePriority
Critical
Details
Details
Assignee
Kevin DowneyReporter
Alex MillerPatch
Priority
Critical
This is effectively the same issue as (which was theoretically fixed in ), and I think is probably the same basic problem as or , although those may be different variants.
The changes in 138 lift the locals into a thunk to prevent head-holding, but I think the ssa for
loopandletand maybe others are not properly clearing the head of those seqs when put into the state vector of the compiled go code.Repro:
onto-chan! is:
Here, I believe
collandvsboth end up in the state vector and one of them is not being cleared when it could be.