neilconway commented on code in PR #22029:
URL: https://github.com/apache/datafusion/pull/22029#discussion_r3235411309
##########
datafusion/functions/src/strings.rs:
##########
@@ -538,6 +584,79 @@ pub(crate) const STRING_VIEW_INIT_BLOCK_SIZE: u32 = 8 *
1024;
/// grows to; matches Arrow's `GenericByteViewBuilder` default.
pub(crate) const STRING_VIEW_MAX_BLOCK_SIZE: u32 = 2 * 1024 * 1024;
+/// Append-only writer handed to closures passed to `append_with`.
+pub(crate) trait StringWriter {
+ fn write_str(&mut self, s: &str);
+ fn write_char(&mut self, c: char);
+}
+
+/// [`StringWriter`] for [`GenericStringArrayBuilder`]. Writes go straight to
+/// the value buffer.
+pub(crate) struct GenericStringWriter<'a> {
+ value_buffer: &'a mut MutableBuffer,
+}
+
+impl StringWriter for GenericStringWriter<'_> {
+ #[inline(always)]
+ fn write_str(&mut self, s: &str) {
+ push_bytes_to_mutable_buffer(self.value_buffer, s.as_bytes());
+ }
+
+ #[inline(always)]
+ fn write_char(&mut self, c: char) {
+ push_char_to_mutable_buffer(self.value_buffer, c);
+ }
+}
+
+/// Write `bytes` into `value_buffer`. For repeated small writes,
+/// MutableBuffer::extend_from_slice can be slow (memcpy per call), so we
extend
+/// the buffer here directly and force inlining.
+#[inline(always)]
+fn push_bytes_to_mutable_buffer(value_buffer: &mut MutableBuffer, bytes:
&[u8]) {
+ let n = bytes.len();
+ let old_len = value_buffer.len();
+ value_buffer.reserve(n);
+
+ // SAFETY: we reserved `n` bytes; the source and destination do not alias
+ // because `bytes` was passed in by the caller and `value_buffer` is owned.
+ unsafe {
+ let dst = value_buffer.as_mut_ptr().add(old_len);
+ let src = bytes.as_ptr();
+ match n {
+ 0 => {}
+ 1 => std::ptr::copy_nonoverlapping(src, dst, 1),
+ 2 => std::ptr::copy_nonoverlapping(src, dst, 2),
+ 3 => std::ptr::copy_nonoverlapping(src, dst, 3),
+ 4 => std::ptr::copy_nonoverlapping(src, dst, 4),
+ 5 => std::ptr::copy_nonoverlapping(src, dst, 5),
+ 6 => std::ptr::copy_nonoverlapping(src, dst, 6),
+ 7 => std::ptr::copy_nonoverlapping(src, dst, 7),
+ 8 => std::ptr::copy_nonoverlapping(src, dst, 8),
+ _ => std::ptr::copy_nonoverlapping(src, dst, n),
+ }
+ value_buffer.set_len(old_len + n);
+ }
+}
+
+#[inline(always)]
+fn push_char_to_mutable_buffer(value_buffer: &mut MutableBuffer, c: char) {
+ let len = c.len_utf8();
+ let old_len = value_buffer.len();
+ value_buffer.reserve(len);
+
+ // SAFETY: we reserved `len` bytes above, write valid UTF-8 into those
Review Comment:
This was the specific case I ran into with the "empty-`from`" benchmark
regression, so I believe this one earns its keep.
--
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]
