Running DGGAL thread safe

[MichaelJendryke]

Can't you do impl Send for DGGRS or similar in your code base to add these if necessary?

I have to check if that is possible without touching the dggal_cffi.rs script that you created.

What were you trying to run in parallel?

Please consider this minimal example:

extern crate ecrt;
use ecrt::Application;
extern crate dggal;
use dggal::{DGGAL, DGGRS, GeoPoint};
use std;
use std::env;
use std::time::Instant;

fn main() {
    let args: Vec<String> = env::args().collect();
    let my_app = Application::new(&args);
    let dggal = DGGAL::new(&my_app);
    let dggrs: DGGRS = DGGRS::new(&dggal, "IVEA3H").expect("Unknown DGGRS");

    let pnt: GeoPoint = GeoPoint {
        lat: 52.3,
        lon: 12.3,
    };

    let zone = dggrs.getZoneFromWGS84Centroid(8, &pnt);
    println!("The Zone ID for the given point is: \n{:?}\n\n", zone);

    let sub_zones = dggrs.getSubZones(zone, 5);

    let t0 = Instant::now();
    let ga: Vec<_> = sub_zones
        .iter()
        .map(|zone: &u64| {
            let z: u64 = *zone;
            println!("{:?}", z);
            dggrs.getZoneWGS84Vertices(z)
        })
        .collect();
    println!("iter getZoneWGS84Verticies() took {:.2?}", t0.elapsed());

    let t1 = Instant::now();
    use rayon::prelude::*;
    let ga: Vec<_> = sub_zones
        .par_iter() // WARN: par_iter does not work because the underlying ecere/dggal C FFI is not thread safe.
        .map(|zone: &u64| {
            let z: u64 = *zone;
            println!("{:?}", z);
            dggrs.getZoneWGS84Vertices(z)
        })
        .collect();
    println!("par_iter getZoneWGS84Verticies() took {:.2?}", t1.elapsed());
}

I am looping over the subzone IDs with iter() and par_iter() to e.g. generate the geometry. You can comment out the lower part with par_iter() to see that the regular iter() works. Thanks!

Originally posted by @MichaelJendryke in GeoPlegma/GeoPlegma#25 (comment)

[jerstlouis]

@MichaelJendryke I was able to build and run that code successfully after the simple fix:

ecere/dggal@4ab4b4c

adding:

unsafe impl Sync for DGGRS {

}

It's quite annoying/baffling that you can't implement traits outside the crates where the types/traits are defined. Kind of defeats the whole flexibility/purpose of traits if you ask me!

iter getZoneWGS84Verticies() took 3.36ms
par_iter getZoneWGS84Verticies() took 6.90ms

As you can see, the parallel version actually takes longer to execute in that exact test.

I think these calls to DGGAL are really not the place where parallelism is going to be most useful.

Rather, it is more time-intensive operations done on separate zones / sub-zones.

Also the println() are really taking all the time.
Without the println() and a relative depth 13 instead of 5 I get:

iter getZoneWGS84Verticies() took 1.88s
par_iter getZoneWGS84Verticies() took 286.58ms

so that is an improvement :)

[MichaelJendryke]

That's amazing. Yeah, the print statement is a left over and I would only consider doing this for e.g. large amounts of points that need to be coded as ZoneIDs.

[jerstlouis]

@MichaelJendryke Curiously, even with 13 relative depth and no println(), the parallel version takes longer for getZoneRefinedWGS84Vertices():

iter getZoneRefinedWGS84Verticies() took 3.18s
par_iter getZoneRefinedWGS84Verticies() took 8.54s

I would have expected this to benefit more from the parallelism, so I'm not sure what's really going on?

I do see in htop a single core used for the iter part, and all of my 12 cores at 100% for par_iter, so this is a bit strange!

Is the final collection causing extra data copy which takes longer than the actual computation? (said computation which should be significantly more involved for getZoneRefinedWGS84Vertices() compared to getZoneWGS84Vertices())

[MichaelJendryke]

It's quite annoying/baffling that you can't implement traits outside the crates where the types/traits are defined. Kind of defeats the whole flexibility/purpose of traits if you ask me!

Maybe the Rust book helps, which says in the last paragraph of that section:

But we can’t implement external traits on external types.

[MichaelJendryke]

Is the final collection causing extra data copy which takes longer than the actual computation?

are you referring to the .collect() at the end of the iterator? I would have to explore more as I also have not used rayon like this before.

[jerstlouis]

Maybe the Rust book helps,

Well I had realized that you can't, but the rationale given is indeed insightful:

This rule ensures that other people’s code can’t break your code and vice versa. Without the rule, two crates could implement the same trait for the same type, and Rust wouldn’t know which implementation to use.

Wouldn't it make more sense to only raise the issue when this conflict occur and provide a mechanism to resolve it one way or the other? I really see it as a limitation that you can't extend a trait defined in A for a type defined in B how you want in your own project using A and B!

are you referring to the .collect() at the end of the iterator?

Yes, exactly.