Repository navigation
Conversation
ShowPagesCommand looked up the compressed size of every page by opening a new stream on the file and reading the page headers of the column chunk from its start up to that page. A column chunk with N pages therefore cost N stream opens and N(N+1)/2 header reads, so the command slowed down quadratically with the number of pages per column chunk. Keep one stream open for the command and read the headers of the current column chunk once, in order, as its pages are formatted. The printed sizes are unchanged.
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
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.
Rationale for this change
parquet pagestakes time quadratic in the number of pages in a column chunk. Since #3326 the size printed for a page comes from its page header, andgetPageCompressedSize()finds that header by opening a new stream on the file and reading the headers of the column chunk from its start up to the current page. A chunk with N pages costs N stream opens and N(N+1)/2 header reads. On a file with 8,000 pages per column chunk the command took 41.6 s, whereparquet pages --rawtook 0.8 s.What changes are included in this PR?
The command opens one stream next to the reader and keeps it for the whole run. The formatter reads the headers of the current column chunk once, in order, continuing from the last header it read as the pages are formatted, and starts again at the next column chunk. The sizes come from the same headers as before, so the output does not change.
Are these changes tested?
testReadsEachPageHeaderOncewrites a file with more than one row group, two columns and 10 rows per page, runs the command through a local file system registered under its own scheme, and compares every printed size with the page headers. It then checks that the command read less than three times the file length from that file system. On master it fails withExpecting actual: 611298L to be less than: 58965L. With the change the command read 17,378 bytes of the 19,655-byte file. The size comparison alone passes on master too, since the output is the same.mvn -pl parquet-cli verifypasses: 90 tests (2 skipped, as on master), spotless, rat and japicmp. I skipped javadoc there.I also timed
parquet pages <file>with parquet-cli built from master and from this branch, on files with one row group, two columns (int64,int32) and 100 rows per page, JDK 17 in a Linux container:--raw(master)On all four files the output of the two builds is byte for byte the same (same MD5).
I did not run it on encrypted files or on a remote file system. The headers read are the same as before, so I expect no difference there, but that is not tested.
Are there any user-facing changes?
No change in output.
parquet pagesis faster on column chunks with many pages.Closes #3952