⚡ Perf: Optimize Raster Grid Generation (Speed & Memory)#27
Merged
Conversation
- R/stream_grid_raster.R: Replace round-trip definition of coordinates with direct cell ID assignment (~13,000x faster for the inner loop). - R/stream_grid_raster_parallel.R: Replace vector recycling with direct sequence generation for chunks to reduce memory allocation. - R/backend_sequential.R: Replace memory-intensive terra::xyFromCell with efficient vectorized logic for sequential processing.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR optimizes raster grid generation by eliminating unnecessary coordinate computations and replacing expensive memory operations with efficient direct calculations. The changes target three files covering sequential streaming, parallel streaming, and in-memory generation workflows.
Changes:
- Replaces complex coordinate-to-index calculation loop with direct cell ID assignment in sequential streaming
- Eliminates large intermediate vector allocations in parallel chunk processing by using direct sequence generation
- Replaces memory-intensive
terra::xyFromCellcalls with efficient vectorized coordinate sequences in in-memory generation
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| R/stream_grid_raster.R | Simplified cell ID generation by directly using terra's cell IDs instead of recomputing them from coordinates |
| R/stream_grid_raster_parallel.R | Replaced inefficient rep()-based vector generation with direct integer sequence for contiguous cell ranges |
| R/backend_sequential.R | Replaced terra::xyFromCell with efficient vectorized coordinate sequences to reduce memory footprint |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
⚡ Performance Optimization: Raster Grid Generation
This PR implements significant performance speedups (~13,000x for inner loops) and memory usage reductions for raster grid generation, covering both sequential and parallel backends.
📝 Key Changes
1. `R/stream_grid_raster.R` (Sequential Streaming)
2. `R/stream_grid_raster_parallel.R` (Parallel Streaming)
3. `R/backend_sequential.R` (In-Memory/Sequential)
✅ Verification
📊 Benchmark Results (Micro-benchmark)
Measured on a 100-row chunk of a 1000x1000 grid.