GH-40872: [C++][Parquet] Encoding: Optimize DecodeArrow/Decode(bitmap) for PlainBooleanDecoder by mapleFU · Pull Request #40876 · apache/arrow

mapleFU · 2024-03-28T15:58:23Z

Rationale for this change

This is for enhance boolean decoding. I optimized the DecodeArrow for PlainBoolean

What changes are included in this PR?

Optimize DecodeArrow/Decode(bitmap) for PlainBooleanDecoder, and add benchmarks

Are these changes tested?

Yes

Are there any user-facing changes?

Minor optimization. And Decode boolean will change the syntax

GitHub Issue: [C++][Parquet] Encoding: Optimize DecodeArrow for PlainBooleanDecoder #40872

github-actions · 2024-03-28T15:58:50Z

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

mapleFU · 2024-03-29T02:08:12Z

cpp/src/parquet/encoding.cc

+  } else {
+    int64_t previous_offset = 0;
+    int64_t previous_value_offset = 0;
+    PARQUET_THROW_NOT_OK(::arrow::internal::VisitSetBitRuns(


If the data is interleaving "null, non-null, null, non-null, ...". Don't know that would performance be

I guess the best performance might getting a BitRun scanner here

We can start with this. Are there benchmarks for this case?

I've pasted this in the pr, PTAL

The benchmarks in this PR always have null_probability = 0, right?

Yes. Aha previously all null_probability is 0...

cpp/src/parquet/encoding.cc

mapleFU · 2024-03-29T02:56:44Z

Before:

BM_PlainDecodingBoolean/1024                 249 ns          249 ns      2809214 bytes_per_second=3.82556G/s
BM_PlainDecodingBoolean/4096                 600 ns          600 ns      1164784 bytes_per_second=6.36244G/s
BM_PlainDecodingBoolean/32768               3858 ns         3853 ns       181202 bytes_per_second=7.91948G/s
BM_PlainDecodingBoolean/65536               7524 ns         7517 ns        93990 bytes_per_second=8.11909G/s
BM_PlainDecodingBooleanToBitmap/1024        7101 ns         7095 ns        97939 bytes_per_second=137.642M/s
BM_PlainDecodingBooleanToBitmap/4096       27520 ns        27510 ns        25488 bytes_per_second=141.994M/s
BM_PlainDecodingBooleanToBitmap/32768     219035 ns       218869 ns         3197 bytes_per_second=142.779M/s
BM_PlainDecodingBooleanToBitmap/65536     439026 ns       438836 ns         1598 bytes_per_second=142.422M/s

After:

BM_PlainDecodingBoolean/1024                 248 ns          248 ns      2820193 bytes_per_second=3.84033G/s
BM_PlainDecodingBoolean/4096                 610 ns          604 ns      1168361 bytes_per_second=6.31453G/s
BM_PlainDecodingBoolean/32768               3817 ns         3815 ns       181765 bytes_per_second=8.00006G/s
BM_PlainDecodingBoolean/65536               7483 ns         7478 ns        93656 bytes_per_second=8.16155G/s
BM_PlainDecodingBooleanToBitmap/1024         133 ns          133 ns      5271521 bytes_per_second=7.1644G/s
BM_PlainDecodingBooleanToBitmap/4096         139 ns          139 ns      5047009 bytes_per_second=27.3761G/s
BM_PlainDecodingBooleanToBitmap/32768        192 ns          192 ns      3691555 bytes_per_second=159.17G/s
BM_PlainDecodingBooleanToBitmap/65536        253 ns          253 ns      2845413 bytes_per_second=241.105G/s

mapleFU · 2024-03-29T03:10:14Z

cpp/src/parquet/encoding.cc

@@ -1214,18 +1234,15 @@ inline int PlainBooleanDecoder::DecodeArrow(

 int PlainBooleanDecoder::Decode(uint8_t* buffer, int max_values) {


When refering to #38355 . Previously we won't crack the trailing bit, now we'll . Should we keep the same syntax? @pitrou @wgtmac

Yes, I think it's ok.

mapleFU · 2024-04-03T11:10:32Z

After:

BM_DecodeArrowBooleanPlain/DecodeArrowWithNull_Dense/num-values:1024/null-prob:10        3631 ns         3612 ns       195883 bytes_per_second=33.7927M/s
BM_DecodeArrowBooleanPlain/DecodeArrowWithNull_Dense/num-values:2048/null-prob:10        6854 ns         6724 ns       104659 bytes_per_second=36.3087M/s
BM_DecodeArrowBooleanPlain/DecodeArrowWithNull_Dense/num-values:4096/null-prob:10       13050 ns        12957 ns        54526 bytes_per_second=37.6833M/s
BM_DecodeArrowBooleanPlain/DecodeArrowWithNull_Dense/num-values:8192/null-prob:10       26314 ns        25650 ns        26516 bytes_per_second=38.0727M/s
BM_DecodeArrowBooleanPlain/DecodeArrowWithNull_Dense/num-values:16384/null-prob:10      51909 ns        51337 ns        13720 bytes_per_second=38.0451M/s
BM_DecodeArrowBooleanPlain/DecodeArrowWithNull_Dense/num-values:32768/null-prob:10     110306 ns       107744 ns         6624 bytes_per_second=36.2549M/s
BM_DecodeArrowBooleanPlain/DecodeArrowWithNull_Dense/num-values:65536/null-prob:10     221000 ns       220405 ns         3107 bytes_per_second=35.4461M/s
BM_DecodeArrowBooleanPlain/DecodeArrowWithNull_Dense/num-values:1024/null-prob:50        5016 ns         4974 ns       141186 bytes_per_second=24.5432M/s
BM_DecodeArrowBooleanPlain/DecodeArrowWithNull_Dense/num-values:2048/null-prob:50        9773 ns         9529 ns        72019 bytes_per_second=25.6217M/s
BM_DecodeArrowBooleanPlain/DecodeArrowWithNull_Dense/num-values:4096/null-prob:50       21108 ns        18906 ns        37395 bytes_per_second=25.8263M/s
BM_DecodeArrowBooleanPlain/DecodeArrowWithNull_Dense/num-values:8192/null-prob:50       39810 ns        38057 ns        18095 bytes_per_second=25.6605M/s
BM_DecodeArrowBooleanPlain/DecodeArrowWithNull_Dense/num-values:16384/null-prob:50     101819 ns        91886 ns         7236 bytes_per_second=21.256M/s
BM_DecodeArrowBooleanPlain/DecodeArrowWithNull_Dense/num-values:32768/null-prob:50     209925 ns       201723 ns         3433 bytes_per_second=19.3644M/s
BM_DecodeArrowBooleanPlain/DecodeArrowWithNull_Dense/num-values:65536/null-prob:50     440182 ns       431220 ns         1641 bytes_per_second=18.1172M/s

Before:

BM_DecodeArrowBooleanPlain/DecodeArrowWithNull_Dense/num-values:1024/null-prob:10        8560 ns         8509 ns        81931 bytes_per_second=14.3458M/s
BM_DecodeArrowBooleanPlain/DecodeArrowWithNull_Dense/num-values:2048/null-prob:10       16993 ns        16875 ns        41546 bytes_per_second=14.4676M/s
BM_DecodeArrowBooleanPlain/DecodeArrowWithNull_Dense/num-values:4096/null-prob:10       33400 ns        33270 ns        20855 bytes_per_second=14.6761M/s
BM_DecodeArrowBooleanPlain/DecodeArrowWithNull_Dense/num-values:8192/null-prob:10       68693 ns        67399 ns        10604 bytes_per_second=14.4893M/s
BM_DecodeArrowBooleanPlain/DecodeArrowWithNull_Dense/num-values:16384/null-prob:10     136645 ns       134282 ns         5094 bytes_per_second=14.545M/s
BM_DecodeArrowBooleanPlain/DecodeArrowWithNull_Dense/num-values:32768/null-prob:10     267212 ns       264922 ns         2656 bytes_per_second=14.7449M/s
BM_DecodeArrowBooleanPlain/DecodeArrowWithNull_Dense/num-values:65536/null-prob:10     531085 ns       528560 ns         1307 bytes_per_second=14.7807M/s
BM_DecodeArrowBooleanPlain/DecodeArrowWithNull_Dense/num-values:1024/null-prob:50        6681 ns         6655 ns       104864 bytes_per_second=18.3438M/s
BM_DecodeArrowBooleanPlain/DecodeArrowWithNull_Dense/num-values:2048/null-prob:50       13626 ns        13570 ns        51543 bytes_per_second=17.9908M/s
BM_DecodeArrowBooleanPlain/DecodeArrowWithNull_Dense/num-values:4096/null-prob:50       30337 ns        30249 ns        22973 bytes_per_second=16.1423M/s
BM_DecodeArrowBooleanPlain/DecodeArrowWithNull_Dense/num-values:8192/null-prob:50       69539 ns        68845 ns        10315 bytes_per_second=14.1848M/s
BM_DecodeArrowBooleanPlain/DecodeArrowWithNull_Dense/num-values:16384/null-prob:50     147163 ns       146116 ns         4624 bytes_per_second=13.3669M/s
BM_DecodeArrowBooleanPlain/DecodeArrowWithNull_Dense/num-values:32768/null-prob:50     299173 ns       298553 ns         2320 bytes_per_second=13.084M/s
BM_DecodeArrowBooleanPlain/DecodeArrowWithNull_Dense/num-values:65536/null-prob:50     615457 ns       613450 ns         1141 bytes_per_second=12.7353M/s

cpp/src/parquet/encoding.cc

cpp/src/parquet/encoding_benchmark.cc

pitrou · 2024-04-03T12:23:06Z

cpp/src/parquet/encoding_benchmark.cc

+  }
+};
+
+class BM_DecodeArrowBooleanRle : public BenchmarkDecodeArrowBoolean {


You don't need this, you just need some member variable with Encoding::PLAIN or Encoding::RLE, right?

done yet, PTAL again

cpp/src/parquet/encoding_benchmark.cc

conbench-apache-arrow · 2024-04-04T05:46:25Z

After merging your PR, Conbench analyzed the 7 benchmarking runs that have been run so far on merge-commit a4acb64.

There was 1 benchmark result with an error:

Commit Run on ursa-i9-9960x at 2024-04-03 20:57:29Z
- tpch (R) with engine=arrow, format=parquet, language=R, memory_map=False, query_id=TPCH-16, scale_factor=1

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 7 possible false positives for unstable benchmarks that are known to sometimes produce them.

…oding (#41037) ### Rationale for this change Previous patches ( #40876 , #40995 ) introduce some bugs in boolean decoding. This patch add tests for DecodeArrow and fix the bug ### What changes are included in this PR? 1. Add tests for DecodeArrow with nulls and without nulls 2. Fix 2 bugs ### Are these changes tested? Yes ### Are there any user-facing changes? no * GitHub Issue: #41032 Lead-authored-by: mwish <maplewish117@gmail.com> Co-authored-by: mwish <anmmscs_maple@qq.com> Signed-off-by: Antoine Pitrou <antoine@python.org>

…bitmap) for PlainBooleanDecoder (apache#40876) ### Rationale for this change This is for enhance boolean decoding. I optimized the `DecodeArrow` for PlainBoolean ### What changes are included in this PR? Optimize DecodeArrow/Decode(bitmap) for PlainBooleanDecoder, and add benchmarks ### Are these changes tested? Yes ### Are there any user-facing changes? Minor optimization. And `Decode` boolean will change the syntax * GitHub Issue: apache#40872 Lead-authored-by: mwish <maplewish117@gmail.com> Co-authored-by: Antoine Pitrou <antoine@python.org> Signed-off-by: mwish <maplewish117@gmail.com>

…ow decoding (apache#41037) ### Rationale for this change Previous patches ( apache#40876 , apache#40995 ) introduce some bugs in boolean decoding. This patch add tests for DecodeArrow and fix the bug ### What changes are included in this PR? 1. Add tests for DecodeArrow with nulls and without nulls 2. Fix 2 bugs ### Are these changes tested? Yes ### Are there any user-facing changes? no * GitHub Issue: apache#41032 Lead-authored-by: mwish <maplewish117@gmail.com> Co-authored-by: mwish <anmmscs_maple@qq.com> Signed-off-by: Antoine Pitrou <antoine@python.org>

github-actions bot added Component: Parquet Component: C++ awaiting review Awaiting review labels Mar 28, 2024

mapleFU force-pushed the plain-boolean-decoding-arrow branch from 4099c1b to 962dcfa Compare March 28, 2024 15:59

Encoding: Optimize DecodeArrow for PlainBooleanDecoder

b03728b

mapleFU force-pushed the plain-boolean-decoding-arrow branch from 962dcfa to b03728b Compare March 28, 2024 16:22

mapleFU changed the title ~~GH-40872: [C++][Parquet] Encoding: Optimize DecodeArrow for PlainBooleanDecoder~~ GH-40872: [C++][Parquet] Encoding: Optimize DecodeArrow/Decode(bitmap) for PlainBooleanDecoder Mar 28, 2024

mapleFU commented Mar 29, 2024

View reviewed changes

github-actions bot added awaiting review Awaiting review awaiting committer review Awaiting committer review and removed awaiting review Awaiting review awaiting committer review Awaiting committer review labels Mar 29, 2024

mapleFU commented Mar 29, 2024

View reviewed changes

github-actions bot added Component: R Component: Java Component: Go Component: C# Component: Documentation labels Mar 29, 2024

Add benchmark for Plain Decoding

47d6a1b

mapleFU force-pushed the plain-boolean-decoding-arrow branch from f8eaf1f to 47d6a1b Compare March 29, 2024 03:24

mapleFU removed Component: R Component: Java Component: Go Component: C# Component: Documentation labels Mar 29, 2024

Add DecodeArrow for boolean

32eea9f

fix lint and add null_prob benchmark

edf2272

mapleFU requested a review from pitrou April 3, 2024 11:11

fix lint

ca862a3

pitrou reviewed Apr 3, 2024

View reviewed changes

mapleFU and others added 2 commits April 3, 2024 22:18

resolve comment

3d7f250

Add SetItemsProcessed calls

6eb007d

mapleFU requested a review from pitrou April 3, 2024 16:34

pitrou approved these changes Apr 3, 2024

View reviewed changes

pitrou mentioned this pull request Apr 3, 2024

[C++] supports UnsafeAppend{Values|Nulls} in ::arrow::BooleanBuilder #40978

Closed

mapleFU merged commit a4acb64 into apache:main Apr 3, 2024

mapleFU removed the awaiting committer review Awaiting committer review label Apr 3, 2024

mapleFU mentioned this pull request Apr 3, 2024

[C++][Parquet] Encoding: Optimize DecodeArrow for PlainBooleanDecoder #40872

Closed

mapleFU deleted the plain-boolean-decoding-arrow branch April 4, 2024 04:02

pitrou mentioned this pull request Apr 5, 2024

[C++] Regression in PlainBooleanDecoder::DecodeArrow #41032

Closed

mapleFU mentioned this pull request Apr 5, 2024

GH-41032: [C++][Parquet] Bugfixes and more tests in boolean arrow decoding #41037

Merged

		@@ -1214,18 +1234,15 @@ inline int PlainBooleanDecoder::DecodeArrow(

		int PlainBooleanDecoder::Decode(uint8_t* buffer, int max_values) {

Conversation

mapleFU commented Mar 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

github-actions bot commented Mar 28, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mapleFU commented Mar 29, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mapleFU commented Apr 3, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

conbench-apache-arrow bot commented Apr 4, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mapleFU commented Mar 28, 2024 •

edited

Loading