Skip to content

Reduce allocations in some Array.Parallel funcs#17505

Merged
vzarytovskii merged 3 commits into
dotnet:mainfrom
brianrourkeboll:array-parallel-tweaks
Aug 9, 2024
Merged

Reduce allocations in some Array.Parallel funcs#17505
vzarytovskii merged 3 commits into
dotnet:mainfrom
brianrourkeboll:array-parallel-tweaks

Conversation

@brianrourkeboll

@brianrourkeboll brianrourkeboll commented Aug 7, 2024

Copy link
Copy Markdown
Contributor

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.

@github-actions

github-actions Bot commented Aug 7, 2024

Copy link
Copy Markdown
Contributor

⚠️ 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
Copy Markdown
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
Copy Markdown
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

psfinaki commented Aug 8, 2024

Copy link
Copy Markdown
Contributor

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 2 pipeline(s).

@psfinaki psfinaki left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good stuff, thank you!

Comment thread src/FSharp.Core/array.fs

@KevinRansom KevinRansom left a comment

Copy link
Copy Markdown
Contributor

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
@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