[WIP]: speed up rope iteration and serving #4008
Closed
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
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
9 Ignored Deployments
|
ForsakenHarmony
approved these changes
Feb 28, 2023
crates/turbo-tasks-fs/src/rope.rs
Outdated
| @@ -686,6 +702,22 @@ impl From<RopeElem> for StackElem { | |||
| } | |||
| } | |||
|
|
|||
| trait PtrEq { | |||
There was a problem hiding this comment.
I don't think we need a new trait for this?
Could add a small helper function for Bytes, but even that might be more overhead to read than just copy pasting 2 times
crates/turbo-tasks-fs/src/rope.rs
Outdated
| let mut left = RopeReader::new(left, index); | ||
| let mut right = RopeReader::new(right, index); | ||
| loop { | ||
| match (left.fill_buf(), right.fill_buf()) { | ||
| match (left.next(), right.next()) { | ||
| // fill_buf should always return Ok, with either some number of bytes or 0 bytes |
There was a problem hiding this comment.
this comment probable needs to be changed
🟢 CI successful 🟢Thanks |
Benchmark for 9eac652
Click to view full benchmark
|
|
Appears this was not the cause, rope iteration must just be really slow. |
This reverts commit 44598d4.
jridgewell
changed the title
Benchmark for 8674b47
Click to view full benchmark
|
Benchmark for 02d2dd6
Click to view full benchmark
|
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 is just a test to see if
Bytesequality is secretly very slow. It uses slice equality, which usesmemcmpfor&[u8]. But maybememcmpdoesn't have the obvious optimization for pointer equality? This is the only reason that I can think of leading to the performance improvements in #3955. The rope traversal can't be that expensive, right?