Skip to content

Remove use of BuildR#2445

Merged
diesalbla merged 2 commits intotypelevel:mainfrom
diesalbla:noleaks_small
Jun 25, 2021
Merged

Remove use of BuildR#2445
diesalbla merged 2 commits intotypelevel:mainfrom
diesalbla:noleaks_small

Conversation

@diesalbla
Copy link
Copy Markdown
Collaborator

@diesalbla diesalbla commented Jun 19, 2021

Retries the change in #2196 , which were reverted in #2394.

This "simplification", was tried before and rolled back because it caused some Memory Leaks. Inspecting a heapdump, we saw that the leaks were caused by the compilation creating many objects of type Bind, BindBind, or DelegateBind, in which the step (the pull on the left of >>=) was just a terminal. To avoid this problem, we add a "smart constructor" method bindView, used when creating a Pull by binding a Pull with a View. This constructor, instead of always creating a new object, applies the "continuation" function directly. This is admisible during compilation.

Edit: one consequence of directly calling the cont method, to avoid accumulating the DelegateBind objects, is that the tailrecM method of the Monad instance for Stream was no longer stacksafe. We could cause a stack overflow due to the long chain of recursive calls to the method cont going up the chain of BindBind objects. To avoid this, we extract a loop (tail-recursive method) to perform this simplification.

One problem, however, is that this seems to cause one test on the MonadError laws for Stream to fail, the stack-safety of tailRecM. However, when we run the code of the stack-safety test, using a different runtime, it seems to pass.

This "simplification", was tried before and rolled back because
it caused some Memory Leaks.
Inspecting a heapdump, we saw that the leaks were caused by the
compilation creating many objects of type `Bind`, `BindBind`, or
`DelegateBind`, in which the  `step` (the pull on the left of >>=)
was just a terminal.
To avoid this problem, we add a "smart constructor" method `bindView`,
used when creating a Pull by binding a Pull with a View.
This constructor, instead of always creating a new object, tries
to apply the evaluation-reduction instead.

One problem, however, is that this seems to cause one test on the
MonadError laws for Stream to fail, the stack-safety of `tailRecM`.

more
@diesalbla diesalbla marked this pull request as draft June 19, 2021 01:02
@diesalbla diesalbla requested a review from mpilquist June 19, 2021 01:02
@diesalbla diesalbla force-pushed the noleaks_small branch 2 times, most recently from ebd1f49 to e385771 Compare June 20, 2021 02:56
@diesalbla diesalbla requested a review from SystemFw June 20, 2021 11:48
The tailRecM - stack safety errors that we observed were genuine, and
they were caused by we having an unlimited chain of recursive calls
to the `cont` method from the BindBind class.

To prevent this, we extract a loop (a tail-recursive method) that
keeps unrolling this chain for as long as it is only terminals that
it meets throughout.
@diesalbla diesalbla marked this pull request as ready for review June 24, 2021 00:13
@diesalbla diesalbla merged commit c7af765 into typelevel:main Jun 25, 2021
@diesalbla diesalbla deleted the noleaks_small branch July 31, 2021 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants