Skip to content

Conversation

@zhangfengcdt
Copy link
Member

This commit introduces GPU-accelerated spatial join capabilities to SedonaDB, enabling significant performance improvements for large-scale spatial join operations.

Key changes:

  • Add new sedona-spatial-join-gpu crate that provides GPU-accelerated spatial join execution using CUDA via the sedona-libgpuspatial library.
  • Implement GpuSpatialJoinExec execution plan with build/probe phases that efficiently handles partitioned data by sharing build-side data across probes.
  • Add GPU backend abstraction (GpuBackend) for geometry data transfer and spatial predicate evaluation on GPU.
  • Extend the spatial join optimizer to automatically select GPU execution when available and beneficial, with configurable thresholds and fallback to CPU.
  • Add configuration options in SedonaOptions for GPU spatial join settings including enable/disable, row thresholds, and CPU fallback behavior.
  • Include comprehensive benchmarks and functional tests for GPU spatial join correctness validation against CPU reference implementations.

…support

This commit introduces GPU-accelerated spatial join capabilities to SedonaDB,
enabling significant performance improvements for large-scale spatial join
operations.

Key changes:
- Add new `sedona-spatial-join-gpu` crate that provides GPU-accelerated spatial
  join execution using CUDA via the `sedona-libgpuspatial` library.
- Implement `GpuSpatialJoinExec` execution plan with build/probe phases that
  efficiently handles partitioned data by sharing build-side data across probes.
- Add GPU backend abstraction (`GpuBackend`) for geometry data transfer and
  spatial predicate evaluation on GPU.
- Extend the spatial join optimizer to automatically select GPU execution when
  available and beneficial, with configurable thresholds and fallback to CPU.
- Add configuration options in `SedonaOptions` for GPU spatial join settings
  including enable/disable, row thresholds, and CPU fallback behavior.
- Include comprehensive benchmarks and functional tests for GPU spatial join
  correctness validation against CPU reference implementations.
@zhangfengcdt
Copy link
Member Author

Will merge the patch to fix the failing example build once this is merged - #486

Copy link
Member

@paleolimbot paleolimbot left a comment

Choose a reason for hiding this comment

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

Thank you for working on this!

In addition to specific comments, I'm concerned about the proliferation of conditional compilation (partiuclarly within sedona-spatial-join, which is a rather important part of our engine to keep clean).

At a high level what sedona-spatial-join-gpu is doing is more like sedona-spatial-join-extension: it provides a simpler (FFI-friendly) mechanism to inject a join operator without dealing with the DataFusion-y details. I think most of the conditional compilation/dead code/unused/ignore directives could be avoided if we add a CPU join extension and use that for all the tests. The GPU extension itself would then be a runtime implementation detail (eventually loaded at runtime via FFI).

Comment on lines +34 to +39
// Helper execution plan that returns a single pre-loaded batch
struct SingleBatchExec {
schema: Arc<Schema>,
batch: RecordBatch,
props: datafusion::physical_plan::PlanProperties,
}
Copy link
Member

Choose a reason for hiding this comment

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

This seems very similar to SessionContext::register_batch() and is a lot of lines of code. Do we need this?

Comment on lines +143 to +154
/// Generate random points within a bounding box
fn generate_random_points(count: usize) -> Vec<String> {
use rand::Rng;
let mut rng = rand::thread_rng();
(0..count)
.map(|_| {
let x: f64 = rng.gen_range(-180.0..180.0);
let y: f64 = rng.gen_range(-90.0..90.0);
format!("POINT ({} {})", x, y)
})
.collect()
}
Copy link
Member

Choose a reason for hiding this comment

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

We have a random geometry generator in sedona-testing (that is used in the non-GPU join tests and elsewhere) that I think we should be using here!

sedona_libgpuspatial::SpatialPredicate::Intersects,
),
device_id: 0,
batch_size: 8192,
Copy link
Member

Choose a reason for hiding this comment

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

Should this be Option<usize> so that it can default to the datafusion.batch_size setting?

let properties = PlanProperties::new(
eq_props,
partitioning,
EmissionType::Final, // GPU join produces all results at once
Copy link
Member

Choose a reason for hiding this comment

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

Just checking that this is correct (I thought that because one side is streaming the output might be incremental?)

Comment on lines +26 to +34
/// GPU backend for spatial operations
#[allow(dead_code)]
pub struct GpuBackend {
device_id: i32,
gpu_context: Option<GpuSpatialContext>,
}

#[allow(dead_code)]
impl GpuBackend {
Copy link
Member

Choose a reason for hiding this comment

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

Can these dead code markers be removed?

let kernels = scalar_kernels();
let sedona_type = SedonaType::Wkb(Edges::Planar, lnglat());

let _cpu_testers: std::collections::HashMap<&str, ScalarUdfTester> = [
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason this variable is not used / can we do this using a for loop to avoid this indirection?


#[cfg(feature = "gpu")]
#[tokio::test]
#[ignore] // Requires GPU hardware
Copy link
Member

Choose a reason for hiding this comment

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

We need to figure out a way to not ignore tests in this repo (in this case I think these tests shouldn't exist if the gpu feature isn't enabled so we shouldn't need the ignore it?)

Comment on lines +1267 to +1273
SpatialRelationType::Intersects => LibGpuPred::Intersects,
SpatialRelationType::Contains => LibGpuPred::Contains,
SpatialRelationType::Covers => LibGpuPred::Covers,
SpatialRelationType::Within => LibGpuPred::Within,
SpatialRelationType::CoveredBy => LibGpuPred::CoveredBy,
SpatialRelationType::Touches => LibGpuPred::Touches,
SpatialRelationType::Equals => LibGpuPred::Equals,
Copy link
Member

Choose a reason for hiding this comment

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

Can we move SpatialRelationType to sedona-geometry or sedona-common to avoid two copies?

Copy link
Member

Choose a reason for hiding this comment

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

git submodule update --recursive should remove this diff

default = ["mimalloc"]
mimalloc = ["dep:mimalloc", "dep:libmimalloc-sys"]
s2geography = ["sedona/s2geography"]
gpu = ["sedona/gpu"]
Copy link
Member

Choose a reason for hiding this comment

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

Because we don't have any tests in Python for this feature I suggest leaving this out for now (a follow-up PR could add Python support + a test)

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