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

Fixing problem with "new Guid()" inside LINQ expressions. #633

Merged
merged 1 commit into from
Nov 17, 2021

Conversation

s-KaiNet
Copy link
Collaborator

This PR fixes #632

The idea is to compile lambda expression with new Guid() and invoke compiled delegate to get a real Guid instance:

Expression.Lambda<Func<Guid>>(expression).Compile().Invoke();

I searched for any info about Expression.Compile() performance and found this SO answer, which indicates that performance is very good (600-700ms for 2M of iterations). I checked results on my own environment for .NET 5 and received similar results. Invoking a compiled delegate is also fast.

@codecov-commenter
Copy link

Codecov Report

Merging #633 (7454eec) into dev (c13436b) will increase coverage by 6.89%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #633      +/-   ##
==========================================
+ Coverage   75.32%   82.22%   +6.89%     
==========================================
  Files         257      411     +154     
  Lines       19708    28266    +8558     
==========================================
+ Hits        14846    23242    +8396     
- Misses       4862     5024     +162     
Impacted Files Coverage Δ
...del/SharePoint/Core/Internal/FieldLocationValue.cs 0.00% <0.00%> (-35.49%) ⬇️
...ore.Auth/Utilities/ApplicationBuilderExtensions.cs 62.06% <0.00%> (-26.17%) ⬇️
...h/Public/UsernamePasswordAuthenticationProvider.cs 68.60% <0.00%> (-25.52%) ⬇️
...P.Core/QueryModel/Query/BaseDataModelExtensions.cs 38.88% <0.00%> (-23.07%) ⬇️
...k/PnP.Core/Model/Teams/Internal/TeamIdentitySet.cs 50.00% <0.00%> (-16.67%) ⬇️
src/sdk/PnP.Core/Utilities/StringExtensions.cs 85.07% <0.00%> (-14.93%) ⬇️
.../Model/Teams/Internal/TeamChatMessageAttachment.cs 85.71% <0.00%> (-14.29%) ⬇️
...e/Model/SharePoint/Core/Internal/StorageMetrics.cs 77.77% <0.00%> (-9.73%) ⬇️
...rc/sdk/PnP.Core/Services/Core/Query/QueryClient.cs 77.65% <0.00%> (-8.20%) ⬇️
....Core/QueryModel/Query/DataModelQueryTranslator.cs 77.61% <0.00%> (-7.11%) ⬇️
... and 277 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 621061f...7454eec. Read the comment docs.

@jansenbe jansenbe self-assigned this Nov 17, 2021
@jansenbe jansenbe added area: model 📐 Related to the core SDK models area: framework ⚙ Changes in the SDK core framework code and removed area: model 📐 Related to the core SDK models labels Nov 17, 2021
@jansenbe
Copy link
Contributor

Nice improvement @s-KaiNet 💪Also like the fact you've spend time on thinking about perf.

jansenbe added a commit that referenced this pull request Nov 17, 2021
@jansenbe jansenbe merged commit 5caca97 into pnp:dev Nov 17, 2021
@s-KaiNet s-KaiNet deleted the guid-fix branch April 15, 2022 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: framework ⚙ Changes in the SDK core framework code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

System.NotSupportedException when trying to LINQ by Guid id
3 participants