fix: release the GIL while covering a polygon

[mailletf]

Problem

polygon_to_geohashes took _py: Python and never used it, so the entire cover walk ran with the GIL held. The walk touches no Python objects once __geo_interface__ has been read, and at fine precisions it runs for a long time — verdun at precision 10 takes minutes — blocking every other thread in the interpreter for the duration.

Every other heavy function in this module already detaches.

Fix

Read the geometry under the GIL as before, then py.detach for the walk.

Test

Two concurrent calls against one. Without the fix the pair reports 2.06× — fully serialised.

[mailletf]

This change is part of the following stack:

• fix: don't drop multipolygon parts whose seed lands in an earlier part #30
  • perf: cover polygons by descending the geohash tree #31
    • perf: expand geohash sets on packed integers instead of strings #32
      • fix: cap the expansion hop count instead of hanging near the poles #33
        • fix: release the GIL while covering a polygon #34 ◀
          • chore: delete the dead handbrake cover #35
            • chore: cargo fmt #36
              • perf: add Arrow-native WKB and EWKB output #37
                • perf: add Arrow-native encode and coordinate decode #38
                  • perf: optionally dictionary-encode geog_id in the Arrow mapping output #39
                  • perf: cover large polygons across Rayon workers #40

_{Change managed by git-spice.}

[Copilot]

Pull request overview

This PR updates the polygon_to_geohashes PyO3 binding to release the Python GIL during the (potentially long-running) polygon cover walk, and adds a regression test intended to detect accidental GIL retention by checking that two concurrent calls overlap rather than serialize.

Changes:

• Release the GIL for the polygons_to_geohashes cover walk by running it inside py.detach(...) after reading __geo_interface__.
• Add a new pytest that times one call vs two concurrent calls to validate that the cover walk does not hold the GIL.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
src/lib.rs	Detaches from the GIL during the cover walk to avoid blocking other Python threads.
tests/test_geohasher.py	Adds a timing-based concurrency test intended to verify GIL release behavior.

Suppressed comments (1)

tests/test_geohasher.py:241

• Exceptions raised inside the worker threads won't currently fail the test (they're swallowed by threading.Thread). Also, starting threads one-by-one can skew the concurrency timing. Consider synchronizing the start and capturing any thread exceptions explicitly.

    def work():
        geohash_polygon.polygon_to_geohashes(polygon_verdun, precision, False)

    threads = [threading.Thread(target=work) for _ in range(2)]
    start = time.perf_counter()

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[mailletf]

Fixed in the amended 5ca65fb, along with two more problems in the same test:

• Added @pytest.mark.skipif((os.cpu_count() or 1) < 2) — you are right that two CPU-bound threads on one core take ~2x whatever the GIL does.
• The workers now start behind a threading.Barrier. They were started one at a time, so the first had a head start and the pair looked more overlapped than it was.
• Worker exceptions are collected and asserted on. threading.Thread swallowed them, so a call that raised inside a worker would have passed on its (fast) timing.

The barrier tightened the measurement enough to expose a thin margin, so each timing now repeats the walk 10 times. Measured on this fixture: held GIL 1.97-2.21x, released 1.16-1.26x, against the 1.7x threshold. Verified the held-GIL numbers by building with the py.detach removed.

[mailletf]

Precision drops to 8 later in the stack (#40), for a different reason — above PARALLEL_COVER_MIN_CELLS one call already saturates every core and the test cannot tell a held GIL from a released one. The runtime concern lands in the same place. Full pytest run on this branch is ~10s.