Optimize and simplify line tessellation code #8303
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
31 tasks
mourner
force-pushed
the
improve-round-join-tessellation
branch
from
0aeafdc to
858e45a
Compare
mourner
force-pushed
the
optimize-line-bucket
branch
from
8fc3a04 to
79e63c8
Compare
ansis
approved these changes
Aug 20, 2019
| this.distance = 0; | ||
| this.scaledDistance = 0; | ||
| this.totalDistance = 0; | ||
|
|
There was a problem hiding this comment.
Do you think creating a line for each object is a significant cost? putting per-line values directly on the bucket seems a bit less nice but probably ok if there is a significant benefit.
There was a problem hiding this comment.
@ansis we had distance as a bucket state before this PR, so the latter doesn't change the status quo in that sense — I moved the line stats to shared state too for simplicity/consistency, but I think we can revisit this in a follow-up PR.
mourner
force-pushed
the
optimize-line-bucket
branch
from
79e63c8 to
931bc0f
Compare
|
Layout benchmarks are not affected:
|
6 tasks
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 refactors the line tessellation code, removing code repetition and some unnecessary logic, making it both faster and easier to follow.
The potential performance improvement comes from reducing the amount of temporary
Pointobject allocations by ~2 times, which theoretically reduces the amount of subsequent GC pauses. I'll try to find a way to measure real-world performance impact, but roughly it should be 5-10% less time spent tessellating lines on the worker side.It brings some ideas from #7085 but in a less intrusive way.
The PR is made on top of #8275 so needs to be applied after it lands.
Launch Checklist
write tests for all new functionalityalready covereddocument any changes to public APIsno changes