kazantsev-maksim commented on code in PR #5042:
URL: https://github.com/apache/datafusion-comet/pull/5042#discussion_r3874597256
##########
native/spark-expr/src/string_funcs/levenshtein.rs:
##########
@@ -26,67 +26,188 @@ use datafusion::common::{cast::as_generic_string_array,
DataFusionError, Result}
use datafusion::physical_plan::ColumnarValue;
use std::sync::Arc;
+// Thread-local scratch buffers to avoid heap allocations in the row
processing loop
+thread_local! {
+ static LEVENSHTEIN_SCRATCH: std::cell::RefCell<(Vec<i32>, Vec<i32>)> =
+ std::cell::RefCell::new((Vec::with_capacity(64),
Vec::with_capacity(64)));
Review Comment:
Thanks for pointing this out. I have addressed the retained thread-local
scratch memory finding (P2):
**Retained scratch buffer memory cap:**
Added an upper bound check (`MAX_RETAINED_CAPACITY = 1024`, ~4 KB). If
the buffer capacity exceeds this limit due to an occasional pathologically long
string, it gets reset/shrunk back to the bounded capacity on subsequent
evaluations via `prepare_scratch`:
```rust
const MAX_RETAINED_CAPACITY: usize = 1024;
#[inline]
fn prepare_scratch(buf: &mut Vec<i32>, len: usize, default_val: i32) {
if buf.capacity() > MAX_RETAINED_CAPACITY && len <=
MAX_RETAINED_CAPACITY {
*buf = Vec::with_capacity(MAX_RETAINED_CAPACITY);
}
buf.clear();
buf.resize(len, default_val);
}
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]