Skip to content

Commit

Permalink
ruff_source_file: remove bound check elision
Browse files Browse the repository at this point in the history
Eliding bounds checks very rarely results in a meaningful performance
improvement. This is usually because branch predictors are very good,
and in cases like this, the branch predictor likely predicts perfectly
given that the code is correct.

It looks like this use of unsafe was added in #3985 as part of an
optimization to use `memchr`. But `memchr` is where the real win is.

Benchmarks for before and after:

    $ critcmp main test
    group                                       main                                   test
    -----                                       ----                                   ----
    linter/all-rules/large/dataset.py           1.01      5.2±0.03ms     7.8 MB/sec    1.00      5.2±0.01ms     7.9 MB/sec
    linter/all-rules/numpy/ctypeslib.py         1.00   1375.3±6.38µs    12.1 MB/sec    1.00  1379.3±10.77µs    12.1 MB/sec
    linter/all-rules/numpy/globals.py           1.01    157.1±0.63µs    18.8 MB/sec    1.00    155.5±0.73µs    19.0 MB/sec
    linter/all-rules/pydantic/types.py          1.00      2.6±0.00ms     9.9 MB/sec    1.00      2.6±0.00ms     9.9 MB/sec
    linter/all-rules/unicode/pypinyin.py        1.00    647.7±1.27µs     6.5 MB/sec    1.00    647.5±3.46µs     6.5 MB/sec
    linter/default-rules/large/dataset.py       1.00      2.3±0.00ms    17.5 MB/sec    1.00      2.3±0.00ms    17.6 MB/sec
    linter/default-rules/numpy/ctypeslib.py     1.00    456.1±1.13µs    36.5 MB/sec    1.00    458.3±1.53µs    36.3 MB/sec
    linter/default-rules/numpy/globals.py       1.00     39.7±0.21µs    74.2 MB/sec    1.00     39.7±0.23µs    74.3 MB/sec
    linter/default-rules/pydantic/types.py      1.01   1005.8±5.49µs    25.4 MB/sec    1.00    995.5±2.99µs    25.6 MB/sec
    linter/default-rules/unicode/pypinyin.py    1.01    139.9±0.91µs    30.0 MB/sec    1.00    138.0±0.22µs    30.5 MB/sec

Typically, eliding bounds checks makes the most sense when it unlocks
some other kind of optimization (e.g., autovectorization).
  • Loading branch information
BurntSushi committed Nov 28, 2023
1 parent 8900f7b commit b72603d
Showing 1 changed file with 1 addition and 5 deletions.
6 changes: 1 addition & 5 deletions crates/ruff_source_file/src/newlines.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,7 @@ impl<'a> UniversalNewlineIterator<'a> {
pub fn find_newline(text: &str) -> Option<(usize, LineEnding)> {
let bytes = text.as_bytes();
if let Some(position) = memchr2(b'\n', b'\r', bytes) {
// SAFETY: memchr guarantees to return valid positions
#[allow(unsafe_code)]
let newline_character = unsafe { *bytes.get_unchecked(position) };

let line_ending = match newline_character {
let line_ending = match bytes[position] {
// Explicit branch for `\n` as this is the most likely path
b'\n' => LineEnding::Lf,
// '\r\n'
Expand Down

0 comments on commit b72603d

Please sign in to comment.