Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: v2 fsl decode perf fix and some benchmarking utilities #2214

Merged
merged 5 commits into from
Apr 20, 2024

Conversation

westonpace
Copy link
Contributor

  • Add the ability to encode / decode in memory
  • Add some initial decoder benchmarks
  • Fix a perf bug (extra copy) in the fixed size list decoder
  • Add some initial file reader benchmarks
  • Add initial tracing to the decode path
  • Make batch readahead configurable

@codecov-commenter
Copy link

codecov-commenter commented Apr 17, 2024

Codecov Report

Attention: Patch coverage is 14.86486% with 63 lines in your changes are missing coverage. Please review.

Project coverage is 81.08%. Comparing base (bc89dcb) to head (95afe8f).

Files Patch % Lines
rust/lance-encoding/src/encoder.rs 0.00% 38 Missing ⚠️
rust/lance-encoding/src/lib.rs 0.00% 13 Missing ⚠️
rust/lance-encoding/src/decoder.rs 21.42% 11 Missing ⚠️
rust/lance-file/src/v2/reader.rs 85.71% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2214      +/-   ##
==========================================
- Coverage   81.15%   81.08%   -0.08%     
==========================================
  Files         185      186       +1     
  Lines       53508    53533      +25     
  Branches    53508    53533      +25     
==========================================
- Hits        43425    43406      -19     
- Misses       7609     7656      +47     
+ Partials     2474     2471       -3     
Flag Coverage Δ
unittests 81.08% <14.86%> (-0.08%) ⬇️

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.

.col(None, lance_datagen::array::rand_type(&DataType::Int32))
.into_batch_rows(lance_datagen::RowCount::from(1024 * 1024))
.unwrap();
let rt = tokio::runtime::Runtime::new().unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: instead of intializing a runtime inside of a loop, could we move to a static one? Here's an example of using std::sync::OnceLock to accomplish this:

https://github.com/ion-elgreco/delta-rs/blob/4a0e47ee1f82cc65c78a97a5722890fb962f0e34/python/src/utils.rs#L8-L12

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Though I just moved the runtime creation to the top of the benchmark method (outside the loop) instead of making it static.

 * Add the ability to encode / decode in memory
 * Add some initial decoder benchmarks
 * Fix a perf bug (extra copy) in the fixed size list decoder
 * Add some initial file reader benchmarks
 * Add initial tracing to the decode path
 * Make batch readahead configurable
@westonpace westonpace merged commit 568db43 into lancedb:main Apr 20, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants