-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[PARQUET-99] Page size check properties #250
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
Changes from all commits
e7cd54b
a057f46
b49f03c
68794f0
3f7870c
5d99072
71336ee
2090719
18f8d3a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -47,7 +47,6 @@ | |
| final class ColumnWriterV1 implements ColumnWriter { | ||
| private static final Log LOG = Log.getLog(ColumnWriterV1.class); | ||
| private static final boolean DEBUG = Log.DEBUG; | ||
| private static final int INITIAL_COUNT_FOR_SIZE_CHECK = 100; | ||
| private static final int MIN_SLAB_SIZE = 64; | ||
|
|
||
| private final ColumnDescriptor path; | ||
|
|
@@ -57,7 +56,9 @@ final class ColumnWriterV1 implements ColumnWriter { | |
| private ValuesWriter definitionLevelColumn; | ||
| private ValuesWriter dataColumn; | ||
| private int valueCount; | ||
| private int initialRowCountForPageSizeCheck; | ||
| private int valueCountForNextSizeCheck; | ||
| private boolean estimateNextSizeCheck; | ||
|
|
||
| private Statistics statistics; | ||
|
|
||
|
|
@@ -67,13 +68,19 @@ public ColumnWriterV1( | |
| int pageSizeThreshold, | ||
| int dictionaryPageSizeThreshold, | ||
| boolean enableDictionary, | ||
| int initialRowCountForPageSizeCheck, | ||
| boolean estimateNextSizeCheck, | ||
| WriterVersion writerVersion, | ||
| ByteBufferAllocator allocator) { | ||
| this.path = path; | ||
| this.pageWriter = pageWriter; | ||
| this.pageSizeThreshold = pageSizeThreshold; | ||
| // initial check of memory usage. So that we have enough data to make an initial prediction | ||
| this.valueCountForNextSizeCheck = INITIAL_COUNT_FOR_SIZE_CHECK; | ||
| this.initialRowCountForPageSizeCheck = initialRowCountForPageSizeCheck; | ||
| this.valueCountForNextSizeCheck = initialRowCountForPageSizeCheck; | ||
| // Do not attempt to predict next size check. Prevents issues with rows that vary significantly in size. | ||
| this.estimateNextSizeCheck = estimateNextSizeCheck; | ||
|
|
||
| resetStatistics(); | ||
|
|
||
| ParquetProperties parquetProps = new ParquetProperties(dictionaryPageSizeThreshold, writerVersion, enableDictionary, allocator); | ||
|
|
@@ -111,11 +118,17 @@ private void accountForValueWritten() { | |
| + dataColumn.getBufferedSize(); | ||
| if (memSize > pageSizeThreshold) { | ||
| // we will write the current page and check again the size at the predicted middle of next page | ||
| valueCountForNextSizeCheck = valueCount / 2; | ||
| if(estimateNextSizeCheck) { | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe that if
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Forget what I said. Now I see what you're saying. |
||
| valueCountForNextSizeCheck = valueCount / 2; | ||
| } else { | ||
| valueCountForNextSizeCheck = initialRowCountForPageSizeCheck; | ||
| } | ||
| writePage(); | ||
| } else { | ||
| } else if (estimateNextSizeCheck) { | ||
| // not reached the threshold, will check again midway | ||
| valueCountForNextSizeCheck = (int)(valueCount + ((float)valueCount * pageSizeThreshold / memSize)) / 2 + 1; | ||
| } else { | ||
| valueCountForNextSizeCheck += initialRowCountForPageSizeCheck; | ||
| } | ||
| } | ||
| } | ||
|
|
||
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.
This looks good to me now.
Nit: the comments aren't needed and reference values no longer used. Maybe just change the "will check again halfway" by adding " if it is between min and max"