Michael Ho has posted comments on this change.

Change subject: IMPALA-3629: Codegen TransferScratchTuples() in 
hdfs-parquet-scanner
......................................................................


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/3774/3/be/src/exec/hdfs-parquet-scanner-ir.cc
File be/src/exec/hdfs-parquet-scanner-ir.cc:

PS3, Line 44:   if (tuple_size == 0) {
> Given that if (tuple_size == 0) { ...} is handling an entire scratch patch 
On the other hand, tuple_size is known at compilation time so either this loop 
or the code after this loop will be eliminated so the codegen time may not be 
as bad as I initially thought.

If we ever go through this loop multiple times, the branch may be quite 
predictable. Did you see measurable performance difference when say scanning 
large files by removing this if-check ?


http://gerrit.cloudera.org:8080/#/c/3774/3/be/src/exec/hdfs-parquet-scanner.cc
File be/src/exec/hdfs-parquet-scanner.cc:

PS3, Line 520: num_row_to_commit = codegend_transfer_scratch_tuples_fn_(this, 
tuple_size, has_filters);
long line.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic327e437c7cd2b3f92cdb11c1e907bfee2d44ee8
Gerrit-PatchSet: 3
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Thomas Tauber-Marshall <[email protected]>
Gerrit-Reviewer: Michael Ho <[email protected]>
Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-HasComments: Yes

Reply via email to