You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Fredy Wijaya (Code Review)" <ge...@cloudera.org> on 2019/01/22 21:37:45 UTC
[Impala-ASF-CR] IMPALA-7934: Switch to java.util.Base64 implementation
Fredy Wijaya has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12250
Change subject: IMPALA-7934: Switch to java.util.Base64 implementation
......................................................................
IMPALA-7934: Switch to java.util.Base64 implementation
It is shown that java.util.Base64 implementation seems to have better
performance compared to Apache Commons Codec's Base64 implementation,
which can benefit operations, such as incremental stats. This patch
switches the implementation of Base64 from Apache Commons
Codec to java.util.Base64 implementation.
Testing:
- Ran all FE tests
Change-Id: I2d43d4a4f073a800d963ce4c77f21c9efa8471ac
---
M fe/src/main/java/org/apache/impala/catalog/Db.java
M fe/src/main/java/org/apache/impala/catalog/PartitionStatsUtil.java
M fe/src/main/java/org/apache/impala/util/FunctionUtils.java
M fe/src/test/java/org/apache/impala/catalog/CatalogTest.java
4 files changed, 9 insertions(+), 9 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/50/12250/2
--
To view, visit http://gerrit.cloudera.org:8080/12250
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2d43d4a4f073a800d963ce4c77f21c9efa8471ac
Gerrit-Change-Number: 12250
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
[Impala-ASF-CR] IMPALA-7934: Switch to java.util.Base64 implementation
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/12250 )
Change subject: IMPALA-7934: Switch to java.util.Base64 implementation
......................................................................
IMPALA-7934: Switch to java.util.Base64 implementation
It is shown that java.util.Base64 implementation seems to have better
performance compared to Apache Commons Codec's Base64 implementation,
which can benefit operations, such as incremental stats. This patch
switches the implementation of Base64 from Apache Commons
Codec to java.util.Base64 implementation.
This is the JMH benchmark result comparing java.util.Base64 vs Commons
Codec's Base64:
Result "base64.Base64Benchmark.javaBase64":
31.149 ±(99.9%) 1.567 ms/op [Average]
(min, avg, max) = (27.564, 31.149, 34.675), stdev = 2.091
CI (99.9%): [29.583, 32.716] (assumes normal distribution)
Result "base64.Base64Benchmark.codecBase64":
65.921 ±(99.9%) 4.762 ms/op [Average]
(min, avg, max) = (58.072, 65.921, 80.470), stdev = 6.357
CI (99.9%): [61.159, 70.683] (assumes normal distribution)
Benchmark Mode Cnt Score Error Units
Base64Benchmark.javaBase64 avgt 25 31.149 ± 1.567 ms/op
Base64Benchmark.codecBase64 avgt 25 65.921 ± 4.762 ms/op
Testing:
- Ran all FE tests
- Created a table with incremental stats without a patch and read it
with the patch
Change-Id: I2d43d4a4f073a800d963ce4c77f21c9efa8471ac
---
M fe/src/main/java/org/apache/impala/catalog/Db.java
M fe/src/main/java/org/apache/impala/catalog/PartitionStatsUtil.java
M fe/src/main/java/org/apache/impala/util/FunctionUtils.java
M fe/src/test/java/org/apache/impala/catalog/CatalogTest.java
4 files changed, 9 insertions(+), 9 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/50/12250/4
--
To view, visit http://gerrit.cloudera.org:8080/12250
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2d43d4a4f073a800d963ce4c77f21c9efa8471ac
Gerrit-Change-Number: 12250
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-7934: Switch to java.util.Base64 implementation
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12250 )
Change subject: IMPALA-7934: Switch to java.util.Base64 implementation
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1846/ : 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/12250
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d43d4a4f073a800d963ce4c77f21c9efa8471ac
Gerrit-Change-Number: 12250
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 22 Jan 2019 22:22:41 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7934: Switch to java.util.Base64 implementation
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/12250 )
Change subject: IMPALA-7934: Switch to java.util.Base64 implementation
......................................................................
IMPALA-7934: Switch to java.util.Base64 implementation
It is shown that java.util.Base64 implementation seems to have better
performance compared to Apache Commons Codec's Base64 implementation,
which can benefit operations, such as incremental stats. This patch
switches the implementation of Base64 from Apache Commons
Codec to java.util.Base64 implementation.
This is the JMH benchmark result comparing java.util.Base64 vs Commons
Codec's Base64:
Result "base64.Base64Benchmark.javaBase64":
31.149 ±(99.9%) 1.567 ms/op [Average]
(min, avg, max) = (27.564, 31.149, 34.675), stdev = 2.091
CI (99.9%): [29.583, 32.716] (assumes normal distribution)
Result "base64.Base64Benchmark.codecBase64":
65.921 ±(99.9%) 4.762 ms/op [Average]
(min, avg, max) = (58.072, 65.921, 80.470), stdev = 6.357
CI (99.9%): [61.159, 70.683] (assumes normal distribution)
Benchmark Mode Cnt Score Error Units
Base64Benchmark.javaBase64 avgt 25 31.149 ± 1.567 ms/op
Base64Benchmark.codecBase64 avgt 25 65.921 ± 4.762 ms/op
Testing:
- Ran all FE tests
- Created a table with incremental stats without a patch and read it
with the patch
Change-Id: I2d43d4a4f073a800d963ce4c77f21c9efa8471ac
Reviewed-on: http://gerrit.cloudera.org:8080/12250
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/catalog/Db.java
M fe/src/main/java/org/apache/impala/catalog/PartitionStatsUtil.java
M fe/src/main/java/org/apache/impala/util/FunctionUtils.java
M fe/src/test/java/org/apache/impala/catalog/CatalogTest.java
4 files changed, 9 insertions(+), 9 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/12250
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I2d43d4a4f073a800d963ce4c77f21c9efa8471ac
Gerrit-Change-Number: 12250
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-7934: Switch to java.util.Base64 implementation
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12250 )
Change subject: IMPALA-7934: Switch to java.util.Base64 implementation
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1948/ : 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/12250
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d43d4a4f073a800d963ce4c77f21c9efa8471ac
Gerrit-Change-Number: 12250
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 Feb 2019 02:23:39 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7934: Switch to java.util.Base64 implementation
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12250 )
Change subject: IMPALA-7934: Switch to java.util.Base64 implementation
......................................................................
Patch Set 5:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3694/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/12250
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d43d4a4f073a800d963ce4c77f21c9efa8471ac
Gerrit-Change-Number: 12250
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 Feb 2019 02:32:14 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7934: Switch to java.util.Base64 implementation
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12250 )
Change subject: IMPALA-7934: Switch to java.util.Base64 implementation
......................................................................
Patch Set 4:
(2 comments)
Added some benchmark numbers.
http://gerrit.cloudera.org:8080/#/c/12250/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/12250/2//COMMIT_MSG@7
PS2, Line 7: IMPALA-7934: Switch to java.util.Base64 implementation
> I'm pretty sure it is compatible with commons' base64 but could you please
I did. I updated the git commit message.
http://gerrit.cloudera.org:8080/#/c/12250/2//COMMIT_MSG@14
PS2, Line 14:
> Did you see any perf improvements in the loading time of large tables with
I didn't see any significant improvements. However, I did a microbenchmark on the two implementations using JMH and java.util.Base64 performs better overall. So, I think we should still switch to java.util.Base64. I updated the git commit message to reflect that.
--
To view, visit http://gerrit.cloudera.org:8080/12250
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d43d4a4f073a800d963ce4c77f21c9efa8471ac
Gerrit-Change-Number: 12250
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 Feb 2019 01:42:28 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7934: Switch to java.util.Base64 implementation
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12250 )
Change subject: IMPALA-7934: Switch to java.util.Base64 implementation
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/12250
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d43d4a4f073a800d963ce4c77f21c9efa8471ac
Gerrit-Change-Number: 12250
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 Feb 2019 02:32:13 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7934: Switch to java.util.Base64 implementation
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/12250 )
Change subject: IMPALA-7934: Switch to java.util.Base64 implementation
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/12250
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d43d4a4f073a800d963ce4c77f21c9efa8471ac
Gerrit-Change-Number: 12250
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 Feb 2019 02:31:22 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7934: Switch to java.util.Base64 implementation
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12250 )
Change subject: IMPALA-7934: Switch to java.util.Base64 implementation
......................................................................
Patch Set 5: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/12250
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d43d4a4f073a800d963ce4c77f21c9efa8471ac
Gerrit-Change-Number: 12250
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 Feb 2019 06:34:06 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7934: Switch to java.util.Base64 implementation
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/12250 )
Change subject: IMPALA-7934: Switch to java.util.Base64 implementation
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/12250/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/12250/2//COMMIT_MSG@7
PS2, Line 7: IMPALA-7934: Switch to java.util.Base64 implementation
I'm pretty sure it is compatible with commons' base64 but could you please double check manually (if you haven't already done so)? Basically by creating a table with incremental stats (or persistent functions) without the patch, then trying to read them after applying this patch.
http://gerrit.cloudera.org:8080/#/c/12250/2//COMMIT_MSG@14
PS2, Line 14:
Did you see any perf improvements in the loading time of large tables with incremental stats?
--
To view, visit http://gerrit.cloudera.org:8080/12250
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d43d4a4f073a800d963ce4c77f21c9efa8471ac
Gerrit-Change-Number: 12250
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 23 Jan 2019 21:21:54 +0000
Gerrit-HasComments: Yes