Skip to content
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

Use Vec::extend_from_within() in place of custom unsafe code #18

Open
Shnatsel opened this issue Jun 18, 2021 · 3 comments
Open

Use Vec::extend_from_within() in place of custom unsafe code #18

Shnatsel opened this issue Jun 18, 2021 · 3 comments

Comments

@Shnatsel
Copy link

Vec::extend_from_within() is a newly stabilized function that allows appending a part of a vector to itself.

Looks like it could replace some uses of unsafe code in lz4_flex, e.g.

// We cannot simply use memcpy or `extend_from_slice`, because these do not allow
// self-referential copies: http://ticki.github.io/img/lz4_runs_encoding_diagram.svg

If it turns out to be not as good as the unsafe variant (I see that you're specifically avoiding memcpy), perhaps it could still make the safe variant faster.

@PSeitz
Copy link
Owner

PSeitz commented Jun 20, 2021

I did some experiments after your comment on reddit last time. The unsafe version was faster, since there was still some overhead in comparison to the pointer approach.

In the meantime the block api changed and it uses slices now as parameters, so extend_from_slice is not applicable anymore. But there were some gains in the safe variant by using the slice copy methods.

@Shnatsel
Copy link
Author

The slice-based approach as used right now is sadly unsound; I've opened #19 about that.

More generally, to make this sound you need to write either to a Vec without using set_len() or to &mut MaybeUninit to make things sound and fast. Writing to a slice would require initializing it first, which in case of something as fast as LZ4 would cut performance in half.

@Shnatsel
Copy link
Author

Well, I guess writing to the Vec's capacity through raw pointers and only then calling set_len() would also work, provided you're careful about aliasing.

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

No branches or pull requests

2 participants