fix: Slice packed text through its selection layer - #224
Open
SamuelSchlesinger wants to merge 2 commits into
Open
fix: Slice packed text through its selection layer#224SamuelSchlesinger wants to merge 2 commits into
SamuelSchlesinger wants to merge 2 commits into
Conversation
takeColumn slices a packed payload through packedTake, which caps the selection and shares the byte buffer. sliceColumn instead routes through materializePacked, decoding every row of the column to Text before taking the handful of rows asked for, so the cost of a small slice scales with the frame. drop, dropLast, range and takeLast all go through it, and every text column read from CSV is packed. The third case pins the bounds contract the other two representations already keep: a slice running past the end is rejected, not decoded.
sliceColumn decoded the whole column to a boxed Text vector before taking the requested rows, so a small slice cost O(rows) rather than O(slice). Reuse packedGather, which shares the byte buffer, matching what takeColumn already does with packedTake. packedGather decodes an out-of-range index as the empty string where the boxed and unboxed arms reject the slice, so the bounds are checked first and the three representations agree. On a 300k-row CSV with two text columns, range (10, 20) drops from 16.2ms to 1us and drop 10 from 27.6ms to 2us.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
sliceColumnon aPackedTextcolumn materialized the whole column to boxedTextfirst, so a ten-row slice cost the whole frame and came back in a different representation. Gather the slice directly and keep it packed.Also adds the bounds check the boxed/unboxed arms already have —
packedGatherwould otherwise decode an overrun as empty strings.