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.

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

Reduce allocations in some Array.Parallel funcs #17505

Merged
merged 3 commits into from
Aug 9, 2024

Conversation

brianrourkeboll
Copy link
Contributor

@brianrourkeboll brianrourkeboll commented Aug 7, 2024

Description

Followup to #14827.

  • Avoid extra 'T option allocations in Array.Parallel.tryFindIndex, Array.Parallel.tryFind, and Array.Parallel.tryPick.

Checklist

  • Test cases added.
  • Performance benchmarks added in case of performance changes.
  • Release notes entry updated.

Copy link
Contributor

github-actions bot commented Aug 7, 2024

⚠️ Release notes required, but author opted out

Warning

Author opted out of release notes, check is disabled for this pull request.
cc @dotnet/fsharp-team-msft

@brianrourkeboll
Copy link
Contributor Author

Hmm, a3a50c0 reduces allocations significantly, but while it is faster in many scenarios, it is slower in others, depending on the size of the array and the predicate. I may take that change out, unless we know which scenarios we'd prefer to optimize for.

@brianrourkeboll
Copy link
Contributor Author

Hmm, a3a50c0 reduces allocations significantly, but while it is faster in many scenarios, it is slower in others, depending on the size of the array and the predicate. I may take that change out, unless we know which scenarios we'd prefer to optimize for.

Removed that for now.

@brianrourkeboll brianrourkeboll marked this pull request as ready for review August 8, 2024 12:59
@brianrourkeboll brianrourkeboll requested a review from a team as a code owner August 8, 2024 12:59
@psfinaki
Copy link
Member

psfinaki commented Aug 8, 2024

/azp run

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

Copy link
Member

@psfinaki psfinaki left a comment

Choose a reason for hiding this comment

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

Good stuff, thank you!

Copy link
Member

@KevinRansom KevinRansom left a comment

Choose a reason for hiding this comment

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

Looks good, however it needs the release notes updated.

@vzarytovskii vzarytovskii added the NO_RELEASE_NOTES Label for pull requests which signals, that user opted-out of providing release notes label Aug 9, 2024
@vzarytovskii vzarytovskii merged commit 97a2a75 into dotnet:main Aug 9, 2024
30 of 31 checks passed
@brianrourkeboll brianrourkeboll deleted the array-parallel-tweaks branch August 10, 2024 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NO_RELEASE_NOTES Label for pull requests which signals, that user opted-out of providing release notes
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants