Skip to content

[v17] Tweak sortcache.SortCache API #54778

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 16, 2025
Merged

Conversation

rosstimothy
Copy link
Contributor

Backport #52414 and a portion of #54395 to branch/v17

- Adds (SortCache) Clear to allow resetting an existing cache
- Converts (SortCache) Ascend and (SortCache) Descend to return
  an iter.Seq[T] instead of taking an iteration function
- Removes (SortCache) AscendPaginated and (SortCache) DescendPaginated

Leveraging iterates aligns the API with the direction that Go is
heading in regards to iteration. Removing of the paginated functions
does put the onus on callers to paginate, however, in the only
cases these were used there are now fewer allocations. AscendPaginated
was unused and DescendPaginated was only used in two places - both
of which would benefit from migrating away from using streams in
favor of iter.Seq.

The new Clear API makes it easier for callers to reset an existing
cache. Without this change the only way to reset a cache would be
to replace the entire cache with a new one. This places the burden
on callers to apply locking and handle concurrent read/write/deletes
and swapping out the cache entirely. That all is eliminated by
Clear handling the internal locking to reset the cache state.
A new type constraint was added to the sort cache to make indexes
type safe. The types are limited to `comparable` so the indexes can
be used as map keys internally. The primary motivation for the
change is to leverage the compiler to prevent using indexes which
are not valid for a given cache.

While it's still technically possible to use a string literal as
the index if the type index is constrained to `~string` it should
be avoided to ensure the compiler can catch invalid indexes.
@rosstimothy rosstimothy added backport no-changelog Indicates that a PR does not require a changelog entry labels May 13, 2025
@rosstimothy rosstimothy marked this pull request as ready for review May 14, 2025 14:17
@github-actions github-actions bot requested review from avatus, fspmarshall and zmb3 May 14, 2025 14:17
@rosstimothy rosstimothy enabled auto-merge May 15, 2025 11:21
@rosstimothy rosstimothy requested a review from espadolini May 16, 2025 14:01
@rosstimothy rosstimothy added this pull request to the merge queue May 16, 2025
Merged via the queue into branch/v17 with commit 71a17a9 May 16, 2025
45 checks passed
@rosstimothy rosstimothy deleted the tross/backport-52414/v17 branch May 16, 2025 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport no-changelog Indicates that a PR does not require a changelog entry size/sm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants