Has the (left recursive blowing) concat function been fixed?

297 views
Skip to first unread message

divy...@helpshift.com

unread,
Sep 12, 2017, 9:56:16 AM9/12/17
to Clojure
I came across this discussion:
https://groups.google.com/forum/#!topic/clojure-dev/ewBuyloeiFs/discussion

Has this patch been considered for merging in Clojure master?
If yes, awesome! We should update / comment-on articles pointing this out.
If not, why?

Alex Miller

unread,
Sep 12, 2017, 10:05:57 AM9/12/17
to Clojure
On Tuesday, September 12, 2017 at 8:56:16 AM UTC-5, divy...@helpshift.com wrote:
I came across this discussion:
https://groups.google.com/forum/#!topic/clojure-dev/ewBuyloeiFs/discussion

Has this patch been considered for merging in Clojure master?

No, this didn't move forward. I don't think it was ever actually filed in the jira system?
 
If yes, awesome! We should update / comment-on articles pointing this out.
If not, why?

 There are lots of ways to avoid this problem (into, transducers, etc) and in practice I think most people don't find it to be an actual issue.

I have not looked closely at the proposed solution, but beyond the basic "does it work" question, there are also a lot of questions about performance, etc that would be need to be evaluated.



divy...@helpshift.com

unread,
Sep 15, 2017, 6:45:43 AM9/15/17
to Clojure
Hi Alex,

 
No, this didn't move forward. I don't think it was ever actually filed in the jira system?

Hmm.
 
 There are lots of ways to avoid this problem (into, transducers, etc) and in practice I think most people don't find it to be an actual issue.
 
Well, we did face this issue in production some time back, and it took quite some time to figure out what was going wrong.
Reading comments on the linked post on Stu's blog shows that quite a lot of other people have also been affected by it.
 
I have not looked closely at the proposed solution, but beyond the basic "does it work" question, there are also a lot of questions about performance, etc that would be need to be evaluated.

I think correctness > performance, at least in a core function that's so widely used (or maybe put a "warning: don't use in eager contexts" in the doc).
Please let me know if I can help in any way.
I'll run tests and some microbenchmarks on this and get back to you.

Thanks,
- Divyansh

Sean Corfield

unread,
Sep 15, 2017, 3:57:26 PM9/15/17
to clo...@googlegroups.com

I think correctness > performance, at least in a core function that's so widely used

 

I don’t want to speak for Alex or any of the Clojure/core folks but…

 

The general position taken in Clojure so far could be more accurately categorized as Garbage-In-Garbage-Out, in favor of performance. In other words, Clojure functions are usually designed for performance on correct data and in correct usage situations – and just fail or do something odd/unpleasant when used on the wrong data or in the wrong situation.

 

The solution to “it blows up when I do X” is “don’t do X” rather than making X slower for everyone, in order to make X deal “better” with cases it wasn’t designed for. It’s why, for example, many things simply blow up with a ClassCastException rather than adding conditional checks – and slowing everyone down – just to produce a “better” error message.

 

Sean Corfield -- (970) FOR-SEAN -- (904) 302-SEAN
An Architect's View -- http://corfield.org/

"If you're not annoying somebody, you're not really alive."
-- Margaret Atwood

--
You received this message because you are subscribed to the Google
Groups "Clojure" group.
To post to this group, send email to clo...@googlegroups.com
Note that posts from new members are moderated - please be patient with your first post.
To unsubscribe from this group, send email to
clojure+u...@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/clojure?hl=en
---
You received this message because you are subscribed to the Google Groups "Clojure" group.
To unsubscribe from this group and stop receiving emails from it, send an email to clojure+u...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

 

Matching Socks

unread,
Sep 15, 2017, 8:36:20 PM9/15/17
to Clojure
I think Alex Miller was pointing out that Jira is the point-of-entry for patches to be evaluated in an orderly way and, particularly, in the context of the contributor agreement.  So, in this case, the merits have not been considered, much less declined.  Perhaps the programmer will step back in and do the necessary?  Until then, further discussion is pointless.  

Speaking of which, my 2c is that the very use of "concat" is a sign you are not counting nanoseconds.  Safety first!

Alex Miller

unread,
Sep 15, 2017, 8:40:38 PM9/15/17
to Clojure
Yes, that. I have too much stuff in the air to juggle patches here. Jira would be great.

Jon Distad

unread,
Sep 18, 2017, 12:45:44 PM9/18/17
to Clojure
I wrote the original patch, but now that it's been 2 years I don't really support it anymore. There are lots of ways one could build up an oversized stack with lazy seqs. Concat might be the most common, but it's only one possibility. I think it's better to have consistent behavior than to have a bunch of ad hoc cases.

I could see an argument though for a version of concat that is strict in its first parameter, kinda like foldl' vs foldl in haskell, and that wouldn't require any changes to the underlying runtime classes.
Reply all
Reply to author
Forward
0 new messages