Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Replace Cow<str> in TableReference to Arc<str> #9764

Closed
jayzhan211 opened this issue Mar 24, 2024 · 16 comments · Fixed by #9824
Closed

Replace Cow<str> in TableReference to Arc<str> #9764

jayzhan211 opened this issue Mar 24, 2024 · 16 comments · Fixed by #9824

Comments

@jayzhan211
Copy link
Contributor 

          Maybe we need some way to make the table reference `Arc<str>` instead of a string 🤔  -- maybe that can be a follow on PR (rather than trying to use the borrow checker)

Originally posted by @alamb in #9595 (comment)

A benchmark for this is required.
@alamb
Copy link
Contributor

alamb commented Mar 24, 2024 •
edited
Loading

Before we make a PR to do this I think it would be worth creating a POC / demonstrate that it would actually improve performance

The natural way to test would be

cargo bench --bench sql_planner

@comphead
Copy link
Contributor

if no one already trying that, I'll check it

@comphead
Copy link
Contributor

Other thing that comes to my mind, is we prob want to use only 1 String type in DF.
Currently we use String, &str, Cow<str>
Otherwise I think we can lose some CPU on conversions.

@jayzhan211
Copy link
Contributor Author

I think the use cases of them depends, not always one beats others.

@alamb
Copy link
Contributor

alamb commented Mar 26, 2024 •
edited
Loading

I suggest Arc for anything that will be copied around and String for anything that needs to be mutable

@jayzhan211
Copy link
Contributor Author

jayzhan211 commented Mar 26, 2024 •
edited
Loading

Consider the atomic operation cost, I doubt that Arc<T> always win. That is why I think benchmark is required.

https://users.rust-lang.org/t/arc-str-is-better-than-string-was-i-using-the-wrong-string-type/99789/4

Small-string-optimized strings win if you have lots of tiny strings which you copy around often and Arc wins if your strings are long while String in neither here not there and makes you code a bit smaller, instead

@comphead
Copy link
Contributor

I'm still on it folks, it requires to go through all the analyzer.

@alamb
Copy link
Contributor

alamb commented Mar 27, 2024

note that #9595 makes substantial changes to the code, so unless you build off that PR you'll likely end up with many many conflicts

@comphead comphead mentioned this issue Mar 27, 2024
Merged
@comphead
Copy link
Contributor

Thanks for letting me know, I made a draft #9824 to play a little but with benches

@comphead
Copy link
Contributor

comphead commented Mar 29, 2024 •
edited
Loading

with Arc<str>

logical_select_one_from_700
                        time:   [580.12 µs 585.06 µs 589.73 µs]
                        change: [-6.3590% -5.7328% -5.0564%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high mild

physical_select_one_from_700
                        time:   [2.3826 ms 2.3916 ms 2.4006 ms]
                        change: [-7.0877% -6.6384% -6.1781%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
  3 (3.00%) high mild

Benchmarking logical_select_all_from_1000: Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 7.2s, or reduce sample count to 60.
logical_select_all_from_1000
                        time:   [70.414 ms 70.720 ms 71.037 ms]
                        change: [+3.3675% +4.0240% +4.6863%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high mild

Benchmarking physical_select_all_from_1000: Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 50.0s, or reduce sample count to 10.
physical_select_all_from_1000
                        time:   [506.06 ms 507.48 ms 508.95 ms]
                        change: [+2.5742% +2.9462% +3.3299%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 4 outliers among 100 measurements (4.00%)
  4 (4.00%) high mild

logical_trivial_join_low_numbered_columns
                        time:   [587.37 µs 592.07 µs 595.97 µs]
                        change: [-7.8608% -6.8978% -6.0233%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 13 outliers among 100 measurements (13.00%)
  7 (7.00%) low severe
  2 (2.00%) low mild
  2 (2.00%) high mild
  2 (2.00%) high severe

logical_trivial_join_high_numbered_columns
                        time:   [630.89 µs 632.24 µs 633.53 µs]
                        change: [-5.9321% -5.4946% -5.0383%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 9 outliers among 100 measurements (9.00%)
  1 (1.00%) low severe
  4 (4.00%) low mild
  4 (4.00%) high mild

logical_aggregate_with_join
                        time:   [823.32 µs 827.32 µs 830.69 µs]
                        change: [-8.0075% -7.5928% -7.1907%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
  1 (1.00%) low severe
  2 (2.00%) low mild

physical_plan_tpch_q1   time:   [3.7308 ms 3.7435 ms 3.7561 ms]
                        change: [-2.6559% -2.1877% -1.7373%] (p = 0.00 < 0.05)
                        Performance has improved.

physical_plan_tpch_q2   time:   [5.7711 ms 5.7942 ms 5.8175 ms]
                        change: [-7.6441% -7.1368% -6.6634%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 2 outliers among 100 measurements (2.00%)
  1 (1.00%) low mild
  1 (1.00%) high mild

Benchmarking physical_plan_tpch_q3: Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 10.0s, enable flat sampling, or reduce sample count to 40.
physical_plan_tpch_q3   time:   [1.9749 ms 1.9794 ms 1.9844 ms]
                        change: [-7.4539% -6.9879% -6.5380%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 7 outliers among 100 measurements (7.00%)
  1 (1.00%) low severe
  1 (1.00%) low mild
  4 (4.00%) high mild
  1 (1.00%) high severe

Benchmarking physical_plan_tpch_q4: Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 8.3s, enable flat sampling, or reduce sample count to 50.
physical_plan_tpch_q4   time:   [1.6496 ms 1.6537 ms 1.6581 ms]
                        change: [-6.7606% -6.0924% -5.4526%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
  1 (1.00%) low mild
  2 (2.00%) high mild

physical_plan_tpch_q5   time:   [2.8363 ms 2.8451 ms 2.8541 ms]
                        change: [-8.7362% -8.3231% -7.8946%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 6 outliers among 100 measurements (6.00%)
  1 (1.00%) low mild
  5 (5.00%) high mild

Benchmarking physical_plan_tpch_q6: Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 5.7s, enable flat sampling, or reduce sample count to 60.
physical_plan_tpch_q6   time:   [1.1385 ms 1.1419 ms 1.1455 ms]
                        change: [-4.6024% -3.9168% -3.1923%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 7 outliers among 100 measurements (7.00%)
  2 (2.00%) low mild
  4 (4.00%) high mild
  1 (1.00%) high severe

physical_plan_tpch_q7   time:   [4.0771 ms 4.0917 ms 4.1067 ms]
                        change: [-7.1486% -6.6862% -6.1909%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high mild

physical_plan_tpch_q8   time:   [6.0481 ms 6.0697 ms 6.0910 ms]
                        change: [-6.7553% -6.2264% -5.7076%] (p = 0.00 < 0.05)
                        Performance has improved.

physical_plan_tpch_q9   time:   [4.5076 ms 4.5234 ms 4.5394 ms]
                        change: [-6.8838% -6.4167% -5.9600%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high mild

physical_plan_tpch_q10  time:   [2.9380 ms 2.9475 ms 2.9574 ms]
                        change: [-7.1054% -6.5586% -6.0542%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 5 outliers among 100 measurements (5.00%)
  5 (5.00%) high mild

physical_plan_tpch_q11  time:   [2.3309 ms 2.3379 ms 2.3450 ms]
                        change: [-5.9245% -5.5252% -5.1065%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
  1 (1.00%) low mild
  2 (2.00%) high mild

physical_plan_tpch_q12  time:   [2.0939 ms 2.1058 ms 2.1193 ms]
                        change: [-4.2735% -3.5686% -2.8289%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 7 outliers among 100 measurements (7.00%)
  3 (3.00%) high mild
  4 (4.00%) high severe

Benchmarking physical_plan_tpch_q13: Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 6.7s, enable flat sampling, or reduce sample count to 60.
physical_plan_tpch_q13  time:   [1.3206 ms 1.3255 ms 1.3305 ms]
                        change: [-3.3886% -2.7905% -2.2341%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high mild

Benchmarking physical_plan_tpch_q14: Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 8.8s, enable flat sampling, or reduce sample count to 50.
physical_plan_tpch_q14  time:   [1.7025 ms 1.7112 ms 1.7211 ms]
                        change: [-14.099% -11.398% -8.8080%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
  3 (3.00%) high mild

physical_plan_tpch_q16  time:   [2.4419 ms 2.4517 ms 2.4616 ms]
                        change: [-10.500% -9.7909% -9.1054%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high mild

physical_plan_tpch_q17  time:   [2.2205 ms 2.2303 ms 2.2406 ms]
                        change: [-5.3559% -4.8812% -4.3619%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
  1 (1.00%) low mild
  1 (1.00%) high mild
  1 (1.00%) high severe

physical_plan_tpch_q18  time:   [2.4850 ms 2.4959 ms 2.5070 ms]
                        change: [-5.9772% -5.3788% -4.8142%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
  3 (3.00%) high mild

physical_plan_tpch_q19  time:   [5.5130 ms 5.5335 ms 5.5539 ms]
                        change: [-3.7923% -3.2835% -2.7258%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) low mild

physical_plan_tpch_q20  time:   [2.9745 ms 2.9860 ms 2.9981 ms]
                        change: [-5.2189% -4.7349% -4.2429%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 2 outliers among 100 measurements (2.00%)
  2 (2.00%) high mild

physical_plan_tpch_q21  time:   [4.4102 ms 4.4247 ms 4.4394 ms]
                        change: [-6.9218% -6.4708% -6.0108%] (p = 0.00 < 0.05)
                        Performance has improved.

physical_plan_tpch_q22  time:   [2.1663 ms 2.1753 ms 2.1852 ms]
                        change: [-5.1750% -4.5239% -3.8635%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 8 outliers among 100 measurements (8.00%)
  1 (1.00%) low mild
  3 (3.00%) high mild
  4 (4.00%) high severe

Benchmarking physical_plan_tpch_all: Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 6.8s, or reduce sample count to 70.
physical_plan_tpch_all  time:   [67.608 ms 67.925 ms 68.253 ms]
                        change: [-7.2306% -6.6810% -6.1284%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high mild

logical_plan_tpch_all   time:   [12.434 ms 12.540 ms 12.636 ms]
                        change: [-3.4299% -2.2768% -1.3315%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 6 outliers among 100 measurements (6.00%)
  4 (4.00%) low severe
  2 (2.00%) low mild

@jayzhan211
Copy link
Contributor Author

with Arc<str>

logical_select_one_from_700
                        time:   [580.12 µs 585.06 µs 589.73 µs]
                        change: [-6.3590% -5.7328% -5.0564%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high mild

physical_select_one_from_700
                        time:   [2.3826 ms 2.3916 ms 2.4006 ms]
                        change: [-7.0877% -6.6384% -6.1781%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
  3 (3.00%) high mild

Benchmarking logical_select_all_from_1000: Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 7.2s, or reduce sample count to 60.
logical_select_all_from_1000
                        time:   [70.414 ms 70.720 ms 71.037 ms]
                        change: [+3.3675% +4.0240% +4.6863%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high mild

Benchmarking physical_select_all_from_1000: Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 50.0s, or reduce sample count to 10.
physical_select_all_from_1000
                        time:   [506.06 ms 507.48 ms 508.95 ms]
                        change: [+2.5742% +2.9462% +3.3299%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 4 outliers among 100 measurements (4.00%)
  4 (4.00%) high mild

logical_trivial_join_low_numbered_columns
                        time:   [587.37 µs 592.07 µs 595.97 µs]
                        change: [-7.8608% -6.8978% -6.0233%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 13 outliers among 100 measurements (13.00%)
  7 (7.00%) low severe
  2 (2.00%) low mild
  2 (2.00%) high mild
  2 (2.00%) high severe

logical_trivial_join_high_numbered_columns
                        time:   [630.89 µs 632.24 µs 633.53 µs]
                        change: [-5.9321% -5.4946% -5.0383%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 9 outliers among 100 measurements (9.00%)
  1 (1.00%) low severe
  4 (4.00%) low mild
  4 (4.00%) high mild

logical_aggregate_with_join
                        time:   [823.32 µs 827.32 µs 830.69 µs]
                        change: [-8.0075% -7.5928% -7.1907%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
  1 (1.00%) low severe
  2 (2.00%) low mild

physical_plan_tpch_q1   time:   [3.7308 ms 3.7435 ms 3.7561 ms]
                        change: [-2.6559% -2.1877% -1.7373%] (p = 0.00 < 0.05)
                        Performance has improved.

physical_plan_tpch_q2   time:   [5.7711 ms 5.7942 ms 5.8175 ms]
                        change: [-7.6441% -7.1368% -6.6634%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 2 outliers among 100 measurements (2.00%)
  1 (1.00%) low mild
  1 (1.00%) high mild

Benchmarking physical_plan_tpch_q3: Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 10.0s, enable flat sampling, or reduce sample count to 40.
physical_plan_tpch_q3   time:   [1.9749 ms 1.9794 ms 1.9844 ms]
                        change: [-7.4539% -6.9879% -6.5380%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 7 outliers among 100 measurements (7.00%)
  1 (1.00%) low severe
  1 (1.00%) low mild
  4 (4.00%) high mild
  1 (1.00%) high severe

Benchmarking physical_plan_tpch_q4: Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 8.3s, enable flat sampling, or reduce sample count to 50.
physical_plan_tpch_q4   time:   [1.6496 ms 1.6537 ms 1.6581 ms]
                        change: [-6.7606% -6.0924% -5.4526%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
  1 (1.00%) low mild
  2 (2.00%) high mild

physical_plan_tpch_q5   time:   [2.8363 ms 2.8451 ms 2.8541 ms]
                        change: [-8.7362% -8.3231% -7.8946%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 6 outliers among 100 measurements (6.00%)
  1 (1.00%) low mild
  5 (5.00%) high mild

Benchmarking physical_plan_tpch_q6: Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 5.7s, enable flat sampling, or reduce sample count to 60.
physical_plan_tpch_q6   time:   [1.1385 ms 1.1419 ms 1.1455 ms]
                        change: [-4.6024% -3.9168% -3.1923%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 7 outliers among 100 measurements (7.00%)
  2 (2.00%) low mild
  4 (4.00%) high mild
  1 (1.00%) high severe

physical_plan_tpch_q7   time:   [4.0771 ms 4.0917 ms 4.1067 ms]
                        change: [-7.1486% -6.6862% -6.1909%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high mild

physical_plan_tpch_q8   time:   [6.0481 ms 6.0697 ms 6.0910 ms]
                        change: [-6.7553% -6.2264% -5.7076%] (p = 0.00 < 0.05)
                        Performance has improved.

physical_plan_tpch_q9   time:   [4.5076 ms 4.5234 ms 4.5394 ms]
                        change: [-6.8838% -6.4167% -5.9600%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high mild

physical_plan_tpch_q10  time:   [2.9380 ms 2.9475 ms 2.9574 ms]
                        change: [-7.1054% -6.5586% -6.0542%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 5 outliers among 100 measurements (5.00%)
  5 (5.00%) high mild

physical_plan_tpch_q11  time:   [2.3309 ms 2.3379 ms 2.3450 ms]
                        change: [-5.9245% -5.5252% -5.1065%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
  1 (1.00%) low mild
  2 (2.00%) high mild

physical_plan_tpch_q12  time:   [2.0939 ms 2.1058 ms 2.1193 ms]
                        change: [-4.2735% -3.5686% -2.8289%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 7 outliers among 100 measurements (7.00%)
  3 (3.00%) high mild
  4 (4.00%) high severe

Benchmarking physical_plan_tpch_q13: Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 6.7s, enable flat sampling, or reduce sample count to 60.
physical_plan_tpch_q13  time:   [1.3206 ms 1.3255 ms 1.3305 ms]
                        change: [-3.3886% -2.7905% -2.2341%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high mild

Benchmarking physical_plan_tpch_q14: Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 8.8s, enable flat sampling, or reduce sample count to 50.
physical_plan_tpch_q14  time:   [1.7025 ms 1.7112 ms 1.7211 ms]
                        change: [-14.099% -11.398% -8.8080%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
  3 (3.00%) high mild

physical_plan_tpch_q16  time:   [2.4419 ms 2.4517 ms 2.4616 ms]
                        change: [-10.500% -9.7909% -9.1054%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high mild

physical_plan_tpch_q17  time:   [2.2205 ms 2.2303 ms 2.2406 ms]
                        change: [-5.3559% -4.8812% -4.3619%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
  1 (1.00%) low mild
  1 (1.00%) high mild
  1 (1.00%) high severe

physical_plan_tpch_q18  time:   [2.4850 ms 2.4959 ms 2.5070 ms]
                        change: [-5.9772% -5.3788% -4.8142%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
  3 (3.00%) high mild

physical_plan_tpch_q19  time:   [5.5130 ms 5.5335 ms 5.5539 ms]
                        change: [-3.7923% -3.2835% -2.7258%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) low mild

physical_plan_tpch_q20  time:   [2.9745 ms 2.9860 ms 2.9981 ms]
                        change: [-5.2189% -4.7349% -4.2429%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 2 outliers among 100 measurements (2.00%)
  2 (2.00%) high mild

physical_plan_tpch_q21  time:   [4.4102 ms 4.4247 ms 4.4394 ms]
                        change: [-6.9218% -6.4708% -6.0108%] (p = 0.00 < 0.05)
                        Performance has improved.

physical_plan_tpch_q22  time:   [2.1663 ms 2.1753 ms 2.1852 ms]
                        change: [-5.1750% -4.5239% -3.8635%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 8 outliers among 100 measurements (8.00%)
  1 (1.00%) low mild
  3 (3.00%) high mild
  4 (4.00%) high severe

Benchmarking physical_plan_tpch_all: Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 6.8s, or reduce sample count to 70.
physical_plan_tpch_all  time:   [67.608 ms 67.925 ms 68.253 ms]
                        change: [-7.2306% -6.6810% -6.1284%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high mild

logical_plan_tpch_all   time:   [12.434 ms 12.540 ms 12.636 ms]
                        change: [-3.4299% -2.2768% -1.3315%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 6 outliers among 100 measurements (6.00%)
  4 (4.00%) low severe
  2 (2.00%) low mild

Which one does this compare with, Arc or main branch?

@comphead
Copy link
Contributor

comphead commented Apr 1, 2024

both comparisons are vs main branch. so let me know your thoughts, Arc<String> looks more attractive to me now, may be because String is more widely used and we dont lose cycles on the conversions

@comphead
Copy link
Contributor

comphead commented Apr 1, 2024

the PR is #9824

I'm still hesitating between Arc<str> and Arc<String> either of them mostly improves the performance.

@alamb
Copy link
Contributor

alamb commented Apr 1, 2024

I'm still hesitating between Arc<str> and Arc<String> either of them mostly improves the performance.

One vote for Arc<str> is that is what is used in DataType from arrow:

https://github.com/apache/arrow-rs/blob/77a3132bbd27ebb3cef0d79f24a9a839f125343c/arrow-schema/src/datatype.rs#L146

@comphead
Copy link
Contributor

comphead commented Apr 1, 2024

But Arc<String> gives more performance benefits #9824 (comment)

@jayzhan211
Copy link
Contributor Author

jayzhan211 commented Apr 2, 2024 •
edited
Loading

If TableReference is mostly read but has little mutation, I would go for Arc<str>.

The benchmark might change if the codebase grows, so we need to consider the role of the TableReference besides the benchmark. The benchmark for me in this PR is to ensure the change is better than the previous (Cow<& 'str>), If both Arc<str> and Arc<String> beats, then we can consider other factors

https://users.rust-lang.org/t/arc-string-vs-arc-str/30571

@alamb alamb mentioned this issue Apr 2, 2024
Merged
@comphead comphead mentioned this issue Apr 4, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

perf: Use Arc<str> instead of Cow<&'a> in the analyzer comphead/arrow-datafusion
3 participants
@alamb @comphead @jayzhan211