You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Tim Armstrong (Code Review)" <ge...@cloudera.org> on 2018/02/05 19:01:47 UTC
[Impala-ASF-CR] Force inlining of BloomFilter::MakeMask
Tim Armstrong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9214
Change subject: Force inlining of BloomFilter::MakeMask
......................................................................
Force inlining of BloomFilter::MakeMask
I noticed that this function was showing up in perf top for TPC-H Q8
running locally. It wasn't inlined into BloomFilter::BucketFindAVX2.
Inlining made the query ~5% faster for me locally.
Change-Id: I89282f6c315570bea5ad8a0f854cb6eea0592923
---
M be/src/util/bloom-filter.h
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/9214/1
--
To view, visit http://gerrit.cloudera.org:8080/9214
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I89282f6c315570bea5ad8a0f854cb6eea0592923
Gerrit-Change-Number: 9214
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] Force inlining of BloomFilter::MakeMask
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/9214 )
Change subject: Force inlining of BloomFilter::MakeMask
......................................................................
Force inlining of BloomFilter::MakeMask
I noticed that this function was showing up in perf top for TPC-H Q8
running locally. It wasn't inlined into BloomFilter::BucketFindAVX2.
Inlining made the query ~5% faster for me locally.
Change-Id: I89282f6c315570bea5ad8a0f854cb6eea0592923
Reviewed-on: http://gerrit.cloudera.org:8080/9214
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
Tested-by: Impala Public Jenkins
---
M be/src/benchmarks/bloom-filter-benchmark.cc
M be/src/util/bloom-filter.h
2 files changed, 86 insertions(+), 84 deletions(-)
Approvals:
Tim Armstrong: Looks good to me, approved
Impala Public Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/9214
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I89282f6c315570bea5ad8a0f854cb6eea0592923
Gerrit-Change-Number: 9214
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Mostafa Mokhtar <mm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] Force inlining of BloomFilter::MakeMask
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/9214 )
Change subject: Force inlining of BloomFilter::MakeMask
......................................................................
Patch Set 4: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/1891/
--
To view, visit http://gerrit.cloudera.org:8080/9214
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I89282f6c315570bea5ad8a0f854cb6eea0592923
Gerrit-Change-Number: 9214
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Mostafa Mokhtar <mm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Feb 2018 20:22:09 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] Force inlining of BloomFilter::MakeMask
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/9214 )
Change subject: Force inlining of BloomFilter::MakeMask
......................................................................
Force inlining of BloomFilter::MakeMask
I noticed that this function was showing up in perf top for TPC-H Q8
running locally. It wasn't inlined into BloomFilter::BucketFindAVX2.
Inlining made the query ~5% faster for me locally.
Change-Id: I89282f6c315570bea5ad8a0f854cb6eea0592923
---
M be/src/util/bloom-filter.h
1 file changed, 2 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/9214/2
--
To view, visit http://gerrit.cloudera.org:8080/9214
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I89282f6c315570bea5ad8a0f854cb6eea0592923
Gerrit-Change-Number: 9214
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] Force inlining of BloomFilter::MakeMask
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/9214 )
Change subject: Force inlining of BloomFilter::MakeMask
......................................................................
Patch Set 4:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1891/
--
To view, visit http://gerrit.cloudera.org:8080/9214
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I89282f6c315570bea5ad8a0f854cb6eea0592923
Gerrit-Change-Number: 9214
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Mostafa Mokhtar <mm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Feb 2018 20:19:33 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] Force inlining of BloomFilter::MakeMask
Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change. ( http://gerrit.cloudera.org:8080/9214 )
Change subject: Force inlining of BloomFilter::MakeMask
......................................................................
Patch Set 2: Code-Review+1
Just for sanity checking, did you run bloom-filter-benchmark?
--
To view, visit http://gerrit.cloudera.org:8080/9214
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I89282f6c315570bea5ad8a0f854cb6eea0592923
Gerrit-Change-Number: 9214
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Mostafa Mokhtar <mm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Feb 2018 17:13:04 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] Force inlining of BloomFilter::MakeMask
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/9214 )
Change subject: Force inlining of BloomFilter::MakeMask
......................................................................
Patch Set 5: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/9214
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I89282f6c315570bea5ad8a0f854cb6eea0592923
Gerrit-Change-Number: 9214
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Mostafa Mokhtar <mm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Feb 2018 03:49:39 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] Force inlining of BloomFilter::MakeMask
Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change. ( http://gerrit.cloudera.org:8080/9214 )
Change subject: Force inlining of BloomFilter::MakeMask
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/9214
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I89282f6c315570bea5ad8a0f854cb6eea0592923
Gerrit-Change-Number: 9214
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Mostafa Mokhtar <mm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Feb 2018 20:15:24 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] Force inlining of BloomFilter::MakeMask
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9214 )
Change subject: Force inlining of BloomFilter::MakeMask
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/9214/2/be/src/util/bloom-filter.h
File be/src/util/bloom-filter.h:
http://gerrit.cloudera.org:8080/#/c/9214/2/be/src/util/bloom-filter.h@162
PS2, Line 162: __attribute__((__target__("avx2")));
> Yes, I see it too in BucketFindAvx2
The __target__ is needed for GCC to emit AVX2 instructions at all, the multi-versioning thing is really an overloading of the __target__ attribute
--
To view, visit http://gerrit.cloudera.org:8080/9214
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I89282f6c315570bea5ad8a0f854cb6eea0592923
Gerrit-Change-Number: 9214
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Mostafa Mokhtar <mm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Feb 2018 16:52:25 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] Force inlining of BloomFilter::MakeMask
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9214 )
Change subject: Force inlining of BloomFilter::MakeMask
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/9214
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I89282f6c315570bea5ad8a0f854cb6eea0592923
Gerrit-Change-Number: 9214
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Mostafa Mokhtar <mm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Feb 2018 20:19:27 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] Force inlining of BloomFilter::MakeMask
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9214 )
Change subject: Force inlining of BloomFilter::MakeMask
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/9214
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I89282f6c315570bea5ad8a0f854cb6eea0592923
Gerrit-Change-Number: 9214
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Mostafa Mokhtar <mm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Feb 2018 00:08:31 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] Force inlining of BloomFilter::MakeMask
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9214 )
Change subject: Force inlining of BloomFilter::MakeMask
......................................................................
Patch Set 2:
On my system after:
With AVX2:
insert: Function iters/ms 10%ile 50%ile 90%ile 10%ile 50%ile 90%ile
(relative) (relative) (relative)
---------------------------------------------------------------------------------------------------------
ndv 10k fpp 10.0% 2.1e+05 2.11e+05 2.13e+05 1X 1X 1X
ndv 10k fpp 1.0% 2.16e+05 2.18e+05 2.19e+05 1.03X 1.03X 1.03X
ndv 10k fpp 0.1% 2.12e+05 2.14e+05 2.16e+05 1.01X 1.01X 1.01X
ndv 1000k fpp 10.0% 1.98e+05 1.99e+05 2.01e+05 0.943X 0.942X 0.945X
ndv 1000k fpp 1.0% 1.96e+05 1.98e+05 1.99e+05 0.935X 0.936X 0.937X
ndv 1000k fpp 0.1% 1.96e+05 1.97e+05 1.99e+05 0.935X 0.934X 0.936X
ndv 100000k fpp 10.0% 5.63e+04 5.8e+04 6.18e+04 0.269X 0.274X 0.291X
ndv 100000k fpp 1.0% 5.64e+04 5.84e+04 6.24e+04 0.269X 0.276X 0.293X
ndv 100000k fpp 0.1% 5.56e+04 5.75e+04 5.86e+04 0.265X 0.272X 0.275X
find: Function iters/ms 10%ile 50%ile 90%ile 10%ile 50%ile 90%ile
(relative) (relative) (relative)
---------------------------------------------------------------------------------------------------------
present ndv 10k fpp 10.0% 1.97e+05 1.98e+05 1.99e+05 1X 1X 1X
absent ndv 10k fpp 10.0% 1.99e+05 2.01e+05 2.03e+05 1.01X 1.01X 1.02X
present ndv 10k fpp 1.0% 1.97e+05 1.98e+05 2e+05 1X 1X 1X
absent ndv 10k fpp 1.0% 2e+05 2.01e+05 2.03e+05 1.02X 1.02X 1.02X
present ndv 10k fpp 0.1% 1.97e+05 1.99e+05 2e+05 1X 1X 1X
absent ndv 10k fpp 0.1% 2e+05 2.02e+05 2.03e+05 1.02X 1.02X 1.02X
present ndv 1000k fpp 10.0% 1.75e+05 1.77e+05 1.78e+05 0.891X 0.893X 0.893X
absent ndv 1000k fpp 10.0% 1.78e+05 1.8e+05 1.81e+05 0.907X 0.907X 0.907X
present ndv 1000k fpp 1.0% 1.8e+05 1.82e+05 1.83e+05 0.917X 0.917X 0.919X
absent ndv 1000k fpp 1.0% 1.84e+05 1.86e+05 1.88e+05 0.937X 0.939X 0.941X
present ndv 1000k fpp 0.1% 1.69e+05 1.7e+05 1.71e+05 0.857X 0.859X 0.858X
absent ndv 1000k fpp 0.1% 1.7e+05 1.72e+05 1.74e+05 0.866X 0.87X 0.871X
present ndv 100000k fpp 10.0% 5.34e+04 5.53e+04 7.21e+04 0.271X 0.279X 0.362X
absent ndv 100000k fpp 10.0% 5.05e+04 5.28e+04 5.52e+04 0.257X 0.267X 0.277X
present ndv 100000k fpp 1.0% 5.43e+04 5.74e+04 8.65e+04 0.276X 0.29X 0.434X
absent ndv 100000k fpp 1.0% 5.09e+04 5.42e+04 5.73e+04 0.259X 0.274X 0.288X
present ndv 100000k fpp 0.1% 5.11e+04 5.24e+04 6.69e+04 0.26X 0.265X 0.336X
absent ndv 100000k fpp 0.1% 4.93e+04 5.02e+04 5.1e+04 0.251X 0.254X 0.256X
union: Function iters/ms 10%ile 50%ile 90%ile 10%ile 50%ile 90%ile
(relative) (relative) (relative)
---------------------------------------------------------------------------------------------------------
ndv 10k fpp 10.0% 6.76e+05 6.8e+05 6.88e+05 1X 1X 1X
ndv 10k fpp 1.0% 6.77e+05 6.81e+05 6.87e+05 1X 1X 0.998X
ndv 10k fpp 0.1% 6.78e+05 6.82e+05 6.86e+05 1X 1X 0.996X
ndv 1000k fpp 10.0% 6.78e+05 6.82e+05 6.88e+05 1X 1X 1X
ndv 1000k fpp 1.0% 6.78e+05 6.83e+05 6.89e+05 1X 1X 1X
ndv 1000k fpp 0.1% 6.77e+05 6.8e+05 6.89e+05 1X 1X 1X
ndv 100000k fpp 10.0% 6.77e+05 6.81e+05 6.88e+05 1X 1X 0.999X
ndv 100000k fpp 1.0% 6.77e+05 6.85e+05 6.89e+05 1X 1.01X 1X
ndv 100000k fpp 0.1% 6.76e+05 6.8e+05 6.88e+05 1X 1X 1X
--
To view, visit http://gerrit.cloudera.org:8080/9214
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I89282f6c315570bea5ad8a0f854cb6eea0592923
Gerrit-Change-Number: 9214
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Mostafa Mokhtar <mm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Feb 2018 20:06:03 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] Force inlining of BloomFilter::MakeMask
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Hello Jim Apple, Mostafa Mokhtar,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/9214
to look at the new patch set (#3).
Change subject: Force inlining of BloomFilter::MakeMask
......................................................................
Force inlining of BloomFilter::MakeMask
I noticed that this function was showing up in perf top for TPC-H Q8
running locally. It wasn't inlined into BloomFilter::BucketFindAVX2.
Inlining made the query ~5% faster for me locally.
Change-Id: I89282f6c315570bea5ad8a0f854cb6eea0592923
---
M be/src/benchmarks/bloom-filter-benchmark.cc
M be/src/util/bloom-filter.h
2 files changed, 86 insertions(+), 84 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/9214/3
--
To view, visit http://gerrit.cloudera.org:8080/9214
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I89282f6c315570bea5ad8a0f854cb6eea0592923
Gerrit-Change-Number: 9214
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Mostafa Mokhtar <mm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] Force inlining of BloomFilter::MakeMask
Posted by "Mostafa Mokhtar (Code Review)" <ge...@cloudera.org>.
Mostafa Mokhtar has posted comments on this change. ( http://gerrit.cloudera.org:8080/9214 )
Change subject: Force inlining of BloomFilter::MakeMask
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/9214/2/be/src/util/bloom-filter.h
File be/src/util/bloom-filter.h:
http://gerrit.cloudera.org:8080/#/c/9214/2/be/src/util/bloom-filter.h@162
PS2, Line 162: __attribute__((__target__("avx2")));
Yes, I see it too in BucketFindAvx2
0.67 ? mov %rdi,%r12 ?
1.07 ? mov %esi,%ebx ?
0.56 ? mov %edx,%edi ?
1.32 ? shl $0x5,%rbx ?
0.72 ? sub $0x18,%rsp ?
1.73 ? ? callq impala::BloomFilter::MakeMask(unsigned int)
Wondering if the multi-versioning attribute is still needed since MakeMask only called from BucketFindAVX2 and BucketInsertAVX2 which also have (__target__("avx2")).
--
To view, visit http://gerrit.cloudera.org:8080/9214
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I89282f6c315570bea5ad8a0f854cb6eea0592923
Gerrit-Change-Number: 9214
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Mostafa Mokhtar <mm...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Feb 2018 16:50:08 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] Force inlining of BloomFilter::MakeMask
Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change. ( http://gerrit.cloudera.org:8080/9214 )
Change subject: Force inlining of BloomFilter::MakeMask
......................................................................
Patch Set 3:
> On my system after:
>
> With AVX2:
>
> insert: Function iters/ms 10%ile 50%ile
> 90%ile 10%ile 50%ile 90%ile
> (relative) (relative) (relative)
> ---------------------------------------------------------------------------------------------------------
> ndv 10k fpp 10.0% 2.1e+05 2.11e+05 2.13e+05
> 1X 1X 1X
> ndv 10k fpp 1.0% 2.16e+05 2.18e+05 2.19e+05
> 1.03X 1.03X 1.03X
> ndv 10k fpp 0.1% 2.12e+05 2.14e+05 2.16e+05
> 1.01X 1.01X 1.01X
> ndv 1000k fpp 10.0% 1.98e+05 1.99e+05 2.01e+05
> 0.943X 0.942X 0.945X
> ndv 1000k fpp 1.0% 1.96e+05 1.98e+05 1.99e+05
> 0.935X 0.936X 0.937X
> ndv 1000k fpp 0.1% 1.96e+05 1.97e+05 1.99e+05
> 0.935X 0.934X 0.936X
> ndv 100000k fpp 10.0% 5.63e+04 5.8e+04 6.18e+04
> 0.269X 0.274X 0.291X
> ndv 100000k fpp 1.0% 5.64e+04 5.84e+04 6.24e+04
> 0.269X 0.276X 0.293X
> ndv 100000k fpp 0.1% 5.56e+04 5.75e+04 5.86e+04
> 0.265X 0.272X 0.275X
>
> find: Function iters/ms 10%ile 50%ile
> 90%ile 10%ile 50%ile 90%ile
> (relative) (relative) (relative)
> ---------------------------------------------------------------------------------------------------------
> present ndv 10k fpp 10.0% 1.97e+05 1.98e+05
> 1.99e+05 1X 1X 1X
> absent ndv 10k fpp 10.0% 1.99e+05 2.01e+05
> 2.03e+05 1.01X 1.01X 1.02X
> present ndv 10k fpp 1.0% 1.97e+05 1.98e+05
> 2e+05 1X 1X 1X
> absent ndv 10k fpp 1.0% 2e+05 2.01e+05
> 2.03e+05 1.02X 1.02X 1.02X
> present ndv 10k fpp 0.1% 1.97e+05 1.99e+05
> 2e+05 1X 1X 1X
> absent ndv 10k fpp 0.1% 2e+05 2.02e+05
> 2.03e+05 1.02X 1.02X 1.02X
> present ndv 1000k fpp 10.0% 1.75e+05 1.77e+05
> 1.78e+05 0.891X 0.893X 0.893X
> absent ndv 1000k fpp 10.0% 1.78e+05 1.8e+05
> 1.81e+05 0.907X 0.907X 0.907X
> present ndv 1000k fpp 1.0% 1.8e+05 1.82e+05
> 1.83e+05 0.917X 0.917X 0.919X
> absent ndv 1000k fpp 1.0% 1.84e+05 1.86e+05
> 1.88e+05 0.937X 0.939X 0.941X
> present ndv 1000k fpp 0.1% 1.69e+05 1.7e+05
> 1.71e+05 0.857X 0.859X 0.858X
> absent ndv 1000k fpp 0.1% 1.7e+05 1.72e+05
> 1.74e+05 0.866X 0.87X 0.871X
> present ndv 100000k fpp 10.0% 5.34e+04 5.53e+04
> 7.21e+04 0.271X 0.279X 0.362X
> absent ndv 100000k fpp 10.0% 5.05e+04 5.28e+04
> 5.52e+04 0.257X 0.267X 0.277X
> present ndv 100000k fpp 1.0% 5.43e+04 5.74e+04
> 8.65e+04 0.276X 0.29X 0.434X
> absent ndv 100000k fpp 1.0% 5.09e+04 5.42e+04
> 5.73e+04 0.259X 0.274X 0.288X
> present ndv 100000k fpp 0.1% 5.11e+04 5.24e+04
> 6.69e+04 0.26X 0.265X 0.336X
> absent ndv 100000k fpp 0.1% 4.93e+04 5.02e+04
> 5.1e+04 0.251X 0.254X 0.256X
>
> union: Function iters/ms 10%ile 50%ile
> 90%ile 10%ile 50%ile 90%ile
> (relative) (relative) (relative)
> ---------------------------------------------------------------------------------------------------------
> ndv 10k fpp 10.0% 6.76e+05 6.8e+05 6.88e+05
> 1X 1X 1X
> ndv 10k fpp 1.0% 6.77e+05 6.81e+05 6.87e+05
> 1X 1X 0.998X
> ndv 10k fpp 0.1% 6.78e+05 6.82e+05 6.86e+05
> 1X 1X 0.996X
> ndv 1000k fpp 10.0% 6.78e+05 6.82e+05 6.88e+05
> 1X 1X 1X
> ndv 1000k fpp 1.0% 6.78e+05 6.83e+05 6.89e+05
> 1X 1X 1X
> ndv 1000k fpp 0.1% 6.77e+05 6.8e+05 6.89e+05
> 1X 1X 1X
> ndv 100000k fpp 10.0% 6.77e+05 6.81e+05 6.88e+05
> 1X 1X 0.999X
> ndv 100000k fpp 1.0% 6.77e+05 6.85e+05 6.89e+05
> 1X 1.01X 1X
> ndv 100000k fpp 0.1% 6.76e+05 6.8e+05 6.88e+05
> 1X 1X 1X
Why did union speed up? In any case, +2, but I'd be curious about
cat /sys/devices/system/cpu/intel_pstate/no_turbo
cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_governor
--
To view, visit http://gerrit.cloudera.org:8080/9214
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I89282f6c315570bea5ad8a0f854cb6eea0592923
Gerrit-Change-Number: 9214
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Mostafa Mokhtar <mm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Feb 2018 20:15:14 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] Force inlining of BloomFilter::MakeMask
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/9214 )
Change subject: Force inlining of BloomFilter::MakeMask
......................................................................
Patch Set 5:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1895/
--
To view, visit http://gerrit.cloudera.org:8080/9214
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I89282f6c315570bea5ad8a0f854cb6eea0592923
Gerrit-Change-Number: 9214
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Mostafa Mokhtar <mm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Feb 2018 00:08:39 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] Force inlining of BloomFilter::MakeMask
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9214 )
Change subject: Force inlining of BloomFilter::MakeMask
......................................................................
Patch Set 3:
I did set them - the benchmark complains otherwise. It is a bit strange, I agree.
tarmstrong@tarmstrong-box2:~/Impala/incubator-impala$ cat /sys/devices/system/cpu/intel_pstate/no_turbo
1
tarmstrong@tarmstrong-box2:~/Impala/incubator-impala$ cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_governor
performance
--
To view, visit http://gerrit.cloudera.org:8080/9214
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I89282f6c315570bea5ad8a0f854cb6eea0592923
Gerrit-Change-Number: 9214
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Mostafa Mokhtar <mm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Feb 2018 20:17:50 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] Force inlining of BloomFilter::MakeMask
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9214 )
Change subject: Force inlining of BloomFilter::MakeMask
......................................................................
Patch Set 2:
The speedup was pretty significant on the benchmark.
On my system before:
With AVX2:
insert: Function iters/ms 10%ile 50%ile 90%ile 10%ile 50%ile 90%ile
(relative) (relative) (relative)
---------------------------------------------------------------------------------------------------------
ndv 10k fpp 10.0% 1.06e+05 1.06e+05 1.07e+05 1X 1X 1X
ndv 10k fpp 1.0% 1.05e+05 1.06e+05 1.07e+05 0.998X 0.999X 0.998X
ndv 10k fpp 0.1% 1.06e+05 1.06e+05 1.07e+05 0.999X 1X 1X
ndv 1000k fpp 10.0% 1.05e+05 1.06e+05 1.07e+05 0.993X 0.992X 0.994X
ndv 1000k fpp 1.0% 1.04e+05 1.05e+05 1.06e+05 0.986X 0.988X 0.986X
ndv 1000k fpp 0.1% 1.04e+05 1.05e+05 1.06e+05 0.986X 0.987X 0.986X
ndv 100000k fpp 10.0% 3.84e+04 4.04e+04 4.4e+04 0.364X 0.38X 0.41X
ndv 100000k fpp 1.0% 3.9e+04 4.11e+04 4.46e+04 0.369X 0.386X 0.416X
ndv 100000k fpp 0.1% 3.77e+04 3.85e+04 4.58e+04 0.357X 0.362X 0.426X
find: Function iters/ms 10%ile 50%ile 90%ile 10%ile 50%ile 90%ile
(relative) (relative) (relative)
---------------------------------------------------------------------------------------------------------
present ndv 10k fpp 10.0% 1.12e+05 1.12e+05 1.13e+05 1X 1X 1X
absent ndv 10k fpp 10.0% 1.11e+05 1.12e+05 1.13e+05 0.998X 0.998X 1X
present ndv 10k fpp 1.0% 1.12e+05 1.12e+05 1.13e+05 1X 0.998X 0.999X
absent ndv 10k fpp 1.0% 1.12e+05 1.12e+05 1.13e+05 0.999X 1X 0.998X
present ndv 10k fpp 0.1% 1.12e+05 1.13e+05 1.13e+05 1X 1X 1X
absent ndv 10k fpp 0.1% 1.12e+05 1.12e+05 1.13e+05 0.999X 0.999X 0.998X
present ndv 1000k fpp 10.0% 1.08e+05 1.09e+05 1.1e+05 0.967X 0.969X 0.967X
absent ndv 1000k fpp 10.0% 1.09e+05 1.09e+05 1.1e+05 0.973X 0.97X 0.973X
present ndv 1000k fpp 1.0% 1.07e+05 1.08e+05 1.09e+05 0.96X 0.96X 0.961X
absent ndv 1000k fpp 1.0% 1.08e+05 1.09e+05 1.1e+05 0.971X 0.971X 0.969X
present ndv 1000k fpp 0.1% 1.05e+05 1.05e+05 1.06e+05 0.937X 0.938X 0.937X
absent ndv 1000k fpp 0.1% 1.06e+05 1.07e+05 1.08e+05 0.951X 0.949X 0.95X
present ndv 100000k fpp 10.0% 3.97e+04 4.08e+04 4.63e+04 0.356X 0.363X 0.408X
absent ndv 100000k fpp 10.0% 3.94e+04 4.08e+04 4.23e+04 0.353X 0.363X 0.374X
present ndv 100000k fpp 1.0% 4.19e+04 5.11e+04 7.02e+04 0.375X 0.454X 0.62X
absent ndv 100000k fpp 1.0% 3.9e+04 4.09e+04 4.28e+04 0.35X 0.364X 0.377X
present ndv 100000k fpp 0.1% 3.94e+04 4.26e+04 6.5e+04 0.353X 0.379X 0.573X
absent ndv 100000k fpp 0.1% 3.67e+04 3.71e+04 3.82e+04 0.329X 0.33X 0.337X
union: Function iters/ms 10%ile 50%ile 90%ile 10%ile 50%ile 90%ile
(relative) (relative) (relative)
---------------------------------------------------------------------------------------------------------
ndv 10k fpp 10.0% 5.46e+05 5.51e+05 5.56e+05 1X 1X 1X
ndv 10k fpp 1.0% 5.46e+05 5.49e+05 5.54e+05 1X 0.996X 0.996X
ndv 10k fpp 0.1% 5.46e+05 5.51e+05 5.56e+05 1X 1X 1X
ndv 1000k fpp 10.0% 5.47e+05 5.51e+05 5.55e+05 1X 1X 0.998X
ndv 1000k fpp 1.0% 5.48e+05 5.54e+05 5.65e+05 1X 1.01X 1.02X
ndv 1000k fpp 0.1% 5.48e+05 5.51e+05 5.55e+05 1X 1X 0.998X
ndv 100000k fpp 10.0% 5.48e+05 5.54e+05 5.65e+05 1X 1.01X 1.02X
ndv 100000k fpp 1.0% 5.43e+05 5.51e+05 5.57e+05 0.994X 1X 1X
ndv 100000k fpp 0.1% 5.47e+05 5.52e+05 5.57e+05 1X 1X 1X
--
To view, visit http://gerrit.cloudera.org:8080/9214
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I89282f6c315570bea5ad8a0f854cb6eea0592923
Gerrit-Change-Number: 9214
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Mostafa Mokhtar <mm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Feb 2018 20:05:43 +0000
Gerrit-HasComments: No