Andrew Sherman 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 8: (4 comments) http://gerrit.cloudera.org:8080/#/c/11582/8/be/src/util/rle-encoding.h File be/src/util/rle-encoding.h: http://gerrit.cloudera.org:8080/#/c/11582/8/be/src/util/rle-encoding.h@259 PS8, Line 259: MaxBufferSize > I am not 100% sure here, but I think that this could be expressed in an eas OK, interesting. I think this is what you mean (see below). I'm saying this is not clearer to humans (but it was worth trying) static int MaxBufferSize(int bit_width, int num_values) { int data_bytes; int overhead; if (bit_width > 2) { // The largest encoded size is for all long literals with no repeated runs. int bytes_per_run = (bit_width * MAX_VALUES_PER_LITERAL_RUN) / 8; int num_runs = static_cast<int>(BitUtil::Ceil(num_values, MAX_VALUES_PER_LITERAL_RUN)); data_bytes = bytes_per_run * num_runs; overhead = num_runs; } else { data_bytes = static_cast<int>(BitUtil::Ceil(num_values * bit_width, 8)); if (bit_width == 1) { // Worst case is we use 4 bytes for every 3 of the input e.g. L8 R16 L8 R16 L8. overhead = 1 + static_cast<int>(BitUtil::Ceil(data_bytes, 3)); } else { // bit_width == 2 // Worst case is we use 3 bytes for every 2 of the input e.g. L8 R8 L8 R8 L8. overhead = 1 + static_cast<int>(BitUtil::Ceil(data_bytes, 2)); } } return std::max(MinBufferSize(bit_width), data_bytes + overhead); } http://gerrit.cloudera.org:8080/#/c/11582/8/be/src/util/rle-encoding.h@261 PS8, Line 261: encoded > Maybe repeated would be better here? Done http://gerrit.cloudera.org:8080/#/c/11582/8/be/src/util/rle-test.cc File be/src/util/rle-test.cc: http://gerrit.cloudera.org:8080/#/c/11582/8/be/src/util/rle-test.cc@636 PS8, Line 636: buffer_len > I think that this var could have a more descriptive name like expected_max_ Done http://gerrit.cloudera.org:8080/#/c/11582/8/be/src/util/rle-test.cc@642 PS8, Line 642: bug > typo: big :) :-( -- 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: 8 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: Mon, 05 Nov 2018 18:45:46 +0000 Gerrit-HasComments: Yes
