Skip to content

perf(analysis): Erase and Update work on several cores (#1252) - #1744

Merged
pick2510 merged 1 commit into
opengeos:mainfrom
pick2510:perf/erase-update-threads
Nov 10, 2026
Merged

pick2510 merged 1 commit into
opengeos:mainfrom
pick2510:perf/erase-update-threads

Conversation

@pick2510

@pick2510 pick2510 commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

#1252 step 2, second part: Erase and Update work on several cores. It follows the overlay tools (#1740).

Measured in a release build on a Xeon Silver 4114 (40 threads), with the same data as #1740: 100,000 input polygons against 20,000 erase/update polygons (64-vertex circles, overlapping):

Tool 1 thread 16 threads (the default limit)
Erase 6.39 s 0.73 s
Update 6.25 s 0.72 s

How

  • The erase/update zones come from overlay_zones_on, which perf(analysis): Intersect, Identity, Union and Symmetrical Difference work on several cores (#1252) #1740 already made work on several cores.
  • Each input feature's part outside the zones is cut on its own on parallel::try_map_on, in a new helper outside_each that both tools share. The cut (outside) is the same code as before.
  • The output rows are made in input order on the calling thread, as before. Erase leaves out features without geometry; Update keeps them as they are. Update's second step, which adds the update polygons, is cheap and stays on one thread.

The #1252 rules hold:

  • the output is the same as on one thread, bit for bit;
  • one thread below the usual threshold of features (PARALLEL_FEATURES) and in the browser;
  • the thread count comes from rustgis_data::threads;
  • Cancel is looked at, and progress counted, feature by feature.

Tests

erase_and_update_on_several_threads_are_on_one runs Erase (on polygons, lines and points) and Update on 1, 2, 7 and 16 threads, and compares every row's geometry and attributes. Its data covers 600 input features against 200 polygons, erase/update features of two overlapping parts, and input features without geometry. The existing Erase and Update tests pass unchanged.

No visible UI change.

Summary by CodeRabbit

  • Bug Fixes
    • Erase and Update now produce consistent results across single-threaded and multi-threaded runs, preserving input order and handling missing geometries and multi-part overlay zones.

opengeos#1252 step 2, Erase and Update. Their erase/update zones come from `overlay_zones_on` (on
several cores since the overlay tools), and each input feature's part outside the zones is now
cut on its own on `parallel::try_map_on`, in `outside_each`. The output rows are made in order on
the calling thread, so the output is the same as on one thread, bit for bit.

Cancel is looked at, and progress counted, feature by feature as before; one thread in the
browser and below the usual threshold of features.
@coderabbitai

coderabbitai Bot commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

Erase and Update now use optional thread counts for overlay-zone construction and per-feature geometry processing. Both tools preserve input order. Regression tests compare single-thread results with results from multiple thread counts.

Changes: Overlay tool parallelism

Layer / File(s) Summary
Thread-configurable Erase and Update
crates/analysis/src/tools.rs
erase and update now delegate to internal helpers that accept an optional thread count. Per-feature outside geometries are processed in parallel and returned in input order.
Output handling and thread-count tests
crates/analysis/src/tools.rs
Update retains input rows without geometry, emits rows with remaining outside geometry, and omits fully covered rows. Tests compare Erase and Update outputs at 2, 7, and 16 threads with single-thread outputs.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant erase
  participant erase_on
  participant update
  participant update_on
  participant outside_each
  erase->>erase_on: Pass optional thread count
  erase_on->>outside_each: Map input rows in parallel
  outside_each-->>erase_on: Return per-row outside geometries in input order
  update->>update_on: Pass optional thread count
  update_on->>outside_each: Map input rows in parallel
  outside_each-->>update_on: Return per-row outside geometries in input order
Loading

Possibly related issues

  • Issue 1252: The issue tracks Erase and Update parallelization, which this change implements.
  • Issue 1291: The issue identifies Erase and Update as tools needing cancellation and progress support; this change uses shared parallel mapping for feature tasks.

Possibly related PRs

Suggested reviewers: mavrick-f

Merge Risk: Minimal · up to 33fed

Erase and Update now process features on several threads while keeping output order and results the same as before. No concrete merge-blocking risk was found.

Pre-merge checks: Passed 5

Check name Status Explanation
Title check Passed The title clearly identifies the main change: enabling Erase and Update to use multiple cores.
Description check Passed The description covers the change, testing, performance results, and no visible UI change. It references related issues #1252 and #1740, but does not include a separate Related issue section.
Docstring Coverage Passed No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 0 files. (1 skipped: 1 too large.)

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

I’m a rabbit with a thread-count chart,
I watch each feature take its part.
The rows return in order, neat,
While polygons and lines compete.
I nibble greens and test once more,
Then hop along the overlay floor.

Recent review info

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 78669b81-c529-4ea7-96b3-a256d9e6ab28
Commits

Reviewing files that changed from the base of the PR and between 4d1dc25 and 33fed9c.

Files selected for processing: 1

  • crates/analysis/src/tools.rs

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

@github-actions

Copy link
Copy Markdown
Contributor

Code review

Reviewed the single changed file, crates/analysis/src/tools.rs, against its surrounding code: try_map_on and control in parallel.rs and control.rs, overlay_zones_on, outside, and the sibling overlay tools on main. I could not compile or run the tests here (no shell), so the findings below rest on reading the code.

Bugs: none found. I checked:

  • Output order: try_map_on sorts results back into item order, so rests lines up with input_rows in the zip.
  • Row mapping: Erase drops features with no geometry and Update keeps them, both as before. Features missing from the dataset are skipped in both tools, as before.
  • Cancel and progress: try_map_on checks Cancel and advances progress per item on every thread, which replaces the old check/reached loop. Worker threads take over the run's control.
  • Cancellation on a small input: the single-thread path checks Cancel before each item, so the cancellation test still applies.

Security: none found.

Performance: none found. The PR's timing table (Erase 6.39 s to 0.73 s, Update 6.25 s to 0.72 s on 16 threads) is not verified here.

Quality (low confidence, for the release quality pass):

  • outside_each returns Vec<Option<Option<Geometry>>>. The three cases (no row or no geometry, nothing left, a remaining part) are only told apart by nesting, and both call sites rely on that. A small named enum would read better. This is a nit, not a defect.
  • outside_each uses threads_for, as the other overlay tools on main do, so it does not apply the threads_for_scratch / memory::thread_scratch budget that AGENTS.md asks for. Split is the one tool that does. This matches the existing overlay pattern rather than introducing a new gap, so it is worth a follow-up, not a change here.

AGENTS.md: the change follows the rules. Threads come from threads_for (one below PARALLEL_FEATURES and in the browser). Cancel and progress are handled per item by try_map_on. The doc comments state these points. The new test erase_and_update_on_several_threads_are_on_one compares every row's geometry and attributes on 1, 2, 7 and 16 threads, and covers polygons, lines and points, multi-part zones and features without geometry. No new command or tool was added, so the cancellation test's lists need no change.

I did not post inline comments because no finding warranted one. Prior review threads: none exist on this PR, so there were no earlier concerns to skip.

Change size

Compared with base: the whole PR against main (merge base 3da5b83 to PR head 33fed9c).

Measure Value
Files changed 1 (added 0, modified 1, removed 0, renamed 0)
Size before 706.0 KB
Size after 710.6 KB
Total size change +4.6 KB
Path Before After Change
crates/analysis/src/tools.rs 706.0 KB 710.6 KB +4.6 KB

@pick2510
pick2510 enabled auto-merge (squash) November 10, 2026 20:08
@pick2510
pick2510 merged commit b20364f into opengeos:main Nov 10, 2026
18 checks passed
@github-actions

Copy link
Copy Markdown
Contributor

📖 Docs preview

Item Value
Preview https://preview.rustgis.app/pr-1744/
Web app https://preview.rustgis.app/pr-1744/app/
Commit 33fed9c

GitHub Pages takes a minute or two to publish a new commit.

@pick2510

pick2510 commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor Author

Thanks. Declining both quality points:

  • The nested Option keeps the three cases in one line at each call site, and both call sites are next to the helper. A named enum adds a type for two uses.
  • threads_for_scratch bounds threads that each hold scratch memory (Zonal Statistics' raster masks). Erase and Update keep no per-thread scratch (|| ()), as the overlay tools don't: the memory a cut needs is per feature and freed with it, so threads_for is the right helper here.

(Edited: an earlier version wrongly said threads_for_scratch wasn't on main; I had checked a stale branch.)

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.

1 participant