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

Potential use of core::hint::unreachable_unchecked to avoid bounds checks for all users #94

Closed
yescallop opened this issue Sep 21, 2021 · 2 comments
Labels

Comments

@yescallop
Copy link

yescallop commented Sep 21, 2021

Consider the following code. It's used as a wrapper in one of my projects to avoid bounds checks in safe code.

#[inline]
fn chr(s: &[u8], b: u8) -> Option<usize> {
    memchr::memchr(b, s).map(|i| {
        if i >= s.len() {
            unsafe { core::hint::unreachable_unchecked() }
        }
        i
    })
}

I wonder if it's practical and sound to insert unreachable hints into memchr crate, so that all its users could get an increase in performance. It's just a tentative suggestion that needs more discussion :)

@yescallop yescallop changed the title Potential use of std::hint::unreachable_unchecked to avoid bounds checks for all users Potential use of core::hint::unreachable_unchecked to avoid bounds checks for all users Sep 21, 2021
@BurntSushi
Copy link
Owner

Sorry, I don't quite understand the request here. I would say there are a few dimensions to it:

  1. Do you have a real benchmark where eliding the check improves things? If so, I'd love to see it, because it is somewhat surprising to me that it would help much. In particular, memchr is always going to result in at least a pointer load and a function call, so a single branch in addition to that probably isn't adding much overhead.
  2. More importantly, it's unclear to me why you have this conditional at all. Why not just elide if i >= s.len() { completely? Maybe what you're saying---without actually saying it---is that the conditional provides a way for other code to avoid bounds checks. If so, then I'd really appreciate more detail, particularly with respect to (1).
  3. How would you go about moving the hint to the inside this crate?

@BurntSushi
Copy link
Owner

Closing due to inactivity and the next steps here are unclear to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants