Skip to content

chore: add utilities to LanceBuffer#2784

Merged
broccoliSpicy merged 8 commits intolance-format:mainfrom
westonpace:chore/add-buffer-utils
Aug 26, 2024
Merged

chore: add utilities to LanceBuffer#2784
broccoliSpicy merged 8 commits intolance-format:mainfrom
westonpace:chore/add-buffer-utils

Conversation

@westonpace
Copy link
Member

This is the first in a series of PRs that eventually reworks the v2 encoders to use DataBlock. This PR doesn't add any public facing capability on its own but merely adds a few useful utilities to LanceBuffer.

@github-actions github-actions bot added the chore label Aug 25, 2024
@codecov-commenter
Copy link

codecov-commenter commented Aug 25, 2024

Codecov Report

Attention: Patch coverage is 77.84810% with 35 lines in your changes missing coverage. Please review.

Project coverage is 79.29%. Comparing base (1e6ee60) to head (46e2a8a).
Report is 1 commits behind head on main.

Files Patch % Lines
rust/lance-encoding/src/buffer.rs 77.84% 34 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2784      +/-   ##
==========================================
- Coverage   79.30%   79.29%   -0.02%     
==========================================
  Files         227      227              
  Lines       68268    68426     +158     
  Branches    68268    68426     +158     
==========================================
+ Hits        54141    54257     +116     
- Misses      11006    11037      +31     
- Partials     3121     3132      +11     
Flag Coverage Δ
unittests 79.29% <77.84%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

}
}

// Mostly useful for unit testing. It is zero-copy
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

huh, Vec::from array copies the stack located array data into heap?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, you are right. I read that "it moves the values into the Vec" and thought that meant zero-copy but it just means the items inside the Vec are moved and not cloned.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed the impl into a copy_array method and updated the comment. I'd like to keep all From implementations zero-copy (again, trying to make it obvious when a copy of data happens)

@broccoliSpicy
Copy link
Contributor

LGTM!

@broccoliSpicy broccoliSpicy merged commit b43dfaf into lance-format:main Aug 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants