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.

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

speed up encode #42

Merged
merged 2 commits into from
May 6, 2024
Merged

speed up encode #42

merged 2 commits into from
May 6, 2024

Conversation

michaelkirk
Copy link
Member

  • attempt to make bench more deterministic
  • [perf] mutate rather than allocate a bunch of heap strings
    encode 10_000 coordinates at precision 1e-5
                            time:   [123.68 µs 123.80 µs 123.94 µs]
                            change: [-74.528% -74.461% -74.401%] (p = 0.00 < 0.05)
                            Performance has improved.
    Found 6 outliers among 100 measurements (6.00%)
      4 (4.00%) high mild
      2 (2.00%) high severe

    encode 10_000 coordinates at precision 1e-6
                            time:   [136.60 µs 137.55 µs 138.55 µs]
                            change: [-73.205% -73.066% -72.932%] (p = 0.00 < 0.05)
                            Performance has improved.
    Found 11 outliers among 100 measurements (11.00%)
      9 (9.00%) high mild
      2 (2.00%) high severe

@michaelkirk michaelkirk mentioned this pull request May 1, 2024
@michaelkirk michaelkirk force-pushed the mkirk/encode-perf branch from 4014433 to 3e2dd5c Compare May 1, 2024 03:11
let current = scale(current, factor);
let previous = scale(previous, factor);
let mut coordinate = (current - previous) << 1;
if (current - previous) < 0 {
coordinate = !coordinate;
}
let mut output: String = "".to_string();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This allocation goes away

@@ -62,7 +61,7 @@ fn encode(current: f64, previous: f64, factor: i32) -> Result<String, String> {
}
let from_char = char::from_u32((coordinate + 63) as u32).ok_or("Couldn't convert character")?;
output.push(from_char);
Copy link
Member Author

@michaelkirk michaelkirk May 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And we now just push directly onto the passed in string. Saving an allocation and a concatenation.

@michaelkirk
Copy link
Member Author

blech, sorry, I missed #40. Let me look at that first.

@mattiZed
Copy link
Contributor

mattiZed commented May 1, 2024

IMHO it should be easy to combine this change with #40 :)

EDIT: Oh sorry, I wasn't quite awake yet. #40 already includes this optimisation!

Mostly the issue is with "bench HUGE polyline6 decoding".
I'm not sure why.

Interestingly this "HUGE" polyine is not much bigger than the other
decode test, which seems to run in much more consistent time.
Bench output:

    encode 10_000 coordinates at precision 1e-5
                            time:   [123.68 µs 123.80 µs 123.94 µs]
                            change: [-74.528% -74.461% -74.401%] (p = 0.00 < 0.05)
                            Performance has improved.
    Found 6 outliers among 100 measurements (6.00%)
      4 (4.00%) high mild
      2 (2.00%) high severe

    encode 10_000 coordinates at precision 1e-6
                            time:   [136.60 µs 137.55 µs 138.55 µs]
                            change: [-73.205% -73.066% -72.932%] (p = 0.00 < 0.05)
                            Performance has improved.
    Found 11 outliers among 100 measurements (11.00%)
      9 (9.00%) high mild
      2 (2.00%) high severe
@michaelkirk michaelkirk force-pushed the mkirk/encode-perf branch from 3e2dd5c to db9b5b9 Compare May 6, 2024 23:29
@michaelkirk michaelkirk merged commit 1419c04 into main May 6, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants