Use foreach and sizeHint in mapAccumulate - #85
Open
cheeseng wants to merge 1 commit into
Open
Conversation
The manual iterator/while loop offered no benefit over foreach here — no early exit, and foreach/iterator are guaranteed to visit elements in the same order for a given collection, so the sequential state threading through f is unaffected by the switch. Use foreach for native per-collection traversal instead of an allocated Iterator plus two virtual calls (hasNext/next) per element. Also add b.sizeHint(this): unlike updatedWith, mapAccumulate always produces exactly one B per input element, so the result size is known exactly in advance rather than just estimated. No behavior changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
mapAccumulate traversed via a manual
val it = iterator; while (it.hasNext) { ... it.next() ... }loop,even though nothing about the method needs iterator-specific
capability — no early exit, and every element is visited
unconditionally.
Switch to foreach, which dispatches to each collection's own native
traversal (e.g. List walking cons cells directly, index-based loops
for array-backed types) instead of allocating an Iterator and paying
two virtual calls (hasNext/next) per element.
This is safe for mapAccumulate specifically because the state (S) is
threaded sequentially through calls to f in traversal order, and
foreach/iterator are guaranteed to visit a given collection's
elements in the same order as each other — so the switch doesn't
change which elements state gets threaded through in which order.
Also add b.sizeHint(this): mapAccumulate always produces exactly one
B per input element, so unlike some other methods in this file where
a sizeHint would only be a guess, here the result size is known
exactly in advance and the builder can size itself accordingly.
No behavior changes.