Skip to content

Filter a unique list in place in List.keep_if - #12645

Merged
Anton-4 merged 5 commits into
roc-lang:mainfrom
braedenfoster:keep-if-in-place
Oct 30, 2026
Merged

Anton-4 merged 5 commits into
roc-lang:mainfrom
braedenfoster:keep-if-in-place

Conversation

@braedenfoster

Copy link
Copy Markdown
Contributor

Fixes #12644.

List.keep_if, drop_if, keep_oks and keep_errs were a fold over List.concat(acc, [item]), so each kept item allocated a one-item list and the result grew by reallocating. This moves them onto the reuse pattern that List.map (#9610, #10594) and sort_impl use.

Changes

keep_if

  • Unique list: list_map_prepare_reuse, then list_owned_unique. Each kept item is swapped down to the end of the kept prefix with list_swap_unsafe, then List.drop_last releases the dropped items. Order is kept, each dropped item is released once, capacity is unchanged, and an all-kept list comes back as it was. No allocation.
  • Shared list: items are tested up to the first dropped one. If none is dropped, the input is returned as it is. Otherwise the kept items are copied with list_append_unsafe into one List.with_capacity(len) buffer, allocated when the first kept item needs a place, so keeping nothing allocates nothing. The predicate still runs once per item.

drop_if calls keep_if with the predicate negated.

keep_oks and keep_errs build their result in one buffer allocated on the first match. They do not work in place: when before and after share a layout it would be possible, but the moved-out tail slots would need a primitive that shortens a list without releasing them. That seems better as a follow-up.

The in-place path uses only list_swap_unsafe and drop_last. listSwap copies a shared list unless the call is marked in place, so a wrong uniqueness answer costs a copy rather than a wrong result (the #10368 failure mode).

The second commit (all-kept shortcut and lazy allocation on the shared path) can be dropped or split out if you prefer a smaller change.

Allocation calls

roc_alloc plus roc_realloc made by the call alone, 12-item list, test/alloc-count, same for --opt=dev and --opt=speed:

Call Before After
keep_if, all kept 16 0
keep_if, 6 of 12 kept 9 0
keep_if, none kept 0 0
drop_if, none dropped 16 0
keep_oks, all Ok 16 1

Tests

  • src/eval/test/eval_tests.zig: order kept; all, none and some kept; empty and emptied lists; a unique seamless slice; a shared list used again afterwards; a predicate that captures the list being filtered; heap Str items; drop_if, keep_oks and keep_errs; allocation counts.
  • src/eval/test/rc_conformance_tests.zig: keep_if, drop_if and keep_oks over unique and shared Str lists.
  • test/alloc-count/app_list_keep_if.roc with dev and speed CLI entries.
  • 8 snapshots changed only in Builtin node ids.

The full eval suite, rc_conformance and the Builtin doc tests pass locally.

@braedenfoster
braedenfoster marked this pull request as ready for review October 26, 2026 20:12
keep_if, drop_if, keep_oks and keep_errs folded over
List.concat(acc, [item]), so keeping 12 items cost 13 allocations and 4
reallocations. keep_if now follows List.map. A unique list is compacted in
place with list_swap_unsafe and truncated with List.drop_last, so it
allocates nothing and keeps its capacity, as its docs promise. A shared
list is copied into one List.with_capacity(len) allocation. drop_if calls
keep_if.

keep_oks and keep_errs change the item type, so they build their result in
one List.with_capacity(len) allocation. An in-place version would need a
primitive that shortens a list without releasing the slots its items were
moved out of.

The Builtin snapshot node ids moved with the new definitions.
A shared or static list went through one List.with_capacity(len)
allocation even when keep_if kept every item or none. The shared path now
tests items until the first dropped one and returns the input when there is
none. Otherwise it copies the kept items into a new list that it allocates
only when the first kept item needs a place, so keeping nothing allocates
nothing. keep_oks and keep_errs also allocate on their first kept value.

The predicate still runs once per item: the copy loop does not test the
items before the first dropped one again.
@Anton-4 Anton-4 self-assigned this Oct 27, 2026
@Anton-4

Anton-4 commented Oct 27, 2026

Copy link
Copy Markdown
Collaborator

Thanks @braedenfoster :)
Just a heads-up, I plan to do some minor cleanup on this tomorrow.

@Anton-4 Anton-4 changed the title Filter a unique list in place in List.keep_if WIP Filter a unique list in place in List.keep_if Oct 27, 2026
@braedenfoster

Copy link
Copy Markdown
Contributor Author

Thanks @braedenfoster :)
Just a heads-up, I plan to do some minor cleanup on this tomorrow.

Awesome thanks 🙏

Anton-4-bot and others added 2 commits October 28, 2026 16:17
The shared path now allocates its List.with_capacity(len) copy up front,
appends the items before the first dropped one with list_append_sublist,
and tests only the items after it. This removes the per-item index check
and the lazy-allocation test from the copy loop. Keeping nothing from a
shared list whose first item is dropped now allocates once, which matches
the documented "same memory as the original" behavior.

The unique path skips list_swap_unsafe when an item is already in place.

Also: document the shared-list copy in the keep_if docs, fix the seamless
slice test comment, prefix the alloc-count CLI cases with the issue number,
and fix a missing space in the rc_conformance test.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Regenerated the dev_object_float_math hashes, which both sides changed.

@Anton-4 Anton-4 left a comment •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks @braedenfoster! It's nice to have these optimized 🚀

@Anton-4 Anton-4 changed the title WIP Filter a unique list in place in List.keep_if Filter a unique list in place in List.keep_if Oct 29, 2026
@Anton-4
Anton-4 merged commit 0602c5f into roc-lang:main Oct 30, 2026
27 checks passed
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.

List.keep_if allocates once per kept item instead of filtering in place

3 participants