From 0aec5e6dea77d105a02521b80b3602f68bdcf7a1 Mon Sep 17 00:00:00 2001 From: sundyli <543950155@qq.com> Date: Thu, 21 Nov 2024 08:16:19 +0800 Subject: [PATCH 1/9] chore(ci): enable overflow-checks in ci profile --- Cargo.lock | 1 - Cargo.toml | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 835bbf246e22..a728bb09b20a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3325,7 +3325,6 @@ dependencies = [ "chrono-tz 0.8.6", "comfy-table", "criterion", - "dashmap 6.1.0", "databend-common-ast", "databend-common-base", "databend-common-column", diff --git a/Cargo.toml b/Cargo.toml index bca41bfd82e7..ea31c3f19537 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -573,7 +573,7 @@ incremental = true [profile.ci] inherits = "release" -overflow-checks = false +overflow-checks = true incremental = false debug-assertions = true From 6eea7e17312ace0492d2a9bfddf248ed48abe883 Mon Sep 17 00:00:00 2001 From: sundy-li <543950155@qq.com> Date: Thu, 21 Nov 2024 15:24:10 +0800 Subject: [PATCH 2/9] update --- Cargo.toml | 2 +- src/query/service/src/interpreters/interpreter.rs | 10 +++++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index ea31c3f19537..86df409276a0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -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 diff --git a/src/query/service/src/interpreters/interpreter.rs b/src/query/service/src/interpreters/interpreter.rs index f7b8d2771e5a..18f13b94c2c5 100644 --- a/src/query/service/src/interpreters/interpreter.rs +++ b/src/query/service/src/interpreters/interpreter.rs @@ -189,12 +189,16 @@ fn log_query_start(ctx: &QueryContext) { } fn log_query_finished(ctx: &QueryContext, error: Option, has_profiles: bool) { - InterpreterMetrics::record_query_finished(ctx, error.clone()); - let now = SystemTime::now(); let session = ctx.get_current_session(); + let mut status = session.get_status().write(); + // already finished + if status.last_query_finished_at.is_some() { + return; + } - session.get_status().write().query_finish(); + status.query_finish(); + InterpreterMetrics::record_query_finished(ctx, error.clone()); let typ = session.get_type(); if typ.is_user_session() { SessionManager::instance().status.write().query_finish(now); From 9b8dd94b12d7f3c8ba2de9a0fa585c0b809e5927 Mon Sep 17 00:00:00 2001 From: sundy-li <543950155@qq.com> Date: Thu, 21 Nov 2024 15:53:13 +0800 Subject: [PATCH 3/9] update --- src/query/service/src/interpreters/interpreter.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/query/service/src/interpreters/interpreter.rs b/src/query/service/src/interpreters/interpreter.rs index 18f13b94c2c5..cb8a0f91684d 100644 --- a/src/query/service/src/interpreters/interpreter.rs +++ b/src/query/service/src/interpreters/interpreter.rs @@ -191,7 +191,8 @@ fn log_query_start(ctx: &QueryContext) { fn log_query_finished(ctx: &QueryContext, error: Option, has_profiles: bool) { let now = SystemTime::now(); let session = ctx.get_current_session(); - let mut status = session.get_status().write(); + let status = session.get_status(); + let mut status = status.write(); // already finished if status.last_query_finished_at.is_some() { return; From ec571279ec523b4cc61e271c04e5ebfd21e02ce2 Mon Sep 17 00:00:00 2001 From: sundy-li <543950155@qq.com> Date: Thu, 21 Nov 2024 15:55:37 +0800 Subject: [PATCH 4/9] update --- tests/suites/1_stateful/02_query/02_0000_kill_query.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/suites/1_stateful/02_query/02_0000_kill_query.py b/tests/suites/1_stateful/02_query/02_0000_kill_query.py index 590a8821f1cc..d538411d4662 100755 --- a/tests/suites/1_stateful/02_query/02_0000_kill_query.py +++ b/tests/suites/1_stateful/02_query/02_0000_kill_query.py @@ -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%';" ) From 6c5a3cd8a3ad0c6ec4d5b5a294cc899dc6806a3a Mon Sep 17 00:00:00 2001 From: sundy-li <543950155@qq.com> Date: Thu, 21 Nov 2024 17:54:39 +0800 Subject: [PATCH 5/9] update --- src/common/column/src/buffer/iterator.rs | 50 +++++------ src/query/expression/benches/bench.rs | 82 +++++++++++++++++++ src/query/expression/src/filter/like.rs | 6 +- src/query/service/src/sessions/session_ctx.rs | 2 +- 4 files changed, 106 insertions(+), 34 deletions(-) diff --git a/src/common/column/src/buffer/iterator.rs b/src/common/column/src/buffer/iterator.rs index 91b4d351b5d5..5771de1ec7d3 100644 --- a/src/common/column/src/buffer/iterator.rs +++ b/src/common/column/src/buffer/iterator.rs @@ -20,20 +20,20 @@ use super::Buffer; /// This crates' equivalent of [`std::vec::IntoIter`] for [`Buffer`]. #[derive(Debug, Clone)] pub struct IntoIter { - values: Buffer, - index: usize, - end: usize, + buffer: Buffer, + current: *const T, + end: *const T, } impl IntoIter { - /// Creates a new [`Buffer`] #[inline] - pub fn new(values: Buffer) -> Self { - let end = values.len(); + pub fn new(buffer: Buffer) -> Self { + let ptr = buffer.as_ptr(); + let len = buffer.len(); Self { - values, - index: 0, - end, + current: ptr, + end: unsafe { ptr.add(len) }, + buffer, } } } @@ -43,40 +43,30 @@ impl Iterator for IntoIter { #[inline] fn next(&mut self) -> Option { - 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) { - (self.end - self.index, Some(self.end - self.index)) - } - - #[inline] - fn nth(&mut self, n: usize) -> Option { - 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 DoubleEndedIterator for IntoIter { #[inline] fn next_back(&mut self) -> Option { - 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 }) } } } diff --git a/src/query/expression/benches/bench.rs b/src/query/expression/benches/bench.rs index b0e18083fc80..c4d1ffd8ef6f 100644 --- a/src/query/expression/benches/bench.rs +++ b/src/query/expression/benches/bench.rs @@ -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; @@ -130,6 +131,81 @@ fn bench(c: &mut Criterion) { }); } } + + for length in [10240] { + 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::>(); + }) + }); + + 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::>(); + }) + }); + + 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::>(); + }) + }, + ); + + 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::>(); + }) + }, + ); + + 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::>(); + }) + }, + ); + + group.bench_function(format!("function_buffer_index_iterator/{length}"), |b| { + b.iter(|| { + let _c = (0..length) + .map(|i| left[i] + right[i]) + .collect::>(); + }) + }); + } } criterion_group!(benches, bench); @@ -155,3 +231,9 @@ fn generate_random_string_data(rng: &mut StdRng, length: usize) -> (Vec, (iter_str, iter_binary) } + +fn generate_random_int_data(rng: &mut StdRng, length: usize) -> (Buffer, Buffer) { + let s: Buffer = (0..length).map(|_| rng.gen_range(-1000..1000)).collect(); + let b: Buffer = (0..length).map(|_| rng.gen_range(-1000..1000)).collect(); + (s, b) +} diff --git a/src/query/expression/src/filter/like.rs b/src/query/expression/src/filter/like.rs index de5a0ba9d17e..1594070f5db5 100644 --- a/src/query/expression/src/filter/like.rs +++ b/src/query/expression/src/filter/like.rs @@ -304,13 +304,13 @@ fn find(mut haystack: &[u8], needle: &[u8]) -> Option { 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 _; + checksum -= needle.get_unchecked(i) as _; } } let mut idx = 0; diff --git a/src/query/service/src/sessions/session_ctx.rs b/src/query/service/src/sessions/session_ctx.rs index 0a0e03b6d4fc..f79f5c5c076e 100644 --- a/src/query/service/src/sessions/session_ctx.rs +++ b/src/query/service/src/sessions/session_ctx.rs @@ -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(); } From ed464083e799122c4d505126f75737e48a952f06 Mon Sep 17 00:00:00 2001 From: sundy-li <543950155@qq.com> Date: Thu, 21 Nov 2024 17:58:15 +0800 Subject: [PATCH 6/9] update --- src/query/expression/src/filter/like.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/query/expression/src/filter/like.rs b/src/query/expression/src/filter/like.rs index 1594070f5db5..0de572a947cc 100644 --- a/src/query/expression/src/filter/like.rs +++ b/src/query/expression/src/filter/like.rs @@ -309,8 +309,8 @@ fn find(mut haystack: &[u8], needle: &[u8]) -> Option { // # Safety // `needle_len` <= haystack_len unsafe { - checksum += haystack.get_unchecked(i) as _; - checksum -= needle.get_unchecked(i) as _; + checksum += *haystack.get_unchecked(i) as i64; + checksum -= *needle.get_unchecked(i) as i64; } } let mut idx = 0; @@ -331,8 +331,8 @@ fn find(mut haystack: &[u8], needle: &[u8]) -> Option { // # 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; } From 1c8b65c285ddd82d4937530696e4173433f10c38 Mon Sep 17 00:00:00 2001 From: sundy-li <543950155@qq.com> Date: Thu, 21 Nov 2024 18:23:45 +0800 Subject: [PATCH 7/9] update --- src/query/expression/benches/bench.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/query/expression/benches/bench.rs b/src/query/expression/benches/bench.rs index c4d1ffd8ef6f..8ff91219fa5c 100644 --- a/src/query/expression/benches/bench.rs +++ b/src/query/expression/benches/bench.rs @@ -132,7 +132,7 @@ fn bench(c: &mut Criterion) { } } - for length in [10240] { + 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| { From 9d6183f44e7873bf9868c86144c1f9f33caf1324 Mon Sep 17 00:00:00 2001 From: sundy-li <543950155@qq.com> Date: Thu, 21 Nov 2024 20:26:43 +0800 Subject: [PATCH 8/9] update --- src/query/expression/src/utils/block_debug.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/query/expression/src/utils/block_debug.rs b/src/query/expression/src/utils/block_debug.rs index 05f877215ea5..d6ffa2191cdc 100644 --- a/src/query/expression/src/utils/block_debug.rs +++ b/src/query/expression/src/utils/block_debug.rs @@ -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! @@ -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()); @@ -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 { From a124031ad8534470758833fb6655b497ddef1633 Mon Sep 17 00:00:00 2001 From: sundy-li <543950155@qq.com> Date: Thu, 21 Nov 2024 21:48:43 +0800 Subject: [PATCH 9/9] update --- src/query/service/src/interpreters/interpreter.rs | 11 +++-------- src/query/service/src/sessions/session_mgr_status.rs | 2 +- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/src/query/service/src/interpreters/interpreter.rs b/src/query/service/src/interpreters/interpreter.rs index cb8a0f91684d..f7b8d2771e5a 100644 --- a/src/query/service/src/interpreters/interpreter.rs +++ b/src/query/service/src/interpreters/interpreter.rs @@ -189,17 +189,12 @@ fn log_query_start(ctx: &QueryContext) { } fn log_query_finished(ctx: &QueryContext, error: Option, has_profiles: bool) { + InterpreterMetrics::record_query_finished(ctx, error.clone()); + let now = SystemTime::now(); let session = ctx.get_current_session(); - let status = session.get_status(); - let mut status = status.write(); - // already finished - if status.last_query_finished_at.is_some() { - return; - } - status.query_finish(); - InterpreterMetrics::record_query_finished(ctx, error.clone()); + session.get_status().write().query_finish(); let typ = session.get_type(); if typ.is_user_session() { SessionManager::instance().status.write().query_finish(now); diff --git a/src/query/service/src/sessions/session_mgr_status.rs b/src/query/service/src/sessions/session_mgr_status.rs index 69e43bea1afa..2db67041972a 100644 --- a/src/query/service/src/sessions/session_mgr_status.rs +++ b/src/query/service/src/sessions/session_mgr_status.rs @@ -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) }