Skip to content

Feature/improve allocations#49

Merged
PhenX merged 5 commits into
mainfrom
feature/improve-allocations
Jun 1, 2025
Merged

Feature/improve allocations#49
PhenX merged 5 commits into
mainfrom
feature/improve-allocations

Conversation

@PhenX

@PhenX PhenX commented May 31, 2025

Copy link
Copy Markdown
Owner

No description provided.

@PhenX PhenX requested a review from SebastianStehle May 31, 2025 21:32

@SebastianStehle SebastianStehle left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good in general, but as mentioned a micro benchmark would be helpful. I cannot say, where exactly this would safe allocations. Perhaps less boxing? No idea.

Comment thread src/PhenX.EntityFrameworkCore.BulkInsert/Metadata/PropertyAccessor.cs Outdated
Comment thread src/PhenX.EntityFrameworkCore.BulkInsert/Metadata/PropertyAccessor.cs Outdated
Comment thread src/PhenX.EntityFrameworkCore.BulkInsert/Metadata/PropertyAccessor.cs Outdated
@PhenX

PhenX commented Jun 1, 2025

Copy link
Copy Markdown
Owner Author

The allocation that can be prevented with this PR is the one made for the closures here :

return source => converter(actualGetter(source));

The big change is the use of ConvertToProviderExpression instead of ConvertToProvider that makes it possible to compose the getter without any closure.

I'll try to make a micro benchmark

@PhenX

PhenX commented Jun 1, 2025

Copy link
Copy Markdown
Owner Author

@SebastianStehle here is the benchmark results, I think it will be hard to to better

image

@SebastianStehle

Copy link
Copy Markdown
Collaborator

Awesome, good job.

@PhenX PhenX merged commit c0e0c0e into main Jun 1, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants