Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

Added reserve to pushable containers in parquet extend_from_decoder #1301

Merged
merged 2 commits into from Nov 23, 2022

Conversation

ritchie46
Copy link
Collaborator

This ensure we reserve the amount of slots we are going to push into the container up front.

@codecov
Copy link

codecov bot commented Nov 17, 2022

Codecov Report

Base: 83.12% // Head: 83.14% // Increases project coverage by +0.02% 🎉

Coverage data is based on head (a40f689) compared to base (0ba4f8e).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1301      +/-   ##
==========================================
+ Coverage   83.12%   83.14%   +0.02%     
==========================================
  Files         369      369              
  Lines       40180    40211      +31     
==========================================
+ Hits        33399    33434      +35     
+ Misses       6781     6777       -4     
Impacted Files Coverage Δ
src/io/parquet/read/deserialize/binary/utils.rs 66.99% <100.00%> (+2.77%) ⬆️
...arquet/read/deserialize/fixed_size_binary/utils.rs 71.87% <100.00%> (+2.90%) ⬆️
src/io/parquet/read/deserialize/utils.rs 82.23% <100.00%> (+0.89%) ⬆️
src/io/ipc/read/stream_async.rs 75.34% <0.00%> (-1.37%) ⬇️
src/io/ipc/read/file_async.rs 60.44% <0.00%> (-0.75%) ⬇️
src/io/ipc/read/file.rs 97.32% <0.00%> (+0.44%) ⬆️
src/array/utf8/mod.rs 85.67% <0.00%> (+0.91%) ⬆️
src/bitmap/utils/slice_iterator.rs 98.78% <0.00%> (+1.21%) ⬆️
src/io/ipc/read/array/boolean.rs 98.14% <0.00%> (+7.40%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Owner

@jorgecarleitao jorgecarleitao left a comment

Choose a reason for hiding this comment

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

Thanks @ritchie46 !

@ritchie46
Copy link
Collaborator Author

ritchie46 commented Nov 19, 2022

I have been thinking a bit about this one. How often do we call the function that does the reserve? If it only writes to that buffer in one function call, I think it makes sense to reserve up front. If we call this function more often (with the same Pushable) we will trigger a realloc every call because we don't have an exponential growing strategy.

EDIT:

I've adapted the code a bit, so that we first determine how much space we need to reserve, then we do a second pass to actually fill the buffers.

@jorgecarleitao jorgecarleitao changed the title reserve pushable containers in parquet extend_from_decoder Added reserve to pushable containers in parquet extend_from_decoder Nov 23, 2022
@jorgecarleitao jorgecarleitao added the enhancement An improvement to an existing feature label Nov 23, 2022
@jorgecarleitao jorgecarleitao merged commit 619d8da into jorgecarleitao:main Nov 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement An improvement to an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants