Switch to structured errors, inline encoding #48

urschrei · 2024-05-11T13:28:31Z

This PR switches to "real" structured errors, and inlines an encoding step. Perf compared to current main:

encode 10_000 coordinates at precision 1e-5
                        time:   [176.72 µs 177.11 µs 177.57 µs]
                        change: [-7.9523% -7.6511% -7.3754%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high mild

encode 10_000 coordinates at precision 1e-6
                        time:   [207.34 µs 207.55 µs 207.81 µs]
                        change: [-4.6981% -4.4789% -4.1689%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 13 outliers among 100 measurements (13.00%)
  4 (4.00%) high mild
  9 (9.00%) high severe

decode 10_000 coordinates at precision 1e-5
                        time:   [101.06 µs 101.53 µs 102.02 µs]
                        change: [-34.622% -34.329% -34.044%] (p = 0.00 < 0.05)
                        Performance has improved.

decode 10_000 coordinates at precision 1e-6
                        time:   [120.94 µs 121.46 µs 122.02 µs]
                        change: [-30.601% -30.360% -30.095%] (p = 0.00 < 0.05)
                        Performance has improved.

The inlining improves encoding perf. Without inlining, the change to structured errors improves decoding perf as above, but regresses encoding perf by 10 - 12 %.

Please feel free to bikeshed the Error impl: I usually use thiserror for everything but since this crate doesn't depend on geo I didn't want to introduce another dependency.

michaelkirk · 2024-05-11T17:36:37Z

I'm seeing some different results:

On x86 I see some 10-15% improvements to encode, but no change for decode.

$ uname -a
Linux foo 6.1.0-18-amd64 #1 SMP PREEMPT_DYNAMIC Debian 6.1.76-1 (2024-02-01) x86_64 GNU/Linux
$ cargo --version
cargo 1.78.0 (54d8815d0 2024-03-26)

$ cargo bench --bench="*" -- --baseline=sha-5340961
   Compiling polyline v0.10.2 (/pool1/mkirk-src/georust/polyline)
    Finished `bench` profile [optimized] target(s) in 1.51s
     Running benches/benchmarks.rs (target/release/deps/benchmarks-13776a8dbab4bcdb)
Gnuplot not found, using plotters backend
encode 10_000 coordinates at precision 1e-5
                        time:   [197.15 µs 198.60 µs 200.18 µs]
                        change: [-14.911% -14.244% -13.438%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
  3 (3.00%) high mild

encode 10_000 coordinates at precision 1e-6
                        time:   [219.22 µs 219.69 µs 220.28 µs]
                        change: [-14.104% -13.672% -13.255%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 9 outliers among 100 measurements (9.00%)
  6 (6.00%) high mild
  3 (3.00%) high severe

decode 10_000 coordinates at precision 1e-5
                        time:   [133.48 µs 133.94 µs 134.47 µs]
                        change: [-0.6733% +0.6886% +2.6489%] (p = 0.59 > 0.05)
                        No change in performance detected.
Found 17 outliers among 100 measurements (17.00%)
  5 (5.00%) high mild
  12 (12.00%) high severe

decode 10_000 coordinates at precision 1e-6
                        time:   [157.01 µs 157.52 µs 158.16 µs]
                        change: [-2.4903% -1.1355% +0.4666%] (p = 0.12 > 0.05)
                        No change in performance detected.
Found 12 outliers among 100 measurements (12.00%)
  2 (2.00%) high mild
  10 (10.00%) high severe

On my aarch64 machine, large regression in encode and 10-15% improvement in decode.

$ uname -a
Darwin bar 23.4.0 Darwin Kernel Version 23.4.0: Fri Mar 15 00:10:42 PDT 2024; root:xnu-10063.101.17~1/RELEASE_ARM64_T6000 arm64

$ cargo --version
cargo 1.78.0 (54d8815d0 2024-03-26)

$ cargo bench --bench="*" -- --baseline=sha-5340961
   Compiling polyline v0.10.2 (/Users/mkirk/src/georust/polyline)
    Finished `bench` profile [optimized] target(s) in 1.31s
     Running benches/benchmarks.rs (target/release/deps/benchmarks-8b91c88197106af3)
encode 10_000 coordinates at precision 1e-5
                        time:   [180.80 µs 181.20 µs 181.66 µs]
                        change: [+70.949% +71.378% +71.805%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 10 outliers among 100 measurements (10.00%)
  3 (3.00%) low mild
  7 (7.00%) high mild

encode 10_000 coordinates at precision 1e-6
                        time:   [220.65 µs 221.66 µs 222.59 µs]
                        change: [+69.852% +70.752% +71.690%] (p = 0.00 < 0.05)
                        Performance has regressed.

decode 10_000 coordinates at precision 1e-5
                        time:   [84.025 µs 84.819 µs 85.599 µs]
                        change: [-2.0141% -1.0429% -0.0875%] (p = 0.04 < 0.05)
                        Change within noise threshold.

decode 10_000 coordinates at precision 1e-6
                        time:   [99.168 µs 100.22 µs 101.23 µs]
                        change: [-12.613% -11.723% -10.840%] (p = 0.00 < 0.05)
                        Performance has improved.

This reminds me of the "touchy" madness I was talking about earlier. Can you provide any platform details?

urschrei · 2024-05-11T17:39:42Z

Ugh. My results are on macOS arm64. Most of the reason for this PR was to try to move as much of error-handling and formatting out of the encode / decode functions in order to try to address the "touchiness". Bah.

Darwin MacBook-Pro.lan 23.3.0 Darwin Kernel Version 23.3.0: Wed Dec 20 21:31:00 PST 2023; root:xnu-10002.81.5~7/RELEASE_ARM64_T6020 arm64

urschrei · 2024-05-11T17:59:31Z

What I find most confusing here is that you're seeing regressions on the same platform.

Different processor? I'm using an M2 pro plugged into mains power.

I can try updating my macOS version too.

urschrei · 2024-05-11T18:18:07Z

OK, more attempts at consistency.

cargo clean
cargo update
cargo 1.78.0 (54d8815d0 2024-03-26)

`main` (`5340961`)

Command: cargo bench -- --save-baseline main

encode 10_000 coordinates at precision 1e-5
                        time:   [100.76 µs 100.90 µs 101.04 µs]
Found 6 outliers among 100 measurements (6.00%)
  2 (2.00%) low mild
  3 (3.00%) high mild
  1 (1.00%) high severe

encode 10_000 coordinates at precision 1e-6
                        time:   [124.84 µs 125.50 µs 126.24 µs]

decode 10_000 coordinates at precision 1e-5
                        time:   [79.999 µs 80.795 µs 81.611 µs]

decode 10_000 coordinates at precision 1e-6
                        time:   [102.27 µs 103.04 µs 103.88 µs]
Found 5 outliers among 100 measurements (5.00%)
  3 (3.00%) low mild
  1 (1.00%) high mild
  1 (1.00%) high severe

`structured_errors` (`626bbdb`)

Command: cargo bench

encode 10_000 coordinates at precision 1e-5
                        time:   [83.497 µs 83.699 µs 83.968 µs]
Found 3 outliers among 100 measurements (3.00%)
  2 (2.00%) high mild
  1 (1.00%) high severe

encode 10_000 coordinates at precision 1e-6
                        time:   [97.963 µs 98.124 µs 98.290 µs]
Found 9 outliers among 100 measurements (9.00%)
  7 (7.00%) high mild
  2 (2.00%) high severe

decode 10_000 coordinates at precision 1e-5
                        time:   [71.105 µs 71.637 µs 72.167 µs]

decode 10_000 coordinates at precision 1e-6
                        time:   [88.605 µs 89.660 µs 90.705 µs]

Comparison

Command: cargo bench -- --load-baseline new --baseline main

encode 10_000 coordinates at precision 1e-5
                        time:   [83.495 µs 83.699 µs 83.965 µs]
                        change: [-17.406% -17.103% -16.690%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
  2 (2.00%) high mild
  1 (1.00%) high severe

encode 10_000 coordinates at precision 1e-6
                        time:   [97.962 µs 98.124 µs 98.291 µs]
                        change: [-21.526% -21.076% -20.602%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 9 outliers among 100 measurements (9.00%)
  7 (7.00%) high mild
  2 (2.00%) high severe

decode 10_000 coordinates at precision 1e-5
                        time:   [71.110 µs 71.637 µs 72.164 µs]
                        change: [-12.601% -11.857% -11.095%] (p = 0.00 < 0.05)
                        Performance has improved.

decode 10_000 coordinates at precision 1e-6
                        time:   [88.615 µs 89.660 µs 90.693 µs]
                        change: [-15.005% -14.143% -13.241%] (p = 0.00 < 0.05)
                        Performance has improved.

michaelkirk · 2024-05-11T19:30:34Z

I keep reading this: https://bheisler.github.io/criterion.rs/book/user_guide/command_line_options.html#baselines

--save-baseline <name> will compare against the named baseline, then overwrite it.
--baseline <name> will compare against the named baseline without overwriting it.
--load-baseline <name> will load the named baseline as the new data set rather than the previous baseline.

but cannot understand the difference between --load-baseline and --baseline.

urschrei · 2024-05-11T19:40:37Z

Oh yeah it's confusing but the actual example below it makes sense?

michaelkirk · 2024-05-11T20:20:06Z

Ah, I see. It made sense once I just tried to run it.

So load-baseline doesn't run any benchmarks, it only generates a report from a previously saved benchmark against a previously saved baseline. So presumably you'd previously run cargo bench -- --save-baseline new

urschrei · 2024-05-11T20:23:23Z

I also just found this, which works very well and can actually be understood:

https://github.com/BurntSushi/critcmp

michaelkirk · 2024-05-11T20:25:49Z

I'm on M1, but I'd be surprised if there'd be much difference between the architectures of M1 and M2/M3.

Since I'm seeing mostly positive results on x64_64, mixed results on aarch64, and because I think it's "better" to have structural vs. String errors, I think we should merge this.

michaelkirk · 2024-05-11T17:49:56Z

src/lib.rs

    let mut shift = 0;
    let mut result = 0;
-    while let Some((idx, mut byte)) = chars.next() {
+    for (idx, mut byte) in chars.by_ref() {


clippy 😎

michaelkirk · 2024-05-11T20:55:04Z

https://github.com/BurntSushi/critcmp

Ooo this is nice.

Switch to structured errors, inline encoding

cc7195b

urschrei requested a review from michaelkirk May 11, 2024 13:28

urschrei added 3 commits May 11, 2024 14:49

Simplify Display impl for error

b098c5d

Document error fields

4b569c4

Fix error message case according to convention

626bbdb

michaelkirk approved these changes May 11, 2024

View reviewed changes

urschrei added this pull request to the merge queue May 11, 2024

Merged via the queue into main with commit 6872788 May 11, 2024
1 check passed

michaelkirk mentioned this pull request May 11, 2024

Skip intermediate coord representation #49

Merged

michaelkirk mentioned this pull request May 11, 2024

flexpolyline bench #46

Merged

This was referenced May 11, 2024

Improve structured errors #50

Merged

perf gap with flexpolyline #35

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switch to structured errors, inline encoding #48

Switch to structured errors, inline encoding #48

urschrei commented May 11, 2024 •

edited

Loading

michaelkirk commented May 11, 2024

urschrei commented May 11, 2024 •

edited

Loading

urschrei commented May 11, 2024

urschrei commented May 11, 2024

michaelkirk commented May 11, 2024

urschrei commented May 11, 2024

michaelkirk commented May 11, 2024

urschrei commented May 11, 2024

michaelkirk commented May 11, 2024 •

edited

Loading

michaelkirk May 11, 2024

urschrei May 11, 2024

michaelkirk commented May 11, 2024

Switch to structured errors, inline encoding #48

Switch to structured errors, inline encoding #48

Conversation

urschrei commented May 11, 2024 • edited Loading

michaelkirk commented May 11, 2024

urschrei commented May 11, 2024 • edited Loading

urschrei commented May 11, 2024

urschrei commented May 11, 2024

main (5340961)

structured_errors (626bbdb)

Comparison

michaelkirk commented May 11, 2024

urschrei commented May 11, 2024

michaelkirk commented May 11, 2024

urschrei commented May 11, 2024

michaelkirk commented May 11, 2024 • edited Loading

michaelkirk May 11, 2024

Choose a reason for hiding this comment

urschrei May 11, 2024

Choose a reason for hiding this comment

michaelkirk commented May 11, 2024

urschrei commented May 11, 2024 •

edited

Loading

urschrei commented May 11, 2024 •

edited

Loading

`main` (`5340961`)

`structured_errors` (`626bbdb`)

michaelkirk commented May 11, 2024 •

edited

Loading