You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Daniel Becker (Code Review)" <ge...@cloudera.org> on 2020/06/10 17:06:30 UTC
[Impala-ASF-CR] IMPALA-9747: More fine-grained codegen for text file scanners
Daniel Becker has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16059
Change subject: IMPALA-9747: More fine-grained codegen for text file scanners
......................................................................
IMPALA-9747: More fine-grained codegen for text file scanners
Currently if the materialization of any column cannot be codegen'd for
some reason (e.g. it is CHAR(N)), the whole codegen is cancelled
for the text scanner.
This commit improves the case of CHAR(N). For CHAR(N) columns,
HdfsScanner codegens code that calls the interpreted version instead of
failing codegen. For other columns codegen is used as usually.
Change-Id: Id370193af578ecf23ed3c6bfcc65fec448156fa3
---
M be/src/codegen/gen_ir_descriptions.py
M be/src/exec/hdfs-scanner-ir.cc
M be/src/exec/hdfs-scanner.cc
M be/src/exec/hdfs-scanner.h
4 files changed, 54 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/59/16059/3
--
To view, visit http://gerrit.cloudera.org:8080/16059
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id370193af578ecf23ed3c6bfcc65fec448156fa3
Gerrit-Change-Number: 16059
Gerrit-PatchSet: 3
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
[Impala-ASF-CR] IMPALA-9747: More fine-grained codegen for text file scanners
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16059 )
Change subject: IMPALA-9747: More fine-grained codegen for text file scanners
......................................................................
Patch Set 13:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/6042/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/16059
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id370193af578ecf23ed3c6bfcc65fec448156fa3
Gerrit-Change-Number: 16059
Gerrit-PatchSet: 13
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 24 Jun 2020 11:52:13 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9747: More fine-grained codegen for text file scanners
Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has uploaded a new patch set (#11). ( http://gerrit.cloudera.org:8080/16059 )
Change subject: IMPALA-9747: More fine-grained codegen for text file scanners
......................................................................
IMPALA-9747: More fine-grained codegen for text file scanners
Currently if the materialization of any column cannot be codegen'd
because its type is unsupported (e.g. CHAR(N)), the whole codegen is
cancelled for the text scanner.
This commit adds the function TextConverter::SupportsCodegenWriteSlot
that returns whether the given ColumnType is supported. If the type is
not supported, HdfsScanner codegens code that calls the interpreted
version instead of failing codegen. For other columns codegen is used as
usually.
Benchmarks:
Copied and modified a TPCH table with scale factor 5 to add a CHAR
column to it::
USE tpch5;
CREATE TABLE IF NOT EXISTS lineitem_char AS
SELECT *, CAST(l_shipdate AS CHAR(10)) l_shipdate_char
FROM lineitem;
Run the following query 100 times after one warm-up run with and
without this change:
SELECT *
FROM tpch5.lineitem_char
WHERE
l_partkey BETWEEN 500 AND 500000 AND
l_linestatus = 'F' AND
l_quantity < 35 AND
l_extendedprice BETWEEN 2000 AND 8000 AND
l_discount > 0 AND
l_tax BETWEEN 0.04 AND 0.06 AND
l_returnflag IN ('A', 'N') AND
l_shipdate_char < '1996-06-20'
ORDER BY l_shipdate_char
LIMIT 10;
Without this commit: mean: 2.92, standard deviation: 0.13.
With this commit: mean: 2.21, standard deviation: 0.072.
Testing:
The interesting cases regarding char are covered in
https://github.com/apache/impala/blob/0167c5b4242fcebf6be19aba5ecfb440204278ad/testdata/workloads/functional-query/queries/QueryTest/chars.test
Change-Id: Id370193af578ecf23ed3c6bfcc65fec448156fa3
---
M be/src/codegen/gen_ir_descriptions.py
M be/src/exec/hdfs-scanner-ir.cc
M be/src/exec/hdfs-scanner.cc
M be/src/exec/hdfs-scanner.h
M be/src/exec/text-converter.cc
M be/src/exec/text-converter.h
M testdata/workloads/functional-query/queries/QueryTest/datastream-sender-codegen.test
M tests/query_test/test_codegen.py
8 files changed, 68 insertions(+), 51 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/59/16059/11
--
To view, visit http://gerrit.cloudera.org:8080/16059
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id370193af578ecf23ed3c6bfcc65fec448156fa3
Gerrit-Change-Number: 16059
Gerrit-PatchSet: 11
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-9747: More fine-grained codegen for text file scanners
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16059 )
Change subject: IMPALA-9747: More fine-grained codegen for text file scanners
......................................................................
Patch Set 12:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/6408/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/16059
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id370193af578ecf23ed3c6bfcc65fec448156fa3
Gerrit-Change-Number: 16059
Gerrit-PatchSet: 12
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 24 Jun 2020 12:07:19 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9747: More fine-grained codegen for text file scanners
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/16059 )
Change subject: IMPALA-9747: More fine-grained codegen for text file scanners
......................................................................
Patch Set 11:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16059/10/tests/query_test/test_codegen.py
File tests/query_test/test_codegen.py:
http://gerrit.cloudera.org:8080/#/c/16059/10/tests/query_test/test_codegen.py@71
PS10, Line 71:
I would prefer to keep the parts of the test that are still relevant, so the asserts CodegenWriteSlot can go, but I think the other asserts should still be valid.
--
To view, visit http://gerrit.cloudera.org:8080/16059
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id370193af578ecf23ed3c6bfcc65fec448156fa3
Gerrit-Change-Number: 16059
Gerrit-PatchSet: 11
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 24 Jun 2020 11:29:27 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9747: More fine-grained codegen for text file scanners
Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/16059 )
Change subject: IMPALA-9747: More fine-grained codegen for text file scanners
......................................................................
IMPALA-9747: More fine-grained codegen for text file scanners
Currently if the materialization of any column cannot be codegen'd
because its type is unsupported (e.g. CHAR(N)), the whole codegen is
cancelled for the text scanner.
This commit adds the function TextConverter::SupportsCodegenWriteSlot
that returns whether the given ColumnType is supported. If the type is
not supported, HdfsScanner codegens code that calls the interpreted
version instead of failing codegen. For other columns codegen is used as
usually.
Benchmarks:
TODO
Testing:
TODO
Change-Id: Id370193af578ecf23ed3c6bfcc65fec448156fa3
---
M be/src/codegen/gen_ir_descriptions.py
M be/src/exec/hdfs-scanner-ir.cc
M be/src/exec/hdfs-scanner.cc
M be/src/exec/hdfs-scanner.h
M be/src/exec/text-converter.cc
M be/src/exec/text-converter.h
6 files changed, 71 insertions(+), 10 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/59/16059/4
--
To view, visit http://gerrit.cloudera.org:8080/16059
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id370193af578ecf23ed3c6bfcc65fec448156fa3
Gerrit-Change-Number: 16059
Gerrit-PatchSet: 4
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-9747: More fine-grained codegen for text file scanners
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16059 )
Change subject: IMPALA-9747: More fine-grained codegen for text file scanners
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/6328/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/16059
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id370193af578ecf23ed3c6bfcc65fec448156fa3
Gerrit-Change-Number: 16059
Gerrit-PatchSet: 5
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 15 Jun 2020 16:29:09 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9747: More fine-grained codegen for text file scanners
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16059 )
Change subject: IMPALA-9747: More fine-grained codegen for text file scanners
......................................................................
Patch Set 9: Code-Review+2
This is a nice improvement, thank you!
--
To view, visit http://gerrit.cloudera.org:8080/16059
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id370193af578ecf23ed3c6bfcc65fec448156fa3
Gerrit-Change-Number: 16059
Gerrit-PatchSet: 9
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 23 Jun 2020 22:32:02 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9747: More fine-grained codegen for text file scanners
Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has uploaded a new patch set (#12). ( http://gerrit.cloudera.org:8080/16059 )
Change subject: IMPALA-9747: More fine-grained codegen for text file scanners
......................................................................
IMPALA-9747: More fine-grained codegen for text file scanners
Currently if the materialization of any column cannot be codegen'd
because its type is unsupported (e.g. CHAR(N)), the whole codegen is
cancelled for the text scanner.
This commit adds the function TextConverter::SupportsCodegenWriteSlot
that returns whether the given ColumnType is supported. If the type is
not supported, HdfsScanner codegens code that calls the interpreted
version instead of failing codegen. For other columns codegen is used as
usually.
Benchmarks:
Copied and modified a TPCH table with scale factor 5 to add a CHAR
column to it::
USE tpch5;
CREATE TABLE IF NOT EXISTS lineitem_char AS
SELECT *, CAST(l_shipdate AS CHAR(10)) l_shipdate_char
FROM lineitem;
Run the following query 100 times after one warm-up run with and
without this change:
SELECT *
FROM tpch5.lineitem_char
WHERE
l_partkey BETWEEN 500 AND 500000 AND
l_linestatus = 'F' AND
l_quantity < 35 AND
l_extendedprice BETWEEN 2000 AND 8000 AND
l_discount > 0 AND
l_tax BETWEEN 0.04 AND 0.06 AND
l_returnflag IN ('A', 'N') AND
l_shipdate_char < '1996-06-20'
ORDER BY l_shipdate_char
LIMIT 10;
Without this commit: mean: 2.92, standard deviation: 0.13.
With this commit: mean: 2.21, standard deviation: 0.072.
Testing:
The interesting cases regarding char are covered in
https://github.com/apache/impala/blob/0167c5b4242fcebf6be19aba5ecfb440204278ad/testdata/workloads/functional-query/queries/QueryTest/chars.test
Change-Id: Id370193af578ecf23ed3c6bfcc65fec448156fa3
---
M be/src/codegen/gen_ir_descriptions.py
M be/src/exec/hdfs-scanner-ir.cc
M be/src/exec/hdfs-scanner.cc
M be/src/exec/hdfs-scanner.h
M be/src/exec/text-converter.cc
M be/src/exec/text-converter.h
M testdata/workloads/functional-query/queries/QueryTest/datastream-sender-codegen.test
M tests/query_test/test_codegen.py
8 files changed, 73 insertions(+), 19 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/59/16059/12
--
To view, visit http://gerrit.cloudera.org:8080/16059
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id370193af578ecf23ed3c6bfcc65fec448156fa3
Gerrit-Change-Number: 16059
Gerrit-PatchSet: 12
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-9747: More fine-grained codegen for text file scanners
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16059 )
Change subject: IMPALA-9747: More fine-grained codegen for text file scanners
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16059/4/be/src/exec/text-converter.h
File be/src/exec/text-converter.h:
http://gerrit.cloudera.org:8080/#/c/16059/4/be/src/exec/text-converter.h@91
PS4, Line 91: static bool SupportsCodegenWriteSlot(const ColumnType& col_type);
line has trailing whitespace
--
To view, visit http://gerrit.cloudera.org:8080/16059
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id370193af578ecf23ed3c6bfcc65fec448156fa3
Gerrit-Change-Number: 16059
Gerrit-PatchSet: 4
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 11 Jun 2020 12:29:42 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9747: More fine-grained codegen for text file scanners
Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/16059 )
Change subject: IMPALA-9747: More fine-grained codegen for text file scanners
......................................................................
Patch Set 10:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16059/10/tests/query_test/test_codegen.py
File tests/query_test/test_codegen.py:
http://gerrit.cloudera.org:8080/#/c/16059/10/tests/query_test/test_codegen.py@71
PS10, Line 71: assert ("TextConverter::CodegenWriteSlot(): Char isn't supported for CodegenWriteSlot"
> I would prefer to keep the parts of the test that are still relevant, so th
Done
--
To view, visit http://gerrit.cloudera.org:8080/16059
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id370193af578ecf23ed3c6bfcc65fec448156fa3
Gerrit-Change-Number: 16059
Gerrit-PatchSet: 10
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 24 Jun 2020 11:48:14 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9747: More fine-grained codegen for text file scanners
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/16059 )
Change subject: IMPALA-9747: More fine-grained codegen for text file scanners
......................................................................
Patch Set 3:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/16059/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/16059/3//COMMIT_MSG@13
PS3, Line 13: improves
Can you add some benchmark results? It would be good to see a case where this made things faster.
http://gerrit.cloudera.org:8080/#/c/16059/3//COMMIT_MSG@16
PS3, Line 16:
About testing: I guess the mixed case of codegenable and non-codegenable columns is already tested somewhere, but it would be good to point to a table like that.
http://gerrit.cloudera.org:8080/#/c/16059/3/be/src/exec/hdfs-scanner-ir.cc
File be/src/exec/hdfs-scanner-ir.cc:
http://gerrit.cloudera.org:8080/#/c/16059/3/be/src/exec/hdfs-scanner-ir.cc@115
PS3, Line 115: copy_string
Can't we just pass false in WriteSlot?
http://gerrit.cloudera.org:8080/#/c/16059/3/be/src/exec/hdfs-scanner.h
File be/src/exec/hdfs-scanner.h:
http://gerrit.cloudera.org:8080/#/c/16059/3/be/src/exec/hdfs-scanner.h@594
PS3, Line 594: we
typo: with?
http://gerrit.cloudera.org:8080/#/c/16059/3/be/src/exec/hdfs-scanner.h@598
PS3, Line 598: int slot_idx, Tuple* tuple, const char* data, int len, MemPool* pool);
nit: +2 indentation
http://gerrit.cloudera.org:8080/#/c/16059/3/be/src/exec/hdfs-scanner.cc
File be/src/exec/hdfs-scanner.cc:
http://gerrit.cloudera.org:8080/#/c/16059/3/be/src/exec/hdfs-scanner.cc@349
PS3, Line 349: slot_desc->type().type != TYPE_CHAR
I would prefer to add a function like SupportsCodegenWriteSlot to TextConverter, as it is generally the responsibility of that class to decide whether it supports codegen for a given type.
http://gerrit.cloudera.org:8080/#/c/16059/3/be/src/exec/hdfs-scanner.cc@495
PS3, Line 495: this_arg, slot_idx_value, opaque_tuple_arg, data, len, mem_pool_arg};
nit: +2 indentation
--
To view, visit http://gerrit.cloudera.org:8080/16059
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id370193af578ecf23ed3c6bfcc65fec448156fa3
Gerrit-Change-Number: 16059
Gerrit-PatchSet: 3
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 10 Jun 2020 22:09:42 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9747: More fine-grained codegen for text file scanners
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16059 )
Change subject: IMPALA-9747: More fine-grained codegen for text file scanners
......................................................................
Patch Set 9:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/6364/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/16059
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id370193af578ecf23ed3c6bfcc65fec448156fa3
Gerrit-Change-Number: 16059
Gerrit-PatchSet: 9
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 Jun 2020 12:36:20 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9747: More fine-grained codegen for text file scanners
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/16059 )
Change subject: IMPALA-9747: More fine-grained codegen for text file scanners
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16059/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/16059/3//COMMIT_MSG@16
PS3, Line 16: version instead of failing codegen. For other columns codegen is used as
> Do you think it is tested in a unit test or an integration test?
I think that it is tested in https://github.com/apache/impala/blob/0167c5b4242fcebf6be19aba5ecfb440204278ad/testdata/workloads/functional-query/queries/QueryTest/chars.test, but it may make sense to add a few tests (e.g using functional.chars_medium) that read both both char(N) and other types of columns. A test that reads only char(N) (so that non of the columns' writeSlot can be codegend) could be also useful.
--
To view, visit http://gerrit.cloudera.org:8080/16059
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id370193af578ecf23ed3c6bfcc65fec448156fa3
Gerrit-Change-Number: 16059
Gerrit-PatchSet: 4
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 11 Jun 2020 13:16:12 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9747: More fine-grained codegen for text file scanners
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16059 )
Change subject: IMPALA-9747: More fine-grained codegen for text file scanners
......................................................................
Patch Set 10: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/6039/
--
To view, visit http://gerrit.cloudera.org:8080/16059
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id370193af578ecf23ed3c6bfcc65fec448156fa3
Gerrit-Change-Number: 16059
Gerrit-PatchSet: 10
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 24 Jun 2020 03:29:11 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9747: More fine-grained codegen for text file scanners
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16059 )
Change subject: IMPALA-9747: More fine-grained codegen for text file scanners
......................................................................
IMPALA-9747: More fine-grained codegen for text file scanners
Currently if the materialization of any column cannot be codegen'd
because its type is unsupported (e.g. CHAR(N)), the whole codegen is
cancelled for the text scanner.
This commit adds the function TextConverter::SupportsCodegenWriteSlot
that returns whether the given ColumnType is supported. If the type is
not supported, HdfsScanner codegens code that calls the interpreted
version instead of failing codegen. For other columns codegen is used as
usually.
Benchmarks:
Copied and modified a TPCH table with scale factor 5 to add a CHAR
column to it::
USE tpch5;
CREATE TABLE IF NOT EXISTS lineitem_char AS
SELECT *, CAST(l_shipdate AS CHAR(10)) l_shipdate_char
FROM lineitem;
Run the following query 100 times after one warm-up run with and
without this change:
SELECT *
FROM tpch5.lineitem_char
WHERE
l_partkey BETWEEN 500 AND 500000 AND
l_linestatus = 'F' AND
l_quantity < 35 AND
l_extendedprice BETWEEN 2000 AND 8000 AND
l_discount > 0 AND
l_tax BETWEEN 0.04 AND 0.06 AND
l_returnflag IN ('A', 'N') AND
l_shipdate_char < '1996-06-20'
ORDER BY l_shipdate_char
LIMIT 10;
Without this commit: mean: 2.92, standard deviation: 0.13.
With this commit: mean: 2.21, standard deviation: 0.072.
Testing:
The interesting cases regarding char are covered in
https://github.com/apache/impala/blob/0167c5b4242fcebf6be19aba5ecfb440204278ad/testdata/workloads/functional-query/queries/QueryTest/chars.test
Change-Id: Id370193af578ecf23ed3c6bfcc65fec448156fa3
Reviewed-on: http://gerrit.cloudera.org:8080/16059
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/codegen/gen_ir_descriptions.py
M be/src/exec/hdfs-scanner-ir.cc
M be/src/exec/hdfs-scanner.cc
M be/src/exec/hdfs-scanner.h
M be/src/exec/text-converter.cc
M be/src/exec/text-converter.h
M testdata/workloads/functional-query/queries/QueryTest/datastream-sender-codegen.test
M tests/query_test/test_codegen.py
8 files changed, 73 insertions(+), 19 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/16059
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Id370193af578ecf23ed3c6bfcc65fec448156fa3
Gerrit-Change-Number: 16059
Gerrit-PatchSet: 14
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-9747: More fine-grained codegen for text file scanners
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/16059 )
Change subject: IMPALA-9747: More fine-grained codegen for text file scanners
......................................................................
Patch Set 9: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/16059
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id370193af578ecf23ed3c6bfcc65fec448156fa3
Gerrit-Change-Number: 16059
Gerrit-PatchSet: 9
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 22 Jun 2020 12:56:33 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9747: More fine-grained codegen for text file scanners
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16059 )
Change subject: IMPALA-9747: More fine-grained codegen for text file scanners
......................................................................
Patch Set 6:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/6335/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/16059
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id370193af578ecf23ed3c6bfcc65fec448156fa3
Gerrit-Change-Number: 16059
Gerrit-PatchSet: 6
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 16 Jun 2020 14:20:55 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9747: More fine-grained codegen for text file scanners
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16059 )
Change subject: IMPALA-9747: More fine-grained codegen for text file scanners
......................................................................
Patch Set 10:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/6039/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/16059
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id370193af578ecf23ed3c6bfcc65fec448156fa3
Gerrit-Change-Number: 16059
Gerrit-PatchSet: 10
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 23 Jun 2020 22:32:16 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9747: More fine-grained codegen for text file scanners
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16059 )
Change subject: IMPALA-9747: More fine-grained codegen for text file scanners
......................................................................
Patch Set 13: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/16059
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id370193af578ecf23ed3c6bfcc65fec448156fa3
Gerrit-Change-Number: 16059
Gerrit-PatchSet: 13
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 24 Jun 2020 16:51:47 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9747: More fine-grained codegen for text file scanners
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/16059 )
Change subject: IMPALA-9747: More fine-grained codegen for text file scanners
......................................................................
Patch Set 8: Code-Review+1
(2 comments)
http://gerrit.cloudera.org:8080/#/c/16059/8/be/src/exec/hdfs-scanner.cc
File be/src/exec/hdfs-scanner.cc:
http://gerrit.cloudera.org:8080/#/c/16059/8/be/src/exec/hdfs-scanner.cc@490
PS8, Line 490: // TODO: Should we log this?
: const SlotDescriptor* slot_desc = node->materialized_slots_[slot_idx];
: VLOG(google::INFO)
: << "Going to call interpreted code from codegen for column type "
: << slot_desc->type() << ".";
I don't think that we need to log this.
http://gerrit.cloudera.org:8080/#/c/16059/8/be/src/exec/text-converter.cc
File be/src/exec/text-converter.cc:
http://gerrit.cloudera.org:8080/#/c/16059/8/be/src/exec/text-converter.cc@114
PS8, Line 114: if (!SupportsCodegenWriteSlot(col_type)) {
: stringstream msg;
: msg << "TextConverter::CodegenWriteSlot(): "
: << col_type
: << " isn't supported for CodegenWriteSlot.";
: return Status::Expected(msg.str());
: }
Can we actually enter this block? If not, then a DCHECK would be better.
--
To view, visit http://gerrit.cloudera.org:8080/16059
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id370193af578ecf23ed3c6bfcc65fec448156fa3
Gerrit-Change-Number: 16059
Gerrit-PatchSet: 8
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 17 Jun 2020 15:45:04 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9747: More fine-grained codegen for text file scanners
Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/16059 )
Change subject: IMPALA-9747: More fine-grained codegen for text file scanners
......................................................................
IMPALA-9747: More fine-grained codegen for text file scanners
Currently if the materialization of any column cannot be codegen'd
because its type is unsupported (e.g. CHAR(N)), the whole codegen is
cancelled for the text scanner.
This commit adds the function TextConverter::SupportsCodegenWriteSlot
that returns whether the given ColumnType is supported. If the type is
not supported, HdfsScanner codegens code that calls the interpreted
version instead of failing codegen. For other columns codegen is used as
usually.
Benchmarks:
TODO
Testing:
TODO
Change-Id: Id370193af578ecf23ed3c6bfcc65fec448156fa3
---
M be/src/codegen/gen_ir_descriptions.py
M be/src/exec/hdfs-scanner-ir.cc
M be/src/exec/hdfs-scanner.cc
M be/src/exec/hdfs-scanner.h
M be/src/exec/text-converter.cc
M be/src/exec/text-converter.h
6 files changed, 76 insertions(+), 10 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/59/16059/5
--
To view, visit http://gerrit.cloudera.org:8080/16059
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id370193af578ecf23ed3c6bfcc65fec448156fa3
Gerrit-Change-Number: 16059
Gerrit-PatchSet: 5
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-9747: More fine-grained codegen for text file scanners
Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has uploaded a new patch set (#8). ( http://gerrit.cloudera.org:8080/16059 )
Change subject: IMPALA-9747: More fine-grained codegen for text file scanners
......................................................................
IMPALA-9747: More fine-grained codegen for text file scanners
Currently if the materialization of any column cannot be codegen'd
because its type is unsupported (e.g. CHAR(N)), the whole codegen is
cancelled for the text scanner.
This commit adds the function TextConverter::SupportsCodegenWriteSlot
that returns whether the given ColumnType is supported. If the type is
not supported, HdfsScanner codegens code that calls the interpreted
version instead of failing codegen. For other columns codegen is used as
usually.
Benchmarks:
Copied and modified a TPCH table with scale factor 5 to add a CHAR
column to it::
USE tpch5;
CREATE TABLE IF NOT EXISTS lineitem_char AS
SELECT *, CAST(l_shipdate AS CHAR(10)) l_shipdate_char
FROM lineitem;
Run the following query 100 times after one warm-up run with and
without this change:
SELECT *
FROM tpch5.lineitem_char
WHERE
l_partkey BETWEEN 500 AND 500000 AND
l_linestatus = 'F' AND
l_quantity < 35 AND
l_extendedprice BETWEEN 2000 AND 8000 AND
l_discount > 0 AND
l_tax BETWEEN 0.04 AND 0.06 AND
l_returnflag IN ('A', 'N') AND
l_shipdate_char < '1996-06-20'
ORDER BY l_shipdate_char
LIMIT 10;
Without this commit: mean: 2.92, standard deviation: 0.13.
With this commit: mean: 2.21, standard deviation: 0.072.
Testing:
The interesting cases regarding char are covered in
https://github.com/apache/impala/blob/0167c5b4242fcebf6be19aba5ecfb440204278ad/testdata/workloads/functional-query/queries/QueryTest/chars.test
Change-Id: Id370193af578ecf23ed3c6bfcc65fec448156fa3
---
M be/src/codegen/gen_ir_descriptions.py
M be/src/exec/hdfs-scanner-ir.cc
M be/src/exec/hdfs-scanner.cc
M be/src/exec/hdfs-scanner.h
M be/src/exec/text-converter.cc
M be/src/exec/text-converter.h
6 files changed, 76 insertions(+), 10 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/59/16059/8
--
To view, visit http://gerrit.cloudera.org:8080/16059
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id370193af578ecf23ed3c6bfcc65fec448156fa3
Gerrit-Change-Number: 16059
Gerrit-PatchSet: 8
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-9747: More fine-grained codegen for text file scanners
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/16059 )
Change subject: IMPALA-9747: More fine-grained codegen for text file scanners
......................................................................
Patch Set 12: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/16059
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id370193af578ecf23ed3c6bfcc65fec448156fa3
Gerrit-Change-Number: 16059
Gerrit-PatchSet: 12
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 24 Jun 2020 11:51:41 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9747: More fine-grained codegen for text file scanners
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16059 )
Change subject: IMPALA-9747: More fine-grained codegen for text file scanners
......................................................................
Patch Set 10: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/16059
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id370193af578ecf23ed3c6bfcc65fec448156fa3
Gerrit-Change-Number: 16059
Gerrit-PatchSet: 10
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 23 Jun 2020 22:32:14 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9747: More fine-grained codegen for text file scanners
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16059 )
Change subject: IMPALA-9747: More fine-grained codegen for text file scanners
......................................................................
Patch Set 11:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/6407/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/16059
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id370193af578ecf23ed3c6bfcc65fec448156fa3
Gerrit-Change-Number: 16059
Gerrit-PatchSet: 11
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 24 Jun 2020 11:51:56 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9747: More fine-grained codegen for text file scanners
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16059 )
Change subject: IMPALA-9747: More fine-grained codegen for text file scanners
......................................................................
Patch Set 8:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/6337/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/16059
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id370193af578ecf23ed3c6bfcc65fec448156fa3
Gerrit-Change-Number: 16059
Gerrit-PatchSet: 8
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 16 Jun 2020 14:53:49 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9747: More fine-grained codegen for text file scanners
Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has uploaded a new patch set (#9). ( http://gerrit.cloudera.org:8080/16059 )
Change subject: IMPALA-9747: More fine-grained codegen for text file scanners
......................................................................
IMPALA-9747: More fine-grained codegen for text file scanners
Currently if the materialization of any column cannot be codegen'd
because its type is unsupported (e.g. CHAR(N)), the whole codegen is
cancelled for the text scanner.
This commit adds the function TextConverter::SupportsCodegenWriteSlot
that returns whether the given ColumnType is supported. If the type is
not supported, HdfsScanner codegens code that calls the interpreted
version instead of failing codegen. For other columns codegen is used as
usually.
Benchmarks:
Copied and modified a TPCH table with scale factor 5 to add a CHAR
column to it::
USE tpch5;
CREATE TABLE IF NOT EXISTS lineitem_char AS
SELECT *, CAST(l_shipdate AS CHAR(10)) l_shipdate_char
FROM lineitem;
Run the following query 100 times after one warm-up run with and
without this change:
SELECT *
FROM tpch5.lineitem_char
WHERE
l_partkey BETWEEN 500 AND 500000 AND
l_linestatus = 'F' AND
l_quantity < 35 AND
l_extendedprice BETWEEN 2000 AND 8000 AND
l_discount > 0 AND
l_tax BETWEEN 0.04 AND 0.06 AND
l_returnflag IN ('A', 'N') AND
l_shipdate_char < '1996-06-20'
ORDER BY l_shipdate_char
LIMIT 10;
Without this commit: mean: 2.92, standard deviation: 0.13.
With this commit: mean: 2.21, standard deviation: 0.072.
Testing:
The interesting cases regarding char are covered in
https://github.com/apache/impala/blob/0167c5b4242fcebf6be19aba5ecfb440204278ad/testdata/workloads/functional-query/queries/QueryTest/chars.test
Change-Id: Id370193af578ecf23ed3c6bfcc65fec448156fa3
---
M be/src/codegen/gen_ir_descriptions.py
M be/src/exec/hdfs-scanner-ir.cc
M be/src/exec/hdfs-scanner.cc
M be/src/exec/hdfs-scanner.h
M be/src/exec/text-converter.cc
M be/src/exec/text-converter.h
6 files changed, 68 insertions(+), 12 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/59/16059/9
--
To view, visit http://gerrit.cloudera.org:8080/16059
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id370193af578ecf23ed3c6bfcc65fec448156fa3
Gerrit-Change-Number: 16059
Gerrit-PatchSet: 9
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-9747: More fine-grained codegen for text file scanners
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16059 )
Change subject: IMPALA-9747: More fine-grained codegen for text file scanners
......................................................................
Patch Set 13: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/16059
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id370193af578ecf23ed3c6bfcc65fec448156fa3
Gerrit-Change-Number: 16059
Gerrit-PatchSet: 13
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 24 Jun 2020 11:52:12 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9747: More fine-grained codegen for text file scanners
Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/16059 )
Change subject: IMPALA-9747: More fine-grained codegen for text file scanners
......................................................................
Patch Set 9:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/16059/8/be/src/exec/hdfs-scanner.cc
File be/src/exec/hdfs-scanner.cc:
http://gerrit.cloudera.org:8080/#/c/16059/8/be/src/exec/hdfs-scanner.cc@490
PS8, Line 490: llvm::Value* slot_idx_value = codegen->GetI32Constant(slot_idx);
: llvm::Function* interpreted_slot_fn = codegen->GetFunction(
: IRFunction::HDFS_SCANNER_TEXT_CONVERTER_WRITE_SLOT_INTERPRETED_IR, false);
: DCHECK(interpreted_slot_fn != nullptr);
:
> I don't think that we need to log this.
Done
http://gerrit.cloudera.org:8080/#/c/16059/8/be/src/exec/text-converter.cc
File be/src/exec/text-converter.cc:
http://gerrit.cloudera.org:8080/#/c/16059/8/be/src/exec/text-converter.cc@114
PS8, Line 114:
: // Codegen is_null_string
: bool is_default_null = (len == 2 && null_col_val[0] == '\\' && null_col_val[1] == 'N');
: llvm::Function* is_null_string_fn;
: if (is_default_null) {
: is_null_string_fn = codegen->GetFunction(IRFunction::IS_NULL_STRING, false);
: }
> Can we actually enter this block? If not, then a DCHECK would be better.
Done. Also added a comment in the header that states that this function should only be called if the type is supported.
--
To view, visit http://gerrit.cloudera.org:8080/16059
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id370193af578ecf23ed3c6bfcc65fec448156fa3
Gerrit-Change-Number: 16059
Gerrit-PatchSet: 9
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 Jun 2020 12:06:17 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9747: More fine-grained codegen for text file scanners
Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/16059 )
Change subject: IMPALA-9747: More fine-grained codegen for text file scanners
......................................................................
IMPALA-9747: More fine-grained codegen for text file scanners
Currently if the materialization of any column cannot be codegen'd
because its type is unsupported (e.g. CHAR(N)), the whole codegen is
cancelled for the text scanner.
This commit adds the function TextConverter::SupportsCodegenWriteSlot
that returns whether the given ColumnType is supported. If the type is
not supported, HdfsScanner codegens code that calls the interpreted
version instead of failing codegen. For other columns codegen is used as
usually.
Benchmarks:
Copied and modified a TPCH table with scale factor 5 to add a CHAR
column to it::
USE tpch5;
CREATE TABLE IF NOT EXISTS lineitem_char AS
SELECT *, CAST(l_shipdate AS CHAR(10)) l_shipdate_char
FROM lineitem;
Run the following query 100 times after one warm-up run with and
without this change:
SELECT *
FROM tpch5.lineitem_char
WHERE
l_partkey BETWEEN 500 AND 500000 AND
l_linestatus = 'F' AND
l_quantity < 35 AND
l_extendedprice BETWEEN 2000 AND 8000 AND
l_discount > 0 AND
l_tax BETWEEN 0.04 AND 0.06 AND
l_returnflag IN ('A', 'N') AND
l_shipdate_char < '1996-06-20'
ORDER BY l_shipdate_char
LIMIT 10;
Without this commit: mean: 2.92, standard deviation: 0.13.
With this commit: mean: 2.21, standard deviation: 0.072.
Testing:
TODO
Change-Id: Id370193af578ecf23ed3c6bfcc65fec448156fa3
---
M be/src/codegen/gen_ir_descriptions.py
M be/src/exec/hdfs-scanner-ir.cc
M be/src/exec/hdfs-scanner.cc
M be/src/exec/hdfs-scanner.h
M be/src/exec/text-converter.cc
M be/src/exec/text-converter.h
6 files changed, 76 insertions(+), 10 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/59/16059/6
--
To view, visit http://gerrit.cloudera.org:8080/16059
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id370193af578ecf23ed3c6bfcc65fec448156fa3
Gerrit-Change-Number: 16059
Gerrit-PatchSet: 6
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-9747: More fine-grained codegen for text file scanners
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16059 )
Change subject: IMPALA-9747: More fine-grained codegen for text file scanners
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/6271/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/16059
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id370193af578ecf23ed3c6bfcc65fec448156fa3
Gerrit-Change-Number: 16059
Gerrit-PatchSet: 3
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 10 Jun 2020 17:53:37 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9747: More fine-grained codegen for text file scanners
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16059 )
Change subject: IMPALA-9747: More fine-grained codegen for text file scanners
......................................................................
Patch Set 4:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/6285/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/16059
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id370193af578ecf23ed3c6bfcc65fec448156fa3
Gerrit-Change-Number: 16059
Gerrit-PatchSet: 4
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 11 Jun 2020 13:15:51 +0000
Gerrit-HasComments: No