Tim Armstrong has posted comments on this change.

Change subject: Optimized ReadValueBatch() for Parquet scalar column readers.
......................................................................


Patch Set 5:

(1 comment)

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

Line 1566:     valid = Get(&level);
> outside scope of this change, but the LevelDecoder shouldn't be a subclass 
It was my change that made it that way. The difficulty is that the "proper" way 
to abstract this resulted in two BitReaders in each ColumnReader, which bloated 
the ColumnReader enough to require an extra cache line per column. It may be 
less of an issue with column-wise materialisation but I did see a pretty big 
perf gain from this combined with other cache/TLB optimisations.

There wasn't really an obvious way to work around this without a lot of 
refactoring, and the impact is pretty localised so I didn't worry too much 
about the questionable abstraction.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I21fa9b050a45f2dd45cc0091ea5b008d3c0a3f30
Gerrit-PatchSet: 5
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Alex Behm <[email protected]>
Gerrit-Reviewer: Alex Behm <[email protected]>
Gerrit-Reviewer: Marcel Kornacker <[email protected]>
Gerrit-Reviewer: Mostafa Mokhtar <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-HasComments: Yes

Reply via email to