Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[stdlib] Optimize _StringSliceIter to not have branching in forward iteration #3546

Open
wants to merge 19 commits into
base: nightly
Choose a base branch
from
Open

[stdlib] Optimize _StringSliceIter to not have branching in forward iteration #3546

wants to merge 19 commits into from

Conversation

martinvuyk
Copy link
Contributor

@martinvuyk martinvuyk commented Sep 24, 2024 •
edited
Loading

Optimize _StringSliceIter to not have branching in forward iteration.

This also fixes an error in the iterator's __len__ logic which assumed it would be called only at the beginning of the iteration

@martinvuyk
optimize _StringSliceIter
c890b7d 
Signed-off-by: martinvuyk <[email protected]>
@martinvuyk
fix __len__
9754306 
Signed-off-by: martinvuyk <[email protected]>
@martinvuyk
mojo format
fd15159 
Signed-off-by: martinvuyk <[email protected]>
@martinvuyk
fix detail
73b83e8 
Signed-off-by: martinvuyk <[email protected]>
@martinvuyk
fix detail
4e758d3 
Signed-off-by: martinvuyk <[email protected]>
@martinvuyk
Merge remote-tracking branch 'upstream/nightly' into optimize-strings…
d2d0fe8 
…liceiter
@martinvuyk
fix detail
64f5318 
Signed-off-by: martinvuyk <[email protected]>
@martinvuyk martinvuyk mentioned this pull request Sep 25, 2024
Open
1 task
@martinvuyk martinvuyk marked this pull request as ready for review October 20, 2024 22:38
@martinvuyk martinvuyk requested a review from a team as a code owner October 20, 2024 22:38
JoeLoser
JoeLoser reviewed Nov 4, 2024
View reviewed changes
Copy link
Collaborator

@JoeLoser JoeLoser left a comment

Choose a reason for hiding this comment

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

For the part about

This also fixes an error in the iterator's len logic which assumed it would be called only at the beginning of the iteration

Can we add a unit test case that gets fixed as a result of your changes?

@martinvuyk
add iterator len test
e9b79c9 
Signed-off-by: martinvuyk <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JoeLoser JoeLoser Awaiting requested review from JoeLoser

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@martinvuyk @JoeLoser