rbrush/clara-rules — pull request #74 by mrrodriguez (2014-12-26 21:14:01+00:00)
Title: Fixing potential StackOverflowError when retracting logical insertions

There is another occurrence of the issues discussed in a former pull request @ https://github.com/rbrush/clara-rules/pull/59.

I made a note there previously:
>Also, note that c.r.m/add-insertions! has the same nested concat call structure in it. However, I have not reproduced this in a test case yet. I believe it may need to be addressed in the same way as the 3 places addressed in this issue. I have left it alone currently.

I believe I've seen an issue come up when rules ended up needing to perform retractions for a rule that had many facts inserted based on the same LHS logical support. When this happens a lazy sequence with a deeply nested first (and potentially second, etc) element must be realized.

I didn't understand how to recreate this with `clara.rules.memory/add-insertions!` previously when I made the original pull request. Now that I looked into it a bit closer, I understand the issue and have recreated it in a test case as part of this pull request.

I've taken the same approach to fixing this of just replacing `concat` with `into`. The negative side of this is that `into` is eager rather than lazy as `concat` previously was. I don't know how important laziness would really be at this point in Clara's working memory management though. If laziness is something we want to have in these parts of working memory management, we'll have to do something different than stacking `concat` calls up at the head of the lazy seq.

This is the same discussion I had in the previous pull request though, so I think it makes sense to fix the StackOverflowError potential here as we did in those changes before.

I don't see any other usages of `concat` left that are at risk of this this time around.

---

I am going to make another issue that is related to some findings I had when working on this. I think there is a possible area for performance issues in scenarios where many logical insertions having different bound variables (can happen in a query fairly easily) must be retracted due to some working memory changes.
I have a comment in the `clara.test-rules/test-retracting-many-logical-insertions-for-same-rule` that discusses where this can happen at.
I think fixing this may be a bit trickier than the StackOverflowError, so I am keeping it a separate topic. Not to mention it isn't guaranteed to have bad performance, it just has potential.