-
Notifications
You must be signed in to change notification settings - Fork 53
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
Rewrite internals of turtle crate #173
Merged
Conversation
This file contains 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
… only used spawn_blocking for actual blocking code
…lishes a connection
This was
linked to
issues
May 25, 2020
Closed
Closed
This was referenced May 25, 2020
Open
This was referenced May 25, 2020
Open
Closed
This was referenced Jun 11, 2020
Merged
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.
This PR completely rewrites the internals of the turtle crate. This work was done to address a number of issues and set the stage for many exciting features that will be added in the future (see below). This is essentially a more robust version of the work done in #31. Three years after doing that work in a hurry while trying to address a showstopper bug, I have finally had some time to redesign turtle (almost) from scratch.
Overview
The new architecture is a little more complicated than the old one, but still largely the same at a high-level:
The reason it's more complicated now is that we've introduced several platform-specific backends. These deal with issues like #27 while still allowing for more flexibility on platforms where that issue doesn't exist. There are currently three backends:
multiprocessed
(used on MacOS) - The main process and server process are actually separate processes which communicate via IPCmultithreaded
(used on Windows and Linux) - The main process and the server process are separate tasks (basically separate threads) and communicate via channels in the same processtest
(used for running tests) - The server process does not actually create any windows or manage events, etc.Goals
There were several key goals with this rewrite:
ipc-channel
instead of JSON over stdin/stdout (Optimize IPC #50)I chose these goals because I knew that they had the potential to impact the stable public API of the crate. I needed to fully explore the multiple turtle and async turtle features in order to know whether our public API would still work when those features are finally released. I also needed to fix a number of bugs and deal with issues regarding our windowing/graphics APIs so I could know which parts of the API to keep unstable after v1.0.0 is released.
(I've actually been trying to release a turtle v1.0.0 for the past several years. That's why you've seen so many
1.0.0-{alpha,rc}
versions...)Does all of this work ended up being a great exercise because I found several major bugs in turtle and even ended up making some breaking changes. I am definitely glad I put in the time to explore these features even though it will still take time for them to be publically exposed in the crate.
For a full list of the breaking changes, see CHANGELOG.md
Issues Addressed
Fully Asynchronous Internals
The turtle crate is now entirely asynchronous under the hood. This really helps with (#99) since we now only do work when needed and wait for things to happen in all other cases. The turtle crate has 0 CPU usage unless something is drawing, and even then we only use the bare minimum. The
Turtle
struct is now just a blocking interface over a newAsyncTurtle
struct.AsyncTurtle
will be made public when the async turtles feature is completed.(Shout out to tokio, the great async library that the turtle crate relies on. The tokio maintainers are very nice and I received a great deal of help from the people on their Discord. Really enjoyed using the library and I'm glad we have it!)
Multiple Turtles
To enable multiple turtle support, I rewrote the renderer process to track an arbitrary number of turtles and their drawings. Each
Turtle
now remembers itsTurtleId
(not exposed publically) and uses that to tell the renderer process to draw things on behalf of that turtle. As part of #44, all animation is now performed on the renderer process. So turtles no longer need to use IPC to update the "temporary path". Everything is done in the server process asynchronously.(Source)
Having multiple turtles enables a lot of opportunities for concurrency and parallelism. Our previous approach of running one drawing command at a time doesn't really work, especially now that animations are handled in the server process. If one turtle is drawing a line, we don't want other turtles to have to wait for that if they don't have to. A lot of work was done to enable this kind of concurrency while also preserving operations that have to be sequentially consistent (e.g. clear all drawings).