Skip to content

Commit

Permalink
chore(ci): enable overflow-checks in ci profile (#16895)
Browse files Browse the repository at this point in the history
* chore(ci): enable overflow-checks  in ci profile

* update

* update

* update

* update

* update

* update

* update

* update
  • Loading branch information
sundy-li authored Nov 22, 2024
1 parent 38c4411 commit 6f0ae45
Show file tree
Hide file tree
Showing 8 changed files with 115 additions and 43 deletions.
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

0 comments on commit 6f0ae45

Please sign in to comment.