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

Fix System.Data.Linq.Binary performance #3943

Closed
guillaume86 opened this issue Jan 30, 2023 · 4 comments · Fixed by #3944
Closed

Fix System.Data.Linq.Binary performance #3943

guillaume86 opened this issue Jan 30, 2023 · 4 comments · Fixed by #3944
Labels
status: has-pr There is active PR for issue type: bug
Milestone

Comments

@guillaume86
Copy link
Contributor

guillaume86 commented Jan 30, 2023

While benchmarking linq2db on the Northwind sample database vs Linq2SQL (a netcore port with some little fixes), I got disappointing results on some tables, and after investigation, I found the System.Data.Linq.Binary was the issue:

image

The Binary constructor eagearly computes the hash:


which is just wasted cycles if you don't end up using it. The class will compute the hash lazily when needed so it can safely be removed from the constructor if I'm not mistaken.

@MaceWindu
Copy link
Contributor

Why do you need Binary type?

@guillaume86
Copy link
Contributor Author

guillaume86 commented Jan 30, 2023 via email

@MaceWindu
Copy link
Contributor

PR is welcome. Should be easy fix. BTW, it copies netfx behavior, but of course we can improve it

@guillaume86
Copy link
Contributor Author

Thanks, will do. Yes I noticed it was original behavior, it was the netcore port I used as a baseline (https://github.com/mindbox-moscow/data-linq/blob/main/Mindbox.Data.Linq/Binary.cs) that had fixed it, probably because they noticed the same thing.

guillaume86 added a commit to guillaume86/linq2db that referenced this issue Jan 30, 2023
@MaceWindu MaceWindu added this to the 5.0.0 milestone Jan 30, 2023
@MaceWindu MaceWindu added status: has-pr There is active PR for issue and removed good first issue labels Jan 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: has-pr There is active PR for issue type: bug
Development

Successfully merging a pull request may close this issue.

2 participants