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

v0.7.0: minor cleanups #31

Merged
merged 3 commits into from
Feb 5, 2024
Merged

v0.7.0: minor cleanups #31

merged 3 commits into from
Feb 5, 2024

Conversation

nyurik
Copy link
Collaborator

@nyurik nyurik commented Feb 2, 2024

  • optimize tile_id to compute base 15-22x faster
  • use the same features approach as reqwest - introduce "hidden" features to simplify conditional compilation
  • use just fmt to clean up some formatting
  • slightly shorter vars in a few places
  • sort mods first, uses second in the lib.rs
  • move all backends into simple files because this way we can incrementally append new functions to the base reader without any conditionals - each backend file defines its own
  • add a few reqwest crate features to simplify their enablement
  • reexport tilejson crate

tile_id perf optimization

I ran criterion benchmarks for zoom 28 and zoom 12. The hardcoded table values only has 0..20 zooms. Note that this is only base computation, not the hilbert part.

For z28, the performance is 15x better (77.1ns -> 5.3ns)
For z12, the performance is 22x better (24.523ns -> 1.1221ns)

tile_id_original_28     time:   [76.980 ns 77.155 ns 77.386 ns]
Found 11 outliers among 100 measurements (11.00%)
  1 (1.00%) high mild
  10 (10.00%) high severe

tile_id_original_12     time:   [24.518 ns 24.523 ns 24.529 ns]
Found 16 outliers among 100 measurements (16.00%)
  9 (9.00%) high mild
  7 (7.00%) high severe

tile_id_new             time:   [5.3056 ns 5.3072 ns 5.3094 ns]
                        change: [-0.4333% -0.1397% +0.0256%] (p = 0.38 > 0.05)
                        No change in performance detected.
Found 8 outliers among 100 measurements (8.00%)
  1 (1.00%) low mild
  4 (4.00%) high mild
  3 (3.00%) high severe

tile_id_new_12          time:   [1.1171 ns 1.1221 ns 1.1313 ns]
Found 13 outliers among 100 measurements (13.00%)
  6 (6.00%) high mild
  7 (7.00%) high severe
Benchmarking code

add these lines right after the [dependencies] section:

criterion = "0.5.1"
[[bench]]
name = "tile_id"
harness = false

Create a new file benches/tile_id.rs:

#![allow(clippy::unreadable_literal)]

use criterion::{black_box, criterion_group, criterion_main, Criterion};

const PYRAMID_SIZE_BY_ZOOM: [u64; 21] = [
    /*  0 */ 0,
    /*  1 */ 1,
    /*  2 */ 5,
    /*  3 */ 21,
    /*  4 */ 85,
    /*  5 */ 341,
    /*  6 */ 1365,
    /*  7 */ 5461,
    /*  8 */ 21845,
    /*  9 */ 87381,
    /* 10 */ 349525,
    /* 11 */ 1398101,
    /* 12 */ 5592405,
    /* 13 */ 22369621,
    /* 14 */ 89478485,
    /* 15 */ 357913941,
    /* 16 */ 1431655765,
    /* 17 */ 5726623061,
    /* 18 */ 22906492245,
    /* 19 */ 91625968981,
    /* 20 */ 366503875925,
];

pub fn tile_id_new(z: u8) -> u64 {
    // The 0/0/0 case is not needed for the base id computation, but it will fail hilbert_2d::u64::xy2h_discrete
    if z == 0 {
        return 0;
    }

    let z_ind = usize::from(z);
    let base_id = if z_ind < PYRAMID_SIZE_BY_ZOOM.len() {
        PYRAMID_SIZE_BY_ZOOM[z_ind]
    } else {
        let last_ind = PYRAMID_SIZE_BY_ZOOM.len() - 1;
        PYRAMID_SIZE_BY_ZOOM[last_ind] + (last_ind..z_ind).map(|i| 1_u64 << (i << 1)).sum::<u64>()
    };

    base_id
}

pub fn tile_id_original(z: u8) -> u64 {
    if z == 0 {
        return 0;
    }

    // TODO: minor optimization with bit shifting
    let base_id: u64 = 1 + (1..z).map(|i| 4u64.pow(u32::from(i))).sum::<u64>();

    base_id
}

fn bench_original(c: &mut Criterion) {
    c.bench_function("tile_id_original_28", |b| {
        b.iter(|| {
            assert_eq!(tile_id_original(black_box(28)), 24019198012642645);
        })
    });

    c.bench_function("tile_id_original_12", |b| {
        b.iter(|| {
            assert_eq!(tile_id_original(black_box(12)), 5592405);
        })
    });
}

fn bench_new(c: &mut Criterion) {
    c.bench_function("tile_id_new", |b| {
        b.iter(|| {
            assert_eq!(tile_id_new(black_box(28)), 24019198012642645);
        })
    });

    c.bench_function("tile_id_new_12", |b| {
        b.iter(|| {
            assert_eq!(tile_id_new(black_box(12)), 5592405);
        })
    });
}

criterion_group!(benches, bench_original, bench_new);
criterion_main!(benches);

* only add `async-compression/tokio` dependency feature with `mmap-async-tokio`
* use `just fmt` to clean up some formatting
* slightly shorter vars in a few places
* sort mods first, uses second in the lib.rs
@nyurik nyurik force-pushed the cleanups branch 3 times, most recently from a0dd43f to ee05cc4 Compare February 3, 2024 04:41
Copy link
Member

@lseelenbinder lseelenbinder left a comment

Choose a reason for hiding this comment

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

LGTM; good cleanup of the features especially.

@nyurik nyurik merged commit b5a9f82 into stadiamaps:main Feb 5, 2024
1 check passed
@nyurik nyurik deleted the cleanups branch February 5, 2024 18:55
@nyurik
Copy link
Collaborator Author

nyurik commented Feb 5, 2024

Published as v0.7.0

@nyurik nyurik changed the title v0.6.1: minor cleanups v0.7.0: minor cleanups Feb 5, 2024
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.

2 participants