-
Notifications
You must be signed in to change notification settings - Fork 33
Optimize write artifacts to disk in watch mode #784
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
Optimize write artifacts to disk in watch mode #784
Conversation
ArtifactLookupCache is an object that keeps a mapping between relative artifact paths and their content hash. For avoiding triggering infinite reloads, the serialized object sorts the maps per key. It makes use of a Btree data structure to keep a reference to the ordered set of keys.
demos/pet-demo/src/components/__isograph/.artifact_lookup_cache.json
Outdated
Show resolved
Hide resolved
Rename it to FileSystemState, modifies it to map the filepaths to a tuple the file hash and the content as string. Also, remove it's public method that writes to disk and add a methdo to handle to compare its keys with other state keys in order to write/delete to/from file system the proper artifacts.
First of all, keep the file system state in the CompilerState and pass the state to the compile function. A new struct ChangedArtifacts keeps all the artifacts that will be added and deleted. the function get_artifacts_to_write returns the changed artifacts based on the new and old file system states.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing!!! This great work. Insane how much faster the compiler is now 😂 I can't believe we didn't do this sooner 😂 I guess there is still low-hanging fruit
I really like the big picture structure here. Let's change some of the ways we're representing things, namely to:
- reuse existing data structures where possible, and
- handle deleting directories, and
- represent the work that needs to be done as a
Vec<Action>instead of as a struct that we need to interpret.
🙌
Once you address these changes, I'll re-review.
rbalicki2
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, another round of reviews! I know it's a lot of comments, but this work is excellent!! These reviews are mostly about getting it into the "house style", cutting down on cloning and explaining more about our internal APIs.
After you incorporate these I'll re-review.
|
oh and clippy is complaining https://github.com/isographlabs/isograph/actions/runs/19684421184/job/56406099824?pr=784 |
rbalicki2
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets gooo!!! thank you so much for tackling this. Amazing work
|
Can you address the clippy errors? FWIW I'll look into why CI is not auto-running for you. (Perhaps after you accept the invitation to be a contributor it will auto-run?) |
rbalicki2
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just some small added nits, in addition to clippy
rbalicki2
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets gooo
Co-authored-by: Lucas Machado <lucasmachadorj@gmail.com>
- Previously, on every recompilation, we deleted and recreated the `__isograph` folder entirely.
- Now, we keep an in-memory data structure (`FileSystemState`) that tracks what we have written to the file system.
- After every recompilation, we recreate the `FileSystemState` from the `Vec<ArtifactPathAndContent>`, and generate a diff (`Vec<FileSystemOperation>`), and apply those changes.
- This substantially improves performance for small changes, and most changes are small changes. Examples:
- in pet demo, updating the graphql schema by adding a new, unused field on pet demo goes from 144ms to 53ms (in a debug build on a macbook pro m1)
- updating the Query.HomeRoute iso literal by adding a new field went from 125ms to 72ms
Amazing!!


This PR creates a FileSystemState that is used to optimize the performance of write-to-disk artifacts in watch mode.
The first compilation, changes in config, or not running compilation in watch mode (
compile_and_print) makes the artifacts directory being deleted and recreated.While in watch mode, on every reload, it creates a new file state based on artifact filepaths, their content and hashes and compare the hashes from old and new states to create a vector of filesystem operations (create dir, create file, delete fir, delete file) which are properly handled by
write_artifacts.rsThis is the implementation of the issue #779