New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removed unbounded member of FsLock #14

Merged
merged 2 commits into from Aug 28, 2017

Conversation

Projects
None yet
2 participants
@blt
Collaborator

blt commented Aug 18, 2017

These two commits resolve the issue of unbounded memory growth in cernan. It turns out every messages sent through Sender would cause 128 bytes to be pushed onto an unbounded VecDeque. In a previous life this VecDeque was used for proper coordination between the Senders and its Receiver but the optimization made in #5 broke this need. In doing so, we also broke the emptying of this deque.

This is now corrected and hopper will use a bounded amount of memory.

Brian L. Troutwine added some commits Aug 18, 2017

Brian L. Troutwine
Begin work to remove unbounded write_bounds
We've known for some time that write_bounds could become unbounded.
It turns out, in practice, it is. This makes the comment of "yikes,
unbounded" even more amusing. The accumulation is very slow, 128 bytes
every so often but it's enough.

Just goes to show, you should never allow unbounded collections.

Signed-off-by: Brian L. Troutwine <blt@postmates.com>
Brian L. Troutwine
Fully remove unbounded VecDeque
We still need write_bounds to control when the senders set the
read index for the receiver they are bound to but we have no need
to buffer them into a VecDeque. In fact, owing to the pushing into
the front of the buffer and popping only from the back a single
Option is sufficient.

This removes the unbounded member of the fslock structure.

Signed-off-by: Brian L. Troutwine <blt@postmates.com>

@blt blt requested a review from tsantero Aug 18, 2017

@tsantero

+1

@tsantero tsantero merged commit ab90388 into master Aug 28, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment