-
Notifications
You must be signed in to change notification settings - Fork 424
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
Add parallel iterators for map.keys and map.values #26519
base: main
Are you sure you want to change the base?
Conversation
It doesn't look nearly identical to me, maybe because of the changes to |
That is correct, this PR predates that work and has not been updated to match yet |
Sure. I was just making a note since github repored that it wasn't going to cause a merge conflict; the pr description is now stale; and the state of the art in |
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
315ab99
to
0e79472
Compare
Signed-off-by: Jade Abraham <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, that looks good! To verify it's working the same as the Set improvement, it'd be nice to see:
- A test of zippering the keys of one map with the keys of a separate map of the same length (and/or the same thing but with values)
- A test of zippering either the keys or the values of a map with a set of the same length.
But I have no reason to believe those cases don't work today, since they're benefiting from the same code as Set.
Signed-off-by: Jade Abraham <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, thank you!
Adds the missing parallel iterators for
map.keys()
andmap.values()
.Resolves #24056
Implementation note: this implements the parallel iterators nearly identically to the ones for
set.these()
[Reviewed by @lydia-duncan]