Avoid index check in char::to_lowercase and char::to_uppercase #150520
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
|
I'm kind of shocked that this wouldn't get elided? Using the result of a search should be easily known to be in bounds of the same array. |
414b551 to
262426d
Compare
|
@asquared31415 the following project suggests that it is indeed the case
[package]
name = "a"
version = "0.1.0"
edition = "2024"
[lib]
crate-type = ["cdylib"]
[dependencies]
wasi = "0.11"
[profile.release]
codegen-units = 1
#![no_std]
#![no_main]
use core::slice;
use core::str;
use wasi;
#[unsafe(no_mangle)]
pub extern "C" fn _str_lowercase(ptr: *const u8, len: usize, mut dest: *mut u8) -> usize {
unsafe {
let s = str::from_utf8_unchecked(slice::from_raw_parts(ptr, len));
let mut len = 0;
for ch in s.chars().flat_map(char::to_lowercase) {
let bytes: &mut [u8; 4] = &mut *(dest as *mut [u8; 4]);
let s = ch.encode_utf8(bytes);
len += s.len();
dest = dest.add(s.len());
}
len
}
}Compiling with Adding |
|
@bors2 try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
Avoid index check in char::to_lowercase and char::to_uppercase
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (6a7e2e6): comparison URL. Overall result: no relevant changes - no action neededBenchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf. @bors rollup=never Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)This benchmark run did not return any relevant results for this metric. CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 482.208s -> 480.343s (-0.39%) |
|
Not particularly surprising, as most cases likely optimize the check already. Given that there is a demo where it doesn't, I'm fine with this change. @bors r+ |
|
☀️ Test successful - checks-actions |
What is this?This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.Comparing 4862272 (parent) -> 4c62aa4 (this PR) Test differencesShow 2 test diffs2 doctest diffs were found. These are ignored, as they are noisy. Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard 4c62aa446803c8f31eafdc0c0119209ff129b3e0 --output-dir test-dashboardAnd then open Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
|
Finished benchmarking commit (4c62aa4): comparison URL. Overall result: ✅ improvements - no action needed@rustbot label: -perf-regression Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary -2.7%, secondary 2.7%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary -7.1%, secondary -12.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 477.392s -> 476.46s (-0.20%) |
This generates a panic free code, with is helpful for smaller binary sizes.