Skip to content

ContainsCursor: name each arrayHas case and document the invariant by example#46

Merged
aliszka merged 1 commit into
mainfrom
refactor/contains-cursor-cases
Jul 7, 2026
Merged

ContainsCursor: name each arrayHas case and document the invariant by example#46
aliszka merged 1 commit into
mainfrom
refactor/contains-cursor-cases

Conversation

@amourao

@amourao amourao commented Jul 3, 2026

Copy link
Copy Markdown

Follow-up to the #44 review discussion on arrayHas readability.

What

No behavioral change. The switch now has one case per probe shape, and the doc carries a worked example:

Invariant: arrPos is the lower bound of the previous probe. Example with
values c = [10 20 30 40] after a previous probe of 25 (arrPos = 2, c[2] = 30):

  y = 30          hit under the cursor          -> true, no search
  y = 31..39      forward of the cursor         -> advanceUntil from index 2
  y = 21..29      inside the gap (20, 30)       -> absent, one compare (20 < y)
  y <= 20         at or behind the previous gap -> full binary search

Why not the boundary-case formulation

The suggested y < c[0] / y > c[N-1] / y == c[i] / y > c[i] / find shape was considered and deliberately not used:

  • y < c[0] and y > c[N-1] are already answered by the cursor cases without touching the array edges: i == 0 with c[i] > y is "y precedes everything", and i == arrN with c[i-1] < y is "y beyond the last value". Reading c[N-1] explicitly would add a potentially cold cache-line access to every probe (the whole point of the cursor is staying on the warm neighbourhood).
  • Dropping the in-gap case would forfeit the main win: for ascending probes with step below the container's mean value gap, ~half of all probes are misses landing in the gap right below the cursor. Sending those to find is what the original code did — restoring it would take smallgap from ~5.3 back to ~15+ ns/op.
  • The gap check cannot "go on and on" (c[i-2] < y < c[i-1], …): the single gap below the cursor is special because the invariant brackets it with one compare that's already in cache. Locating which earlier gap holds y is precisely the binary search — those cases are, collectively, the default: find branch.

Verification

Equivalence + edge tests + -race + full suite green; benchmarks unchanged (smallgap 5.28 ns/op, biggap 21.8, random parity).

… example

Review follow-up on #44: the combined gap guard (i == 0 || c[i-1] < y) packed
two distinct shapes into one condition, and nothing explained why exactly one
predecessor compare suffices. Split the switch into single-purpose cases
(hit / forward / before-first / in-gap / behind-gap) and add a worked example
to the doc showing where each probe shape lands, plus why the single-gap check
cannot be extended to earlier gaps (locating which earlier gap holds y is the
binary search itself) and why explicit y < c[0] / y > c[N-1] boundary cases
are not needed (both are answered from the cursor's neighbours without
re-reading the array edges).

No behavioral change; benchmarks unchanged (smallgap ~5.3 ns/op).

@orca-security-eu orca-security-eu Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Orca Security Scan Summary

Status Check Issues by priority
Passed Passed Infrastructure as Code high 0   medium 0   low 0   info 0 View in Orca
Passed Passed SAST high 0   medium 0   low 0   info 0 View in Orca
Passed Passed Secrets high 0   medium 0   low 0   info 0 View in Orca
Passed Passed Vulnerabilities high 0   medium 0   low 0   info 0 View in Orca

@aliszka aliszka merged commit f5fae1b into main Jul 7, 2026
6 checks passed
@aliszka aliszka deleted the refactor/contains-cursor-cases branch July 7, 2026 11:49
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.

2 participants