Reduce allocations and internal copies #17

JimBobSquarePants · 2025-07-16T07:59:30Z

Some more optimization work @stefannikolei

Precomputed list capacities to reduce dynamic resizing and allocations.
Applied the in modifier for readonly struct parameters to avoid unnecessary copies.
Switched from a binary to a 4-ary heap for the priority queue, matching .NET’s PriorityQueue design for improved cache locality and shallower tree depth.
Changed sweep event processing to accept unordered input, performing a single-pass heap construction (heapify) just before the sweep phase.

I also removed Clipper2 from the benchmarks as I was unable to get reliable output from the geometry.

Benchmarks.

BenchmarkDotNet v0.14.0, Windows 11 (10.0.26100.4652)
11th Gen Intel Core i7-11370H 3.30GHz, 1 CPU, 8 logical and 4 physical cores
.NET SDK 9.0.302
  [Host]     : .NET 9.0.7 (9.0.725.31616), X64 RyuJIT AVX-512F+CD+BW+DQ+VL+VBMI
  DefaultJob : .NET 9.0.7 (9.0.725.31616), X64 RyuJIT AVX-512F+CD+BW+DQ+VL+VBMI

Main

Method	Mean	Error	StdDev	Ratio	RatioSD	Gen0	Gen1	Allocated
PolygonClipper	44.23 us	0.246 us	0.218 us	2.60	0.02	6.7139	0.4883	41.34 KB

PR

Method	Mean	Error	StdDev	Ratio	RatioSD	Gen0	Gen1	Allocated
PolygonClipper	41.92 us	0.178 us	0.139 us	2.35	0.03	5.7983	0.3052	35.8 KB

Comparison to other language implementations

Method	File	Mean	Error	StdDev	Op/s	Gen0	Gen1	Gen2	Allocated
PolygonClipper	asia.geojson	29,136.01 us	280.796 us	234.477 us	34.32	1156.2500	937.5000	468.7500	7277.38 KB
PolygonClipper	hole_hole.geojson	12.49 us	0.130 us	0.122 us	80,061.19	1.7395	0.0305	-	10.7 KB
PolygonClipper	state(...)ojson [21]	1,485.58 us	6.812 us	5.318 us	673.14	95.7031	66.4063	-	597.38 KB

rust-geo-booleanop

We're around .05x - 2.3x faster than the Rust implementation.

benches/hole_hole/union time:   [29.927 µs 32.195 µs 34.983 µs]
benches/state_source/union  time:   [2.6862 ms 3.0259 ms 3.3598 ms]
benches/asia/union      time:   [30.689 ms 33.651 ms 36.414 ms]

Martinez

We're around 2.7x - 3.8x faster than the JavaScript implementation.

Hole_Hole
Martinez x 29,530 ops/sec ±1.65% (85 runs sampled)
JSTS x 2,051 ops/sec ±2.62% (85 runs sampled)
- Fastest is Martinez

Asia union
Martinez x 9.19 ops/sec ±3.30% (26 runs sampled)
JSTS x 7.60 ops/sec ±4.24% (23 runs sampled)
- Fastest is Martinez

States clip
Martinez x 227 ops/sec ±1.10% (82 runs sampled)
JSTS x 100 ops/sec ±2.54% (73 runs sampled)
- Fastest is Martinez

Copilot

Pull Request Overview

A concise description of the purpose of the PR, followed by summarized bullets of changes

Reduced dynamic allocations and struct copies by precomputing list capacities and using in parameters.
Switched the internal priority queue from a binary to a 4-ary heap with a one-pass heapify of unordered input.
Updated benchmarks and tests to reflect the new queue construction and removed Clipper2 dependencies.

Reviewed Changes

Copilot reviewed 18 out of 20 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/TestData/Benchmarks/hole_hole.geojson	Added new GeoJSON file for benchmark test cases.
tests/PolygonClipper.Tests/TestPolygonUtilitiesTests.cs	Removed Clipper2-specific tests and updated the union test to use profiled static polygons.
tests/PolygonClipper.Tests/TestPolygonUtilities.cs	Dropped `ConvertToClipper2Polygon` helper and refined GeoJSON→Polygon conversion with a closure check.
tests/PolygonClipper.Tests/TestData.cs	Introduced `Benchmarks` helper class to load GeoJSON files for benchmarks.
tests/PolygonClipper.Tests/PolygonClipper.Tests.csproj	Removed obsolete Clipper2 NuGet package reference.
tests/PolygonClipper.Benchmarks/PolygonClipper.Benchmarks.csproj	Removed Clipper2 reference and configured the benchmarks project.
tests/PolygonClipper.Benchmarks/Benches.cs	Added `Benches` class to run `BenchmarkDotNet` benchmarks over multiple GeoJSON inputs.
src/PolygonClipper/Vertex.cs	Added `in` modifiers to operator overloads and methods to avoid struct copies.
src/PolygonClipper/SweepEvent.cs	Changed `Below` and `Above` to accept `in Vertex p` parameters.
src/PolygonClipper/StatusLine.cs	Added capacity-taking constructor and preallocate list of sweep events.
src/PolygonClipper/StablePriorityQueue{T,TComparer}.cs	Converted to 4-ary heap, added `Log2Arity`, `Heapify`, and list-based initial construction.
src/PolygonClipper/SegmentComparer.cs	Removed redundant initializer for `inversed` flag.
src/PolygonClipper/Segment.cs	Added `in` modifiers to constructor and equality operators.
src/PolygonClipper/PolygonUtilities.cs	Added `in` modifiers to methods like `SignedArea`, `MidPoint`, and intersection helpers.
src/PolygonClipper/PolygonClipper.cs	Changed sweep setup to build an unordered event list, then heapify; preallocated status line.
src/PolygonClipper/Contour.cs	Updated `AddVertex` to take `in Vertex`.
src/PolygonClipper/Box2.cs	Updated constructors and operators to take `in Vertex`.

Comments suppressed due to low confidence (3)

src/PolygonClipper/StablePriorityQueue{T,TComparer}.cs:29

This constructor initializes heap but does not assign the Comparer property, which could lead to this.Comparer being null if this overload is used. Consider adding this.Comparer = comparer; or removing this overload if it’s no longer needed.

    public StablePriorityQueue(TComparer comparer, int capacity)

src/PolygonClipper/PolygonClipper.cs:389

[nitpick] The parameter named eventQueue is actually a List<SweepEvent> of unordered events, not a priority queue. Rename it to eventsList or unorderedEvents to reduce confusion.

        e1.ContourId = e2.ContourId = contourId;

tests/PolygonClipper.Tests/TestPolygonUtilitiesTests.cs:38

[nitpick] Test names should follow the [MethodUnderTest_StateUnderTest_ExpectedBehavior] pattern and avoid redundant _Test suffixes. Consider renaming this to something like Union_WithPrebuiltPolygons_ReturnsExpectedContours.

    public void PolygonClipper_Union_Profile_Test()

tests/PolygonClipper.Benchmarks/PolygonClipper.Benchmarks.csproj

stefannikolei

Should we add agressiveinlining to more getters in sweep event etc?

stefannikolei · 2025-07-16T13:02:34Z

src/PolygonClipper/PolygonClipper.cs

+        int clippingVertexCount = clipping.GetVertexCount();
+        int eventCount = (subjectVertexCount + clippingVertexCount) * 2;
+
+        SweepEventComparer comparer = new();


We could create a static comparer and then reuse it between runs?

We could yeah. Or we could make it a struct, however allocation-wise it's absolutely tiny.

stefannikolei · 2025-07-16T13:03:51Z

src/PolygonClipper/PolygonUtilities.cs

@@ -33,7 +33,7 @@ public static double SignedArea(Vertex p0, Vertex p1, Vertex p2)
    /// - Returns 1 if the segments intersect at a single point.
    /// - Returns 2 if the segments overlap.
    /// </returns>
-    public static int FindIntersection(Segment seg0, Segment seg1, out Vertex pi0, out Vertex pi1)
+    public static int FindIntersection(in Segment seg0, in Segment seg1, out Vertex pi0, out Vertex pi1)


Would using uint be beneficial here?

No, it's just a count of the number of intersections, if anything it could be an enum.

JimBobSquarePants added 4 commits July 16, 2025 14:05

Reduce allocations and internal copies

3085f3e

Merge branch 'main' into js/optimize-possible-intersection

c164036

Use 4ry heap with delayed sorting

0de9052

Update benchmarks

ee453aa

JimBobSquarePants requested a review from Copilot July 16, 2025 07:59

JimBobSquarePants added the performance label Jul 16, 2025

Copilot AI reviewed Jul 16, 2025

View reviewed changes

tests/PolygonClipper.Benchmarks/PolygonClipper.Benchmarks.csproj Show resolved Hide resolved

stefannikolei reviewed Jul 16, 2025

View reviewed changes

Cleanup

7889d92

stefannikolei approved these changes Jul 17, 2025

View reviewed changes

JimBobSquarePants merged commit 46f0eb1 into main Jul 17, 2025
7 checks passed

JimBobSquarePants deleted the js/optimize-possible-intersection branch July 23, 2025 00:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Reduce allocations and internal copies #17

Reduce allocations and internal copies #17

Uh oh!

JimBobSquarePants commented Jul 16, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

stefannikolei left a comment

Uh oh!

stefannikolei Jul 16, 2025

Uh oh!

JimBobSquarePants Jul 17, 2025

Uh oh!

stefannikolei Jul 16, 2025

Uh oh!

JimBobSquarePants Jul 17, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reduce allocations and internal copies #17

Reduce allocations and internal copies #17

Uh oh!

Conversation

JimBobSquarePants commented Jul 16, 2025

Benchmarks.

Comparison to other language implementations

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

stefannikolei left a comment

Choose a reason for hiding this comment

Uh oh!

stefannikolei Jul 16, 2025

Choose a reason for hiding this comment

Uh oh!

JimBobSquarePants Jul 17, 2025

Choose a reason for hiding this comment

Uh oh!

stefannikolei Jul 16, 2025

Choose a reason for hiding this comment

Uh oh!

JimBobSquarePants Jul 17, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!