Skip to content

Fix just a few stacked borrow issues, and improve extend performance #111

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

Merged
merged 3 commits into from
Nov 25, 2018

Conversation

bluss
Copy link
Owner

@bluss bluss commented Nov 25, 2018

Extend: This simplification -- borrowing self.len instead of self, leads to
an improvement in the extend_from_slice benchmark.

It's also guided by the discussion of stacked borrows; the old code
would be invalid, because the whole self is borrowed while ptr is derived from
self.

In truncate, don't access self while holding raw pointer derived from self

Again, stacked borrows model makes the self.set_len() call illegal
because we are holding (and are going to use) another raw pointer
derived from self, tail.

bench comparison for extend. Caveat might be that optimization can also be influenced by how the code can be optimized in the benchmark.

 name                  63 ns/iter       62 ns/iter       diff ns/iter   diff % 
 extend_with_constant  290 (1765 MB/s)  296 (1729 MB/s)             6    2.07% 
 extend_with_range     288 (1777 MB/s)  287 (1783 MB/s)            -1   -0.35% 
 extend_with_slice     828 (618 MB/s)   218 (2348 MB/s)          -610  -73.67%

This simplification -- borrowing self.len instead of self, leads to
an improvement in the extend_from_slice benchmark.

It's also guided by the discussion of stacked borrows; the old code
would be invalid, because the whole self is borrowed while ptr is derived from
self.
The benchmark was optimized out totally. We think of that as a good
sign, the new extend became transparent to the compiler and we had to
get smarter in how to fool it.
… from self

Again, stacked borrows model makes the `self.set_len()` call illegal
because we are holding (and are going to use) another raw pointer
derived from self, `tail`.
@bluss
Copy link
Owner Author

bluss commented Nov 25, 2018

There is bound to be more issues to fix than this. cc #97

@bluss bluss merged commit d84cb37 into master Nov 25, 2018
@bluss bluss deleted the miri-complaints branch November 25, 2018 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant