Remove allocation for builder cache hits #40
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
Codecov Report
@@ Coverage Diff @@
## master #40 +/- ##
==========================================
- Coverage 79.00% 78.26% -0.74%
==========================================
Files 8 9 +1
Lines 543 612 +69
==========================================
+ Hits 429 479 +50
- Misses 114 133 +19
Continue to review full report at Codecov.
|
|
Clippy failure is expected due to the beta stabilization making a polyfill unused, tracked in #17. |
|
rust-lang/rust#72583 and rust-lang/rust#72584 propose stabilizing/adding the API into the stdlib to make this work nicely. #41 implements just the subset of this that works and makes sense even on today's stable. |
|
superseded by #45 |
bors bot
added a commit
that referenced
this pull request
Jun 23, 2020
45: Remove allocation for builder cache hits r=CAD97 a=CAD97 Credit to @simonvandel [over at the main rowan repository](rust-analyzer/rowan#59) for pointing out how this is possible. Previously: #40 Now that the necessary APIs for this are riding the trains to stable, here's an actual decent implementation of alloc-free cache hits! Also adds a bit more explicit typing on pointers, as that led to a bug in the construction of this patch. Co-authored-by: CAD97 <cad97@cad97.com>
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.
Credit to @simonvandel over at the main rowan repository for pointing out how this is possible.
Draft proof of concept includes many problematic corners that make this not mergeable:
vec::Drain::as_slice(Tracking issue for slice::IterMut::as_slice and vec::Drain::as_slice rust-lang/rust#58957)ptr-unionwrangling ingreen/builder.rsthat shouldn't need to be therefn Union2::as_untagged_ptrrather than unpacking the uniontrait AsSliceis a hack to accept theas_slice-having by-value iteratorsAsRef<[T]>is probably the correct bound hereIntoIteratorrather thanIteratordirectlyIntobound is nice but unnecessaryas_slice/RandomAccessIteratorand performs the old behavior of collecting into a provisional nodematch(though not as nice asmatch match match)