sunchao commented on code in PR #5042:
URL: https://github.com/apache/datafusion-comet/pull/5042#discussion_r3837586414
##########
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:
[P2] Bound the retained capacity of the scratch vectors
Could you cap the cached capacity or keep these buffers at batch/task scope?
`Vec::resize` reduces the length but never releases capacity, so moving the
vectors into TLS retains the largest allocation until the worker thread exits,
including after the query finishes. I ran the exact-head kernels with two equal
16,000,000-character ASCII strings and threshold 0, then dropped the input and
evaluated a one-character pair. The two vectors still retained 128,000,008
bytes after that small call. These are valid inputs and the thresholded case
completes in linear time. Comet's long-lived worker threads can therefore
retain this allocation independently, outside the query memory pool. The
previous call-local vectors released it on return.
--
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]
