Improve the paths subsets of jobs #220
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
11 tasks
9 tasks
sophiedeziel
force-pushed
the
introduce-indexers
branch
from
230012b to
a9ba64b
Compare
|
Visit the preview URL for this PR (updated for commit 8c2c327): https://gcode-preview--pr220-introduce-indexers-34ft8rl4.web.app (expires Fri, 15 Nov 2024 00:54:46 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 59bd114ae4847b32c2bba0b68620b9069a3e3531 |
sophiedeziel
force-pushed
the
introduce-indexers
branch
2 times, most recently
from
638adb2 to
76a4c17
Compare
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @sophiedeziel and the rest of your teammates on |
sophiedeziel
force-pushed
the
introduce-indexers
branch
2 times, most recently
from
3071658 to
cdaebe1
Compare
sophiedeziel
force-pushed
the
introduce-indexers
branch
3 times, most recently
from
678a96f to
256595a
Compare
sophiedeziel
force-pushed
the
introduce-indexers
branch
2 times, most recently
from
e32e879 to
d81f5ea
Compare
Merged
sophiedeziel
force-pushed
the
introduce-indexers
branch
from
d81f5ea to
8c2c327
Compare
This was referenced Oct 16, 2024
This was referenced Oct 16, 2024
Merged
sophiedeziel
added a commit
that referenced
this pull request
Oct 19, 2024
* Introduce the traveltype indexer * Add the layer indexer * don't clear for all types * Fix the calls to the getters * Cleaner error management * Fix all tests
sophiedeziel
added a commit
that referenced
this pull request
Oct 24, 2024
* Introduce the traveltype indexer * Add the layer indexer * don't clear for all types * Fix the calls to the getters * Cleaner error management * Fix all tests
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.
Follow up to #211
layers,extrusionsandtravelsare now more performant!Instead of filtering the whole
pathsarray each time, the job uses indexers to categorize paths as they are added. They are then accessed as readily available data without additional logic.To implement indexes, logic to have paths "in progress" and finish them had to be implemented to only index when the path is finished.
Paths are indexed as they are added to a job, which is less expensive than filtering the array on the spot. Jobs have a set of default indexers. They can be added or removed for the different rendering modes. It paves the way to #121 as we'll have to index for the different types of extrusions. It is also a possible implementation for #183.
When an indexer throws an error, it is interpreted as not applicable for a specific job. It is removed from the list of indexers for the rest of the commands. I am pretty happy about that design; it stops some code and conditions from running for the rest of the list of paths we are iterating on.
Adding/removing indexers will have a significant impact later when we have many of them. Other logic related to stats and calculations may follow a similar design and benefit from being composed. If they become expensive, we'll only use what is needed.
Next PR would be to bring back the tolerance logic inside the indexer that was omitted in #211
Note that this branch was not rebased after #215, making progressive rendering a bit funky. With the fix, you'll notice the smoothness is back.