Skip to content

Conversation

Scooletz
Copy link

@Scooletz Scooletz commented Mar 12, 2025

This PR combines gather/scatter fields in SafeFileHandle.ThreadPoolValueTaskSource into one. This saves one reference field in TPVTS.

@ghost ghost added the area-System.IO label Mar 12, 2025
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Mar 12, 2025
@Scooletz
Copy link
Author

This is my first PR to the mighty runtime 😅

Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

This is a performance optimization. Do you have any benchmark that shows this makes a difference?

@jkotas jkotas added the tenet-performance Performance related issue label Mar 12, 2025
@Scooletz
Copy link
Author

This is a performance optimization. Do you have any benchmark that shows this makes a difference?

Let me spin some tomorrow and share what it does.

@stephentoub
Copy link
Member

@Scooletz, were you able to gather any performance data demonstrating this is a net positive?

While this is shaving off a field, it's on an instance that's typically pooled, so in what's expected to be the common case, it's only saving a field on a single instance of a long-lived object. And in exchange it's paying for a cast on each use. It's not obvious to me this is the right trade-off.

@stephentoub stephentoub added the needs-author-action An issue or pull request that requires more info or actions from the author. label May 29, 2025
@Scooletz
Copy link
Author

I was not able to show anything measurable here. @stephentoub I think we can close it.

@Scooletz Scooletz closed this May 29, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Jun 29, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-System.IO community-contribution Indicates that the PR has been added by a community member needs-author-action An issue or pull request that requires more info or actions from the author. tenet-performance Performance related issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants