Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11582 )

Change subject: IMPALA-6658: improve Parquet RLE for low bit widths
......................................................................


Patch Set 3: Code-Review+1

(3 comments)

If you do not want the implement the "use shorter min_repeated_run_length if 
the last run was a repeated run" logic then lgtm.

http://gerrit.cloudera.org:8080/#/c/11582/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/11582/2//COMMIT_MSG@20
PS2, Line 20: length 16 for single bit values. All other bit widths will use the
            : existing length 8 runs.
            :
> I agree with your calculations. I think sometimes min_run_length=24 is bett
>This could be avoided by using smaller
>min_repeated_run_length if the last run was a repeated run:

Did you think about the logic above? I think that it would be very close to 
optimal encoding, and would not need extreme code changes (e.g. there could be 
a min_repeated_run_length_after_literal_run_, and a 
min_repeated_run_length_after_repeated_run_, and FlushLiteralRun / 
FlushRepeatedRun could switch between them).

About the "typical use case": I agree that interrupting repeated runs should be 
more common (but we shouldn't cause a regression in other cases).

I think that the most typical use case where we can win a few bytes is when one 
value is much less frequent than another (e.g.  most values in a column are 
NULL, while some are not), which leads to alternating short literal runs and 
longer repeated runs. If the ratio is around 1/8 - 1/24,  most of the repeated 
runs will be short enough to win with encoding as literal.


http://gerrit.cloudera.org:8080/#/c/11582/3/be/src/util/rle-encoding.h
File be/src/util/rle-encoding.h:

http://gerrit.cloudera.org:8080/#/c/11582/3/be/src/util/rle-encoding.h@266
PS3, Line 266: // Using min_repeated_run_length=16 is always better than 8.
I prefer to phrase it as "never worse than 8". For alternating long repeated 
runs and runs of 8 it is the same.


http://gerrit.cloudera.org:8080/#/c/11582/3/be/src/util/rle-encoding.h@267
PS3, Line 267: better than 8
Maybe "better than 16" would be better.



--
To view, visit http://gerrit.cloudera.org:8080/11582
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I191a581d3f699b6669e48ac9dc39c76ed77c4a76
Gerrit-Change-Number: 11582
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Sherman <[email protected]>
Gerrit-Reviewer: Andrew Sherman <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Thomas Marshall <[email protected]>
Gerrit-Comment-Date: Wed, 24 Oct 2018 11:18:05 +0000
Gerrit-HasComments: Yes

Reply via email to