-
Notifications
You must be signed in to change notification settings - Fork 55
New Process-based Architecture #31
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
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cbf8aef to
e3c3c5b
Compare
…d to ship one executable)
This reverts commit 9e55756.
…d only accept drawing commands
…ing code + exposing the turtle::start() function
… to explain why we call that in Turtle::new
…st the thread's exit status
…ate in turtle window
… "renderer process"
…ad on all platforms
… struct takes care of that
…server.rs Renamed setup back to start
…re just using cfg on each line that needs it
…sh the same with just a few more normal cfg attributes. No more possibly hiding legitimate warnings.
…a reference too long -- we no longer need to do that because we just fetch an entire copy of the turtle state anyway
…is project, but the config added does start to get close
This was referenced Dec 18, 2017
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #27
Originally, our architecture worked in one process with two threads: a main thread and a rendering thread. The main thread was where the Turtle was created, its commands and animations were run, and other processing took place. The rendering thread created the window (which is not thread-safe) and continuously rendered the turtle's drawings over and over again. This architecture was great because it was fairly straightforward and everything could be done in a single process.
Unfortunately, we ran into an issue (#27) on MacOS which prevents us from creating the window on a separate thread like we used to. On MacOS, we must always create and manage the window from the main thread. We don't want to introduce anymore complexity into the API of the library because it needs to be as simple as possible in order to be usable by people with any skill level. In addition to that requirement, this library must work on all major platforms (Windows, Mac, Linux Mint, Ubuntu, etc.).
So how do we accomplish both the goal of working on every major platform and keeping the library simple to use? We somehow need to have two main threads. One would be used for creating the turtle, running commands, etc. and one would do the rendering and manage the window. Since each process on an operating system can only have one main thread, the solution must be to use two processes.
This PR re-architects the crate to use two processes. The main thread (where
Turtle::new()is run) spawns a new process which creates a window and runs the renderer. The processes communicate with each other using the stdin/stdout streams shared between them when the second process is spawned. We do this with Rust's built-in synchronous IO by spawning a thread in each of the two processes to continuously read from the other process.This is all done transparently, so none of the public API changed as a result of this PR and most of the original code is still the same. Only a single new function (
turtle::start()) was added to the public API in order to account for some other implementation details.Implementation
To prepare for this PR, the master branch was made to fail with a regression test. That test made it so that new commits would all pass on Linux, but fail on MacOS.
The renderer was split into its own process and some code was added to allow the two processes to communicate with each other and manage the state correctly. We chose a pretty simple and naive querying structure on purpose in order to reduce the complexity and boilerplate required to complete this implementation.
Conceptually, these changes are actually not too complicated. The implementation took two weeks mostly because of all the random issues that came up along the way. The commit dates are a bit messed up due to rebasing.
To Do