Replace usage of unsafe with Vec::extend_from_within

[paolobarbolini]

When this will eventually become stable it will allow unsafe to be removed from here:

zstd-rs/src/decoding/decodebuffer.rs

Lines 117 to 144 in b093ade

	/*
	const BATCH_SIZE: usize = 32;
	let full_copies = match_length / BATCH_SIZE;
	let partial_copy_size = match_length % BATCH_SIZE;
	let mut buf = [0u8; BATCH_SIZE];
	for x in 0..full_copies {
	let idx = start_idx + x * BATCH_SIZE;
	let source = &self.buffer.as_slice()[idx..idx + BATCH_SIZE];
	buf[0..BATCH_SIZE].copy_from_slice(source);
	self.buffer.extend(&buf[0..BATCH_SIZE]);
	}
	let idx = start_idx + full_copies * BATCH_SIZE;
	let source = &self.buffer.as_slice()[idx..idx + partial_copy_size];
	buf[0..partial_copy_size].copy_from_slice(source);
	self.buffer.extend(&buf[0..partial_copy_size]);
	*/
	// using this unsafe block instead of the above increases performance by ca 5% when decoding the enwik9 dataset
	self.buffer.reserve(match_length);
	unsafe {
	self.buffer.set_len(self.buffer.len() + match_length);
	let slice = &mut self.buffer[start_idx..];
	let src = slice.as_mut_ptr();
	let dst = src.add(slice.len() - match_length);
	std::ptr::copy_nonoverlapping(src, dst, match_length);
	}

I just saw the tracking issue and I immediately remembered your crate could use something like this 😃

[KillingSpark]

Yep that is basically what this code needs. Good to have an issue for that here too, thanks :)

Do you know how fast these things normally get stabilized? I don't follow new features that closely before they get released.

[paolobarbolini]

Do you know how fast these things normally get stabilized?

It depends on many factors, including if anything new comes up in the next months. For example core::slice::fill took about 10 months, if we also count the ~3 months it took for two versions to go by for it go get into beta and then finally into stable (source: rust-lang/rust#70758)

If we have nightly users who are really interested into making things like cargo geiger happier, a nightly feature could be added to conditionally replace the unsafe function with the safe one.

[paolobarbolini]

Looks like this is going to come with 1.53 rust-lang/rust#84642

[Shnatsel]

Vec::extend_from_within is now stabilized, so the last unsafe block in this codebase can be dropped!

I wrote the RFC for Vec::extend_from_within with use cases like this in mind, and I'm very happy to see it finally land!

[KillingSpark]

this is great! I am not familiar with how this is usually handled. Is it ok to just require all my users to have rustc with the latest version available or do I do a conditional compilation for some time and remove the unsafe code at a later time?

[Shnatsel]

Long story short, there is no clear consensus.

https://crates.io/crates/rustversion is used by quote, thiserror, anyhow and other major crates. It provides conditional compilation.

Some widely used libraries take the conservative approach and only use functions requiring a newer stdlib after the required version has percolated everywhere.

On the other end of the spectrum, some projects explicitly require the latest compiler at all times and don't support anything else.

[KillingSpark]

Well I guess since this is the only update and I don't see any big other changes in the nearish future, just making a new release stating that this only trades support for older compiler for removing of unsafe code should be fine.

People requiring older compilers can just use the older version then, it's functionally equivalent.

[CodesInChaos]

Perhaps add #![forbid(unsafe_code)] to the crate now?

[KillingSpark]

Done! I think that concludes this issue :)