neilconway commented on code in PR #26039:
URL: https://github.com/apache/datafusion/pull/26039#discussion_r4186481784
##########
datafusion/functions/src/strings.rs:
##########
@@ -1217,6 +1219,73 @@ pub(crate) fn append_view(
views_buffer.push(sub_view);
}
+/// Values of at most this many bytes are stored inline in their view.
+const MAX_INLINE_LEN: usize = MAX_INLINE_VIEW_LEN as usize;
+
+/// Returns the view for a substring of the value an existing view refers to.
+///
+/// # Arguments
+/// - view: The original view value
+/// - source: The bytes the original view refers to
+/// - range: The byte range within `source` of the substring to return a view
for
+///
+/// Substrings longer than 12 bytes point into the same data buffer as `view`;
+/// shorter ones are stored inline in the new view.
+///
+/// This uses shifts and masks rather than [`make_view`], which picks copy code
+/// based on the substring's length; the CPU often mispredicts that choice when
+/// lengths vary from row to row.
+#[inline]
+pub(crate) fn sub_view(view: u128, source: &[u8], range: Range<usize>) -> u128
{
+ debug_assert!(range.start <= range.end && range.end <= source.len());
+
+ // The substring's first bytes, in the low-order bits. Any bits past the
+ // end of the substring are masked off by `inline_view`.
+ let leading_bytes = if source.len() <= MAX_INLINE_LEN {
+ // `source` is stored in `view` itself, after its 4-byte length.
+ (view >> 32) >> (8 * range.start)
+ } else {
+ // `source` has more than 12 bytes, so read the 12 bytes starting at
+ // `range.start`, or the last 12 bytes if that would run past the end,
+ // and skip any that come before `range.start`.
+ let window_start = range.start.min(source.len() - MAX_INLINE_LEN);
+ let window = source[window_start..window_start + MAX_INLINE_LEN]
+ .try_into()
+ .unwrap();
+ read_12_bytes(window) >> (8 * (range.start - window_start))
+ };
+
+ let len = range.len();
+ if len <= MAX_INLINE_LEN {
+ inline_view(leading_bytes, len)
+ } else {
+ let original = ByteView::from(view);
+ ByteView {
+ length: len as u32,
+ prefix: leading_bytes as u32,
+ offset: original.offset + range.start as u32,
+ ..original
+ }
+ .as_u128()
+ }
+}
Review Comment:
Yep, makes sense!
--
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]