Skip to content

GH-35729: [C++][Parquet] Implement batch interface for BloomFilter in Parquet - #35731

Merged
pitrou merged 17 commits into
apache:mainfrom
mapleFU:parquet/enhance-bloom-filter
May 30, 2023
Merged

GH-35729: [C++][Parquet] Implement batch interface for BloomFilter in Parquet#35731
pitrou merged 17 commits into
apache:mainfrom
mapleFU:parquet/enhance-bloom-filter

Conversation

@mapleFU

@mapleFUmapleFU commented May 24, 2023

Copy link
Copy Markdown
Member

Rationale for this change

For optimizing #35691 . We need batch execution for BloomFilter

What changes are included in this PR?

@mapleFU
mapleFU requested a review from wjones127 as a code ownerMay 24, 2023 03:28
@github-actions

Copy link
Copy Markdown

@github-actions

Copy link
Copy Markdown

⚠️ GitHub issue #35729has been automatically assigned in GitHub to PR creator.

@mapleFU

Copy link
Copy Markdown
MemberAuthor

cc @pitrou@wgtmac

Comment threadcpp/src/parquet/hasher.h Outdated
Comment threadcpp/src/parquet/bloom_filter_test.cc Outdated
Comment threadcpp/src/parquet/bloom_filter.h Outdated
Comment threadcpp/src/parquet/bloom_filter.h Outdated
Comment threadcpp/src/parquet/bloom_filter.h Outdated
@github-actionsgithub-actionsBot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels May 24, 2023
@mapleFU

Copy link
Copy Markdown
MemberAuthor

@wgtmac test added and some comment fixed

@mapleFU

mapleFU commented May 24, 2023

Copy link
Copy Markdown
MemberAuthor

I've no idea why windows CI failed on TYPED_TEST...
@kou Would you mind take a look? I think my testing is same as previous tests, but it report compile failed in https://ci.appveyor.com/project/ApacheSoftwareFoundation/arrow/builds/47126705 . I've commit several patch but didn't find out why...

Comment threadcpp/src/parquet/bloom_filter.h Outdated
Comment threadcpp/src/parquet/bloom_filter_test.cc Outdated
Comment threadcpp/src/parquet/bloom_filter_test.cc Outdated
Comment threadcpp/src/parquet/xxhasher.cc Outdated
Comment threadcpp/src/parquet/hasher.h Outdated
Comment threadcpp/src/parquet/hasher.h Outdated
Comment threadcpp/src/parquet/bloom_filter_test.cc Outdated
Comment threadcpp/src/parquet/bloom_filter_test.cc Outdated
@pitrou

Copy link
Copy Markdown
Member

At some point it might also be nice to add some basic bloom filter benchmarks.

@mapleFU

Copy link
Copy Markdown
MemberAuthor

Sure, I'll add benchmark for them!

@mapleFU

Copy link
Copy Markdown
MemberAuthor

@pitrou I got a wired problem, should I use BloomFilter or BlockSplitBloomFilter in benchmark?

@mapleFU

Copy link
Copy Markdown
MemberAuthor

@pitrou I've add the tests, I extract GenerateData from encoding_test.cc and use it to generate all kinds of data. Mind take a look?

@mapleFU

Copy link
Copy Markdown
MemberAuthor

On my MacOS with Release(O2):

----------------------------------------------------------------------------------------
Benchmark Time CPU Iterations UserCounters...
----------------------------------------------------------------------------------------
BM_CountHash<Int32Type> 7318 ns 7309 ns 88867 items_per_second=140.096M/s
BM_CountHash<Int64Type> 6120 ns 6099 ns 112450 items_per_second=167.886M/s
BM_CountHash<FloatType> 6574 ns 6507 ns 104999 items_per_second=157.375M/s
BM_CountHash<DoubleType> 6732 ns 6726 ns 103284 items_per_second=152.254M/s
BM_BatchCountHash<Int32Type> 2854 ns 2850 ns 245992 items_per_second=359.34M/s
BM_BatchCountHash<Int64Type> 2640 ns 2639 ns 265086 items_per_second=388.007M/s
BM_BatchCountHash<FloatType> 3151 ns 3149 ns 226064 items_per_second=325.222M/s
BM_BatchCountHash<DoubleType> 2993 ns 2987 ns 235524 items_per_second=342.78M/s
BM_InsertHash 22694 ns 22676 ns 31097 items_per_second=45.1588M/s
BM_InsertHash 23941 ns 23438 ns 31044 items_per_second=43.6899M/s
BM_BatchInsertHash 23246 ns 23079 ns 30079 items_per_second=44.3696M/s
BM_BatchInsertHash 23125 ns 23057 ns 30729 items_per_second=44.4115M/s
BM_FindHash 17945 ns 17734 ns 39837 items_per_second=57.7415M/s
BM_FindHash 17298 ns 17284 ns 39557 items_per_second=59.2463M/s

@mapleFU
mapleFUforce-pushed the parquet/enhance-bloom-filter branch from 0f60930 to 13fdeb2CompareMay 24, 2023 12:11
@pitrou

Copy link
Copy Markdown
Member

You should benchmark BloomFilter as that's the public interface that will be called.

@mapleFU

Copy link
Copy Markdown
MemberAuthor

I've change the implemention to benchmark BloomFilter* now

@pitrou

Copy link
Copy Markdown
Member

@mapleFU No problem, and thanks for doing this :-)

@mapleFU

Copy link
Copy Markdown
MemberAuthor

Using 1mib as a insert batch is really too large:

BM_BatchComputeHash<ByteArrayType> 85150536 ns 85024375 ns 8 items_per_second=12.3327M/s
BM_BatchComputeHash<FLBAType> 78013056 ns 77946556 ns 9 items_per_second=13.4525M/s
BM_BatchComputeHash<Int96Type> 95809131 ns 95748143 ns 7 items_per_second=10.9514M/s
BM_InsertHash 910213833 ns 904038000 ns 1 items_per_second=1.15988M/s
BM_BatchInsertHash 870639958 ns 866905000 ns 1 items_per_second=1.20956M/s

@mapleFU

mapleFU commented May 26, 2023

Copy link
Copy Markdown
MemberAuthor

I use 16K as batch size now, here are benchmark under MacOS, Release(O2):

BM_ComputeHash<Int32Type> 131101 ns 130977 ns 5358 items_per_second=125.091M/s
BM_ComputeHash<Int64Type> 127365 ns 127252 ns 5491 items_per_second=128.752M/s
BM_ComputeHash<FloatType> 135583 ns 135515 ns 5161 items_per_second=120.902M/s
BM_ComputeHash<DoubleType> 129931 ns 129668 ns 5383 items_per_second=126.353M/s
BM_ComputeHash<ByteArrayType> 141672 ns 141621 ns 4942 items_per_second=115.689M/s
BM_ComputeHash<FLBAType> 135274 ns 135263 ns 5175 items_per_second=121.127M/s
BM_ComputeHash<Int96Type> 114990 ns 114899 ns 6075 items_per_second=142.595M/s
BM_BatchComputeHash<Int32Type> 53836 ns 52917 ns 13367 items_per_second=309.616M/s
BM_BatchComputeHash<Int64Type> 49348 ns 49299 ns 14201 items_per_second=332.341M/s
BM_BatchComputeHash<FloatType> 58428 ns 58419 ns 11910 items_per_second=280.458M/s
BM_BatchComputeHash<DoubleType> 55413 ns 55363 ns 12648 items_per_second=295.936M/s
BM_BatchComputeHash<ByteArrayType> 110692 ns 110690 ns 6297 items_per_second=148.017M/s
BM_BatchComputeHash<FLBAType> 105711 ns 105709 ns 6630 items_per_second=154.991M/s
BM_BatchComputeHash<Int96Type> 73298 ns 73292 ns 9562 items_per_second=223.544M/s
BM_InsertHash 431891 ns 431808 ns 1581 items_per_second=37.9428M/s
BM_BatchInsertHash 423573 ns 422990 ns 1658 items_per_second=38.7337M/s
BM_FindExistsHash 450268 ns 443190 ns 1592 items_per_second=36.9683M/s
BM_FindNotExistsHash 556775 ns 510555 ns 1000 items_per_second=32.0906M/s

@pitrou

@mapleFU
mapleFU requested a review from pitrouMay 30, 2023 10:43
@pitrou
pitrouforce-pushed the parquet/enhance-bloom-filter branch from 806edff to 46f542bCompareMay 30, 2023 16:26
@pitrou

pitrou commented May 30, 2023

Copy link
Copy Markdown
Member

Thanks @mapleFU . I've pushed some changes to improve some benchmarks and also to improve BloomFilter::Find performance (around 2x faster).

Current benchmarks here:

---------------------------------------------------------------------------------------------
Benchmark Time CPU Iterations UserCounters...
---------------------------------------------------------------------------------------------
BM_ComputeHash<Int32Type> 715521 ns 715427 ns 951 items_per_second=22.901M/s
BM_ComputeHash<Int64Type> 740955 ns 740872 ns 878 items_per_second=22.1145M/s
BM_ComputeHash<FloatType> 720903 ns 720789 ns 946 items_per_second=22.7306M/s
BM_ComputeHash<DoubleType> 691569 ns 691479 ns 1005 items_per_second=23.6941M/s
BM_ComputeHash<ByteArrayType> 706878 ns 706770 ns 986 items_per_second=23.1815M/s
BM_ComputeHash<FLBAType> 762304 ns 762178 ns 941 items_per_second=21.4963M/s
BM_ComputeHash<Int96Type> 892253 ns 892111 ns 809 items_per_second=18.3654M/s
BM_BatchComputeHash<Int32Type> 250193 ns 250154 ns 2858 items_per_second=65.4957M/s
BM_BatchComputeHash<Int64Type> 277338 ns 277289 ns 2518 items_per_second=59.0863M/s
BM_BatchComputeHash<FloatType> 247188 ns 247143 ns 2832 items_per_second=66.2936M/s
BM_BatchComputeHash<DoubleType> 278888 ns 278850 ns 2517 items_per_second=58.7557M/s
BM_BatchComputeHash<ByteArrayType> 256035 ns 255996 ns 2740 items_per_second=64.0009M/s
BM_BatchComputeHash<FLBAType> 252759 ns 252714 ns 2783 items_per_second=64.8323M/s
BM_BatchComputeHash<Int96Type> 328592 ns 328540 ns 2119 items_per_second=49.8692M/s
BM_InsertHash 689503 ns 689394 ns 1010 items_per_second=23.7658M/s
BM_BatchInsertHash 496698 ns 496568 ns 1482 items_per_second=32.9945M/s
BM_FindExistingHash 622037 ns 621724 ns 1126 items_per_second=26.3525M/s
BM_FindNonExistingHash 658163 ns 657842 ns 1068 items_per_second=24.9057M/s

@mapleFU

Copy link
Copy Markdown
MemberAuthor

Nice catch!

Personally I guess this bloom filter would not be the bottleneck, I guess read whole bloom filter would be much more expensive, but improve the performance would be great!

@pitrou

Copy link
Copy Markdown
Member

CI failures are unrelated, I'm merging.

@pitrou
pitrou merged commit 2695293 into apache:mainMay 30, 2023
@mapleFU
mapleFU deleted the parquet/enhance-bloom-filter branch May 31, 2023 04:26
@ursabot

Copy link
Copy Markdown

Benchmark runs are scheduled for baseline = 9dc63c2 and contender = 2695293. 2695293 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Failed ⬇️1.39% ⬆️0.08%] test-mac-arm
[Finished ⬇️1.62% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️1.26% ⬆️0.54%] ursa-thinkcentre-m75q
Buildkite builds:
[Finished] 26952930 ec2-t3-xlarge-us-east-2
[Failed] 26952930 test-mac-arm
[Finished] 26952930 ursa-i9-9960x
[Finished] 26952930 ursa-thinkcentre-m75q
[Finished] 9dc63c24 ec2-t3-xlarge-us-east-2
[Failed] 9dc63c24 test-mac-arm
[Finished] 9dc63c24 ursa-i9-9960x
[Finished] 9dc63c24 ursa-thinkcentre-m75q
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

@ursabot

Copy link
Copy Markdown

['Python', 'R'] benchmarks have high level of regressions.
ursa-i9-9960x

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[C++][Parquet] Support Batch Interface in BloomFilter

4 participants

@mapleFU@pitrou@ursabot@wgtmac