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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should apply the same optimization to V2 path below 🤔
Also, @mapleFU recently updated the compression check for V2 pages to use the uncompressed values if the compression didn't actually reduce the space. Maybe we should apply that to V1 pages too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The cost of copy is pretty insignifiant because memcpy speed it's around 10000MB/s and compression speed it's around 600MB/s. Underlayer vector use shink method https://doc.rust-lang.org/alloc/alloc/trait.Allocator.html#method.shrink. In standard malloc threadhold for switch to mmap allocation it's 128k and for shrink the system only unmap page and no need memory copy.
In V2 page buffer is not reserved
For no compress page when compression it's bad i can be a good idea to apply for V1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's not possible to disable compression on the fly with page v1
https://github.com/apache/parquet-format/blob/9fd57b59e0ce1a82a69237dcf8977d3e72a2965d/src/main/thrift/parquet.thrift#L671-L692