[Impala-ASF-CR] IMPALA-7924: Generate Thrift 11 Python Code
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12036 ) Change subject: IMPALA-7924: Generate Thrift 11 Python Code .. Patch Set 4: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/12036 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3432c3e29d28ec3ef6a0a22156a18910f511fed0 Gerrit-Change-Number: 12036 Gerrit-PatchSet: 4 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 09 Jan 2019 05:54:58 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7924: Generate Thrift 11 Python Code
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12036 ) Change subject: IMPALA-7924: Generate Thrift 11 Python Code .. IMPALA-7924: Generate Thrift 11 Python Code Upgrades the version of the toolchain in order to pull in Thrift 0.11.0. Updates the CMake build to write generated Python code using Thrift 0.11 to shell/build/thrift-11-gen/gen-py/. The Thrift 0.11 Python deserialization code has some big performance improvements that allow faster parsing of runtime profiles. By adding the ability to generate the Thrift Python code using Thrift 0.11 we can take advantage of the Python performance improvements without going through a full Thrift upgrade from 0.9 to 0.11. Set USE_THRIFT11_GEN_PY=true and then run bin/set-pythonpath.sh to add the Thrift 0.11 Python generated code to the PYTHONPATH rather than the 0.9 generated code. Testing: - Ran core tests Change-Id: I3432c3e29d28ec3ef6a0a22156a18910f511fed0 Reviewed-on: http://gerrit.cloudera.org:8080/12036 Reviewed-by: Tim Armstrong Tested-by: Impala Public Jenkins --- M CMakeLists.txt M bin/bootstrap_toolchain.py M bin/impala-config.sh M bin/set-pythonpath.sh M common/thrift/CMakeLists.txt 5 files changed, 96 insertions(+), 38 deletions(-) Approvals: Tim Armstrong: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/12036 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I3432c3e29d28ec3ef6a0a22156a18910f511fed0 Gerrit-Change-Number: 12036 Gerrit-PatchSet: 5 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7924: Generate Thrift 11 Python Code
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12036 ) Change subject: IMPALA-7924: Generate Thrift 11 Python Code .. Patch Set 4: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3622/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/12036 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3432c3e29d28ec3ef6a0a22156a18910f511fed0 Gerrit-Change-Number: 12036 Gerrit-PatchSet: 4 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 09 Jan 2019 01:06:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7924: Generate Thrift 11 Python Code
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12036 ) Change subject: IMPALA-7924: Generate Thrift 11 Python Code .. Patch Set 4: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3619/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/12036 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3432c3e29d28ec3ef6a0a22156a18910f511fed0 Gerrit-Change-Number: 12036 Gerrit-PatchSet: 4 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 08 Jan 2019 18:15:42 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7924: Generate Thrift 11 Python Code
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12036 ) Change subject: IMPALA-7924: Generate Thrift 11 Python Code .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12036 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3432c3e29d28ec3ef6a0a22156a18910f511fed0 Gerrit-Change-Number: 12036 Gerrit-PatchSet: 4 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 08 Jan 2019 18:15:30 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7924: Generate Thrift 11 Python Code
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12036 ) Change subject: IMPALA-7924: Generate Thrift 11 Python Code .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1738/ : 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/12036 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3432c3e29d28ec3ef6a0a22156a18910f511fed0 Gerrit-Change-Number: 12036 Gerrit-PatchSet: 4 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 08 Jan 2019 15:16:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7924: Generate Thrift 11 Python Code
Hello Lars Volker, Philip Zeyliger, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12036 to look at the new patch set (#4). Change subject: IMPALA-7924: Generate Thrift 11 Python Code .. IMPALA-7924: Generate Thrift 11 Python Code Upgrades the version of the toolchain in order to pull in Thrift 0.11.0. Updates the CMake build to write generated Python code using Thrift 0.11 to shell/build/thrift-11-gen/gen-py/. The Thrift 0.11 Python deserialization code has some big performance improvements that allow faster parsing of runtime profiles. By adding the ability to generate the Thrift Python code using Thrift 0.11 we can take advantage of the Python performance improvements without going through a full Thrift upgrade from 0.9 to 0.11. Set USE_THRIFT11_GEN_PY=true and then run bin/set-pythonpath.sh to add the Thrift 0.11 Python generated code to the PYTHONPATH rather than the 0.9 generated code. Testing: - Ran core tests Change-Id: I3432c3e29d28ec3ef6a0a22156a18910f511fed0 --- M CMakeLists.txt M bin/bootstrap_toolchain.py M bin/impala-config.sh M bin/set-pythonpath.sh M common/thrift/CMakeLists.txt 5 files changed, 96 insertions(+), 38 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/36/12036/4 -- To view, visit http://gerrit.cloudera.org:8080/12036 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3432c3e29d28ec3ef6a0a22156a18910f511fed0 Gerrit-Change-Number: 12036 Gerrit-PatchSet: 4 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7924: Generate Thrift 11 Python Code
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12036 ) Change subject: IMPALA-7924: Generate Thrift 11 Python Code .. Patch Set 3: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/1736/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/12036 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3432c3e29d28ec3ef6a0a22156a18910f511fed0 Gerrit-Change-Number: 12036 Gerrit-PatchSet: 3 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 08 Jan 2019 13:58:49 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7924: Generate Thrift 11 Python Code
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/12036 ) Change subject: IMPALA-7924: Generate Thrift 11 Python Code .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/12036/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/12036/2//COMMIT_MSG@13 PS2, Line 13: The Thrift 11 Python deserialization code has some big performance > Might be worth briefly mentioning how to take advantage of it - does one ju Yes, it just needs to be added to the PYTHONPATH. I've updated the commit message, I also added some functionality to bin/set-pythonpath.sh that makes this a little more automated. -- To view, visit http://gerrit.cloudera.org:8080/12036 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3432c3e29d28ec3ef6a0a22156a18910f511fed0 Gerrit-Change-Number: 12036 Gerrit-PatchSet: 2 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 08 Jan 2019 13:52:57 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7924: Generate Thrift 11 Python Code
Hello Lars Volker, Philip Zeyliger, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12036 to look at the new patch set (#3). Change subject: IMPALA-7924: Generate Thrift 11 Python Code .. IMPALA-7924: Generate Thrift 11 Python Code Upgrades the version of the toolchain in order to pull in Thrift 0.11.0. Updates the CMake build to write generated Python code using Thrift 0.11 to shell/build/thrift-11-gen/gen-py/. The Thrift 0.11 Python deserialization code has some big performance improvements that allow faster parsing of runtime profiles. By adding the ability to generate the Thrift Python code using Thrift 0.11 we can take advantage of the Python performance improvements without going through a full Thrift upgrade from 0.9 to 0.11. Set USE_THRIFT11_GEN_PY=true and then run bin/set-pythonpath.sh to add the Thrift 0.11 Python generated code to the PYTHONPATH rather than the 0.9 generated code. Testing: - Ran core tests Change-Id: I3432c3e29d28ec3ef6a0a22156a18910f511fed0 --- M CMakeLists.txt M bin/bootstrap_toolchain.py M bin/impala-config.sh M bin/set-pythonpath.sh M common/thrift/CMakeLists.txt 5 files changed, 96 insertions(+), 38 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/36/12036/3 -- To view, visit http://gerrit.cloudera.org:8080/12036 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3432c3e29d28ec3ef6a0a22156a18910f511fed0 Gerrit-Change-Number: 12036 Gerrit-PatchSet: 3 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7924: Generate Thrift 11 Python Code
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12036 ) Change subject: IMPALA-7924: Generate Thrift 11 Python Code .. Patch Set 2: (2 comments) LGTM, just one ask about the commit message then I can start GVD http://gerrit.cloudera.org:8080/#/c/12036/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/12036/2//COMMIT_MSG@13 PS2, Line 13: The Thrift 11 Python deserialization code has some big performance Might be worth briefly mentioning how to take advantage of it - does one just set PYTHONPATH to include it manually? http://gerrit.cloudera.org:8080/#/c/12036/2/common/thrift/CMakeLists.txt File common/thrift/CMakeLists.txt: http://gerrit.cloudera.org:8080/#/c/12036/2/common/thrift/CMakeLists.txt@a230 PS2, Line 230: > thrift-deps depends on thrift11-py, which depends on thrift-ext-data-src. M Ah I missed that. I suppose that works. -- To view, visit http://gerrit.cloudera.org:8080/12036 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3432c3e29d28ec3ef6a0a22156a18910f511fed0 Gerrit-Change-Number: 12036 Gerrit-PatchSet: 2 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 07 Jan 2019 18:22:31 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7924: Generate Thrift 11 Python Code
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/12036 ) Change subject: IMPALA-7924: Generate Thrift 11 Python Code .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/12036/2/common/thrift/CMakeLists.txt File common/thrift/CMakeLists.txt: http://gerrit.cloudera.org:8080/#/c/12036/2/common/thrift/CMakeLists.txt@a230 PS2, Line 230: > Did you mean to remove this as a dependency? thrift-deps depends on thrift11-py, which depends on thrift-ext-data-src. My understanding is thats its basically a chain of dependencies, so the intention was to just add thrift11-py as a node in that chain. But I might be missing something, pretty new to CMake :) -- To view, visit http://gerrit.cloudera.org:8080/12036 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3432c3e29d28ec3ef6a0a22156a18910f511fed0 Gerrit-Change-Number: 12036 Gerrit-PatchSet: 2 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 07 Jan 2019 18:12:12 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7924: Generate Thrift 11 Python Code
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12036 ) Change subject: IMPALA-7924: Generate Thrift 11 Python Code .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/12036/2/common/thrift/CMakeLists.txt File common/thrift/CMakeLists.txt: http://gerrit.cloudera.org:8080/#/c/12036/2/common/thrift/CMakeLists.txt@a230 PS2, Line 230: Did you mean to remove this as a dependency? -- To view, visit http://gerrit.cloudera.org:8080/12036 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3432c3e29d28ec3ef6a0a22156a18910f511fed0 Gerrit-Change-Number: 12036 Gerrit-PatchSet: 2 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 07 Jan 2019 18:05:02 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7924: Generate Thrift 11 Python Code
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12036 ) Change subject: IMPALA-7924: Generate Thrift 11 Python Code .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1711/ : 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/12036 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3432c3e29d28ec3ef6a0a22156a18910f511fed0 Gerrit-Change-Number: 12036 Gerrit-PatchSet: 2 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Fri, 04 Jan 2019 14:44:49 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7924: Generate Thrift 11 Python Code
Sahil Takiar has posted comments on this change. (
http://gerrit.cloudera.org:8080/12036 )
Change subject: IMPALA-7924: Generate Thrift 11 Python Code
..
Patch Set 1:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/12036/1/common/thrift/CMakeLists.txt
File common/thrift/CMakeLists.txt:
http://gerrit.cloudera.org:8080/#/c/12036/1/common/thrift/CMakeLists.txt@125
PS1, Line 125: the
> double word
Done
http://gerrit.cloudera.org:8080/#/c/12036/1/common/thrift/CMakeLists.txt@128
PS1, Line 128: this method should be removed.
> Can you mention IMPALA-7825 here?
Done
http://gerrit.cloudera.org:8080/#/c/12036/1/common/thrift/CMakeLists.txt@140
PS1, Line 140: FIL_WE
> Are FIL and FIL_WE magic strings in CMake? Otherwise, can you find more des
No they aren't. Changed to THRIFT_FILE and THRIFT_FILE_WE. The WE stands for
'without extension', updated the comments to reflect that.
These variables were copied from the function THRIFT_GEN so I changed the
variables names there to make it easier to read too.
http://gerrit.cloudera.org:8080/#/c/12036/1/common/thrift/CMakeLists.txt@142
PS1, Line 142: unqiue
> typo
Done
http://gerrit.cloudera.org:8080/#/c/12036/1/common/thrift/CMakeLists.txt@159
PS1, Line 159: message("Using Thrift 11 compiler: ${THRIFT_COMPILER}")
> THRIFT11_COMPILER here, probably.
Done
http://gerrit.cloudera.org:8080/#/c/12036/1/common/thrift/CMakeLists.txt@180
PS1, Line 180: set(THRIFT11_PYTHON_ARGS ${THRIFT_INCLUDE_DIR_OPTION} -r --gen
py -o )
: set(THRIFT11_PYTHON_ARGS ${THRIFT11_PYTHON_ARGS}
${THRIFT11_PYTHON_OUTPUT_DIR})
> Very minor: you can probably use "\" to do line continuation or just stuff
Done
--
To view, visit http://gerrit.cloudera.org:8080/12036
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3432c3e29d28ec3ef6a0a22156a18910f511fed0
Gerrit-Change-Number: 12036
Gerrit-PatchSet: 1
Gerrit-Owner: Sahil Takiar
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Reviewer: Sahil Takiar
Gerrit-Comment-Date: Fri, 04 Jan 2019 14:14:06 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7924: Generate Thrift 11 Python Code
Hello Lars Volker, Philip Zeyliger, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12036 to look at the new patch set (#2). Change subject: IMPALA-7924: Generate Thrift 11 Python Code .. IMPALA-7924: Generate Thrift 11 Python Code Upgrades the version of the toolchain in order to pull in Thrift 0.11.0. Updates the CMake build to write generated Python code using Thrift 11 to shell/build/thrift-11-gen/gen-py/. The Thrift 11 Python deserialization code has some big performance improvements that allow faster parsing of runtime profiles. By adding the ability to generate the Thrift Python code using Thrift 11 we can take advantage of the Python performance improvements without going through a full Thrift upgrade from 0.9 to 0.11. Testing: - Ran core tests Change-Id: I3432c3e29d28ec3ef6a0a22156a18910f511fed0 --- M CMakeLists.txt M bin/bootstrap_toolchain.py M bin/impala-config.sh M common/thrift/CMakeLists.txt 4 files changed, 87 insertions(+), 37 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/36/12036/2 -- To view, visit http://gerrit.cloudera.org:8080/12036 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3432c3e29d28ec3ef6a0a22156a18910f511fed0 Gerrit-Change-Number: 12036 Gerrit-PatchSet: 2 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger
[Impala-ASF-CR] IMPALA-7924: Generate Thrift 11 Python Code
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/12036 ) Change subject: IMPALA-7924: Generate Thrift 11 Python Code .. Patch Set 1: Code-Review+1 (4 comments) http://gerrit.cloudera.org:8080/#/c/12036/1/common/thrift/CMakeLists.txt File common/thrift/CMakeLists.txt: http://gerrit.cloudera.org:8080/#/c/12036/1/common/thrift/CMakeLists.txt@125 PS1, Line 125: the double word http://gerrit.cloudera.org:8080/#/c/12036/1/common/thrift/CMakeLists.txt@128 PS1, Line 128: this method should be removed. Can you mention IMPALA-7825 here? http://gerrit.cloudera.org:8080/#/c/12036/1/common/thrift/CMakeLists.txt@140 PS1, Line 140: FIL_WE Are FIL and FIL_WE magic strings in CMake? Otherwise, can you find more descriptive names? http://gerrit.cloudera.org:8080/#/c/12036/1/common/thrift/CMakeLists.txt@142 PS1, Line 142: unqiue typo -- To view, visit http://gerrit.cloudera.org:8080/12036 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3432c3e29d28ec3ef6a0a22156a18910f511fed0 Gerrit-Change-Number: 12036 Gerrit-PatchSet: 1 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Wed, 12 Dec 2018 19:24:44 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7924: Generate Thrift 11 Python Code
Philip Zeyliger has posted comments on this change. (
http://gerrit.cloudera.org:8080/12036 )
Change subject: IMPALA-7924: Generate Thrift 11 Python Code
..
Patch Set 1: Code-Review+1
(2 comments)
Thanks! Only minor stuff from me.
http://gerrit.cloudera.org:8080/#/c/12036/1/common/thrift/CMakeLists.txt
File common/thrift/CMakeLists.txt:
http://gerrit.cloudera.org:8080/#/c/12036/1/common/thrift/CMakeLists.txt@159
PS1, Line 159: message("Using Thrift 11 compiler: ${THRIFT_COMPILER}")
THRIFT11_COMPILER here, probably.
http://gerrit.cloudera.org:8080/#/c/12036/1/common/thrift/CMakeLists.txt@180
PS1, Line 180: set(THRIFT11_PYTHON_ARGS ${THRIFT_INCLUDE_DIR_OPTION} -r --gen
py -o )
: set(THRIFT11_PYTHON_ARGS ${THRIFT11_PYTHON_ARGS}
${THRIFT11_PYTHON_OUTPUT_DIR})
Very minor: you can probably use "\" to do line continuation or just stuff this
in a single line and ignore character limit. It's a bit hard to parse that this
is just "+=" effectively.
https://cmake.org/cmake/help/v3.0/manual/cmake-language.7.html#quoted-argument
--
To view, visit http://gerrit.cloudera.org:8080/12036
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3432c3e29d28ec3ef6a0a22156a18910f511fed0
Gerrit-Change-Number: 12036
Gerrit-PatchSet: 1
Gerrit-Owner: Sahil Takiar
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Comment-Date: Wed, 05 Dec 2018 18:12:55 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7924: Generate Thrift 11 Python Code
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12036 ) Change subject: IMPALA-7924: Generate Thrift 11 Python Code .. Patch Set 1: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/1544/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/12036 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3432c3e29d28ec3ef6a0a22156a18910f511fed0 Gerrit-Change-Number: 12036 Gerrit-PatchSet: 1 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Comment-Date: Wed, 05 Dec 2018 16:56:58 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7924: Generate Thrift 11 Python Code
Sahil Takiar has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12036 Change subject: IMPALA-7924: Generate Thrift 11 Python Code .. IMPALA-7924: Generate Thrift 11 Python Code Upgrades the version of the toolchain in order to pull in Thrift 0.11.0. Updates the CMake build to write generated Python code using Thrift 11 to shell/build/thrift-11-gen/gen-py/. The Thrift 11 Python deserialization code has some big performance improvements that allow faster parsing of runtime profiles. By adding the ability to generate the Thrift Python code using Thrift 11 we can take advantage of the Python performance improvements without going through a full Thrift upgrade from 0.9 to 0.11. Testing: - Ran core tests Change-Id: I3432c3e29d28ec3ef6a0a22156a18910f511fed0 --- M CMakeLists.txt M bin/impala-config.sh M common/thrift/CMakeLists.txt 3 files changed, 51 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/36/12036/1 -- To view, visit http://gerrit.cloudera.org:8080/12036 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I3432c3e29d28ec3ef6a0a22156a18910f511fed0 Gerrit-Change-Number: 12036 Gerrit-PatchSet: 1 Gerrit-Owner: Sahil Takiar
