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

chore(ci): enable overflow-checks in ci profile #16895

Merged
merged 10 commits into from
Nov 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -573,7 +573,7 @@ incremental = true

[profile.ci]
inherits = "release"
overflow-checks = false
overflow-checks = true
incremental = false
debug-assertions = true

Expand All @@ -583,7 +583,7 @@ debug-assertions = true

[profile.dev]
split-debuginfo = "unpacked"
overflow-checks = false
overflow-checks = true
# Report to https://github.com/rust-lang/rust/issues/100142 if incremental works well
incremental = true

Expand Down
50 changes: 20 additions & 30 deletions src/common/column/src/buffer/iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,20 +20,20 @@ use super::Buffer;
/// This crates' equivalent of [`std::vec::IntoIter`] for [`Buffer`].
#[derive(Debug, Clone)]
pub struct IntoIter<T: Copy> {
values: Buffer<T>,
index: usize,
end: usize,
buffer: Buffer<T>,
current: *const T,
end: *const T,
}

impl<T: Copy> IntoIter<T> {
/// Creates a new [`Buffer`]
#[inline]
pub fn new(values: Buffer<T>) -> Self {
let end = values.len();
pub fn new(buffer: Buffer<T>) -> Self {
let ptr = buffer.as_ptr();
let len = buffer.len();
Self {
values,
index: 0,
end,
current: ptr,
end: unsafe { ptr.add(len) },
buffer,
}
}
}
Expand All @@ -43,40 +43,30 @@ impl<T: Copy> Iterator for IntoIter<T> {

#[inline]
fn next(&mut self) -> Option<Self::Item> {
if self.index == self.end {
return None;
if self.current == self.end {
None
} else {
let value = unsafe { *self.current };
self.current = unsafe { self.current.add(1) };
Some(value)
}
let old = self.index;
self.index += 1;
Some(*unsafe { self.values.get_unchecked(old) })
}

#[inline]
fn size_hint(&self) -> (usize, Option<usize>) {
(self.end - self.index, Some(self.end - self.index))
}

#[inline]
fn nth(&mut self, n: usize) -> Option<Self::Item> {
let new_index = self.index + n;
if new_index > self.end {
self.index = self.end;
None
} else {
self.index = new_index;
self.next()
}
let len = unsafe { self.end.offset_from(self.current) } as usize;
(len, Some(len))
}
}

impl<T: Copy> DoubleEndedIterator for IntoIter<T> {
#[inline]
fn next_back(&mut self) -> Option<Self::Item> {
if self.index == self.end {
if self.current == self.end {
None
} else {
self.end -= 1;
Some(*unsafe { self.values.get_unchecked(self.end) })
self.end = unsafe { self.end.sub(1) };
Some(unsafe { *self.end })
}
}
}
Expand Down
82 changes: 82 additions & 0 deletions src/query/expression/benches/bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
extern crate criterion;

use criterion::Criterion;
use databend_common_column::buffer::Buffer;
use databend_common_expression::arrow::deserialize_column;
use databend_common_expression::arrow::serialize_column;
use databend_common_expression::types::BinaryType;
Expand Down Expand Up @@ -130,6 +131,81 @@ fn bench(c: &mut Criterion) {
});
}
}

for length in [10240, 102400] {
let (left, right) = generate_random_int_data(&mut rng, length);

group.bench_function(format!("function_iterator_iterator_v1/{length}"), |b| {
b.iter(|| {
let left = left.clone();
let right = right.clone();

let _c = left
.into_iter()
.zip(right.into_iter())
.map(|(a, b)| a + b)
.collect::<Vec<i32>>();
})
});

group.bench_function(format!("function_iterator_iterator_v2/{length}"), |b| {
b.iter(|| {
let _c = left
.iter()
.cloned()
.zip(right.iter().cloned())
.map(|(a, b)| a + b)
.collect::<Vec<i32>>();
})
});

group.bench_function(
format!("function_buffer_index_unchecked_iterator/{length}"),
|b| {
b.iter(|| {
let _c = (0..length)
.map(|i| unsafe { left.get_unchecked(i) + right.get_unchecked(i) })
.collect::<Vec<i32>>();
})
},
);

group.bench_function(
format!("function_slice_index_unchecked_iterator/{length}"),
|b| {
b.iter(|| {
let left = left.as_slice();
let right = right.as_slice();

let _c = (0..length)
.map(|i| unsafe { left.get_unchecked(i) + right.get_unchecked(i) })
.collect::<Vec<i32>>();
})
},
);

let left_vec: Vec<_> = left.iter().cloned().collect();
let right_vec: Vec<_> = right.iter().cloned().collect();

group.bench_function(
format!("function_vec_index_unchecked_iterator/{length}"),
|b| {
b.iter(|| {
let _c = (0..length)
.map(|i| unsafe { left_vec.get_unchecked(i) + right_vec.get_unchecked(i) })
.collect::<Vec<i32>>();
})
},
);

group.bench_function(format!("function_buffer_index_iterator/{length}"), |b| {
b.iter(|| {
let _c = (0..length)
.map(|i| left[i] + right[i])
.collect::<Vec<i32>>();
})
});
}
}

criterion_group!(benches, bench);
Expand All @@ -155,3 +231,9 @@ fn generate_random_string_data(rng: &mut StdRng, length: usize) -> (Vec<String>,

(iter_str, iter_binary)
}

fn generate_random_int_data(rng: &mut StdRng, length: usize) -> (Buffer<i32>, Buffer<i32>) {
let s: Buffer<i32> = (0..length).map(|_| rng.gen_range(-1000..1000)).collect();
let b: Buffer<i32> = (0..length).map(|_| rng.gen_range(-1000..1000)).collect();
(s, b)
}
10 changes: 5 additions & 5 deletions src/query/expression/src/filter/like.rs
Original file line number Diff line number Diff line change
Expand Up @@ -304,13 +304,13 @@ fn find(mut haystack: &[u8], needle: &[u8]) -> Option<usize> {
return None;
}
// Inspired by fast_strstr (https://github.com/RaphaelJ/fast_strstr).
let mut checksum = 0;
let mut checksum: i64 = 0;
for i in 0..needle_len {
// # Safety
// `needle_len` <= haystack_len
unsafe {
checksum += haystack.get_unchecked(i);
checksum -= needle.get_unchecked(i);
checksum += *haystack.get_unchecked(i) as i64;
checksum -= *needle.get_unchecked(i) as i64;
}
}
let mut idx = 0;
Expand All @@ -331,8 +331,8 @@ fn find(mut haystack: &[u8], needle: &[u8]) -> Option<usize> {
// # Safety
// `idx` < `haystack_len` and `idx` + `needle_len` < `haystack_len`.
unsafe {
checksum -= haystack.get_unchecked(idx);
checksum += haystack.get_unchecked(idx + needle_len);
checksum -= *haystack.get_unchecked(idx) as i64;
checksum += *haystack.get_unchecked(idx + needle_len) as i64;
}
idx += 1;
}
Expand Down
6 changes: 3 additions & 3 deletions src/query/expression/src/utils/block_debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ fn create_box_table(
let row_count: usize = results.iter().map(|block| block.num_rows()).sum();
let mut rows_to_render = row_count.min(max_rows);

if row_count <= max_rows + 3 {
if row_count <= max_rows.saturating_add(3) {
// hiding rows adds 3 extra rows
// so hiding rows makes no sense if we are only slightly over the limit
// if we are 1 row over the limit hiding rows will actually increase the number of lines we display!
Expand Down Expand Up @@ -228,7 +228,7 @@ fn create_box_table(
// "..." take up three lengths
if max_width > 0 {
(widths, column_map) =
compute_render_widths(schema, max_width, max_col_width + 3, &res_vec);
compute_render_widths(schema, max_width, max_col_width.saturating_add(3), &res_vec);
}

let mut header = Vec::with_capacity(schema.fields().len());
Expand Down Expand Up @@ -355,7 +355,7 @@ fn compute_render_widths(
for field in schema.fields() {
// head_name = field_name + "\n" + field_data_type
let col_length = field.name().len().max(field.data_type().to_string().len());
widths.push(col_length + 3);
widths.push(col_length.saturating_add(3));
}

for values in results {
Expand Down
2 changes: 1 addition & 1 deletion src/query/service/src/sessions/session_ctx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ impl SessionContext {
index
};

if idx < 0 || idx > (query_ids_len - 1) as i32 {
if query_ids_len < 1 || idx < 0 || idx > (query_ids_len - 1) as i32 {
return "".to_string();
}

Expand Down
2 changes: 1 addition & 1 deletion src/query/service/src/sessions/session_mgr_status.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ impl SessionManagerStatus {
}

pub(crate) fn query_finish(&mut self, now: SystemTime) {
self.running_queries_count -= 1;
self.running_queries_count = self.running_queries_count.saturating_sub(1);
if self.running_queries_count == 0 {
self.last_query_finished_at = Some(now)
}
Expand Down
2 changes: 1 addition & 1 deletion tests/suites/1_stateful/02_query/02_0000_kill_query.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
res = mycursor.fetchone()
kill_query = "kill query " + str(res[0]) + ";"
mycursor.execute(kill_query)
time.sleep(1)
time.sleep(10)
mycursor.execute(
"SELECT * FROM system.processes WHERE extra_info LIKE '%SELECT max(number)%' AND extra_info NOT LIKE '%system.processes%';"
)
Expand Down
Loading