Skip to content
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

Reuse postprocessing environments in a safer way. #286

Merged
merged 3 commits into from
Apr 1, 2016

Conversation

psigen
Copy link
Member

@psigen psigen commented Mar 28, 2016

This should be merged after #278.

The previous implementation of PostProcessPath could potentially generate unbounded numbers of environments, as it could generate a new robot instance every time we cloned into the same environment. (This was still better than the original implementation, which always created a new environment.)

Here, we keep a module-level mapping from environments to postprocessing environments and create them on demand, which guarantees that each cloned environment will have at most one postprocessing environment.

The previous implementation of PostProcessPath could potentially
generate unbounded numbers of environments, as it could generate
a new robot instance every time we clone into the same environment.

Here, we keep a module-level mapping from environments to
postprocessing environments and create them on demand, which
guarantees that each cloned environment will have at most one
postprocessing environment.
@mkoval
Copy link
Member

mkoval commented Apr 1, 2016

👍 @psigen explained this in person. I'm not thrilled about using a module-level variable for this, but I don't see any alternative. 😓

@mkoval mkoval merged commit 8cbfa84 into master Apr 1, 2016
@mkoval mkoval deleted the bugfix/postprocess_reuse branch April 1, 2016 03:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants