Optimize filter expressions

[nikolai-mb]

Adds delegate caching for filter expressions using concurrent dictionaries for improved performance and reduced allocations.

Second commit is only related to in / nin filters and eliminates a 5.5 KB allocation that occurs every time the expression is applied by caching MethodInfo for invoking Expression.Call

Closes #6996

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.85%. Comparing base (398ae74) to head (4386959).

❗ Current head 4386959 differs from pull request most recent head de15f9b. Consider uploading reports for the commit de15f9b to get more accurate results

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #7005   +/-   ##
=======================================
  Coverage   69.84%   69.85%           
=======================================
  Files        2602     2602           
  Lines      130049   130056    +7     
=======================================
+ Hits        90836    90846   +10     
+ Misses      39213    39210    -3     

Flag	Coverage Δ
unittests	69.85% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[nikolai-mb]

Benchmarks

All benchmarks are performed by invoking the associated method in the FilterExpressionBuilder (such as Equals) using static parameters in order to measure the filter expressions in isolation

Commit be56f19

Applies to all filter operators except for list operators All / Any / Some / None

Avoids reflection by using cached delegates for creating the parameter expression for the value

Method	Mean	Error	StdDev	Ratio	Gen0	Allocated	Alloc Ratio
ContainsOld	645.6 ns	6.78 ns	5.66 ns	1.00	0.0134	744 B	1.00
ContainsNew	353.7 ns	1.06 ns	0.83 ns	0.55	0.0100	512 B	0.69
EqualsOld	377.4 ns	1.39 ns	1.09 ns	1.00	0.0095	488 B	1.00
EqualsNew	152.3 ns	0.57 ns	0.51 ns	0.40	0.0050	256 B	0.52
GreaterThanOld	374.2 ns	0.87 ns	0.73 ns	1.00	0.0095	488 B	1.00
GreaterThanNew	156.4 ns	0.86 ns	0.72 ns	0.42	0.0050	256 B	0.52
InOld	3,776.9 ns	20.80 ns	16.24 ns	1.00	0.1068	5840 B	1.00
InNew	3,603.7 ns	28.56 ns	25.31 ns	0.95	0.1068	5608 B	0.96
LowerThanOld	374.4 ns	2.12 ns	1.88 ns	1.00	0.0095	488 B	1.00
LowerThanNew	150.9 ns	0.20 ns	0.18 ns	0.40	0.0050	256 B	0.52
NotContainsOld	647.7 ns	1.20 ns	0.94 ns	1.00	0.0153	792 B	1.00
NotContainsNew	358.1 ns	0.86 ns	0.72 ns	0.55	0.0110	560 B	0.71
NotEqualsOld	377.8 ns	1.08 ns	0.90 ns	1.00	0.0095	488 B	1.00
NotEqualsNew	154.0 ns	0.34 ns	0.28 ns	0.41	0.0050	256 B	0.52
StartsWithOld	629.4 ns	2.56 ns	2.00 ns	1.00	0.0134	744 B	1.00
StartsWithNew	369.2 ns	3.82 ns	3.57 ns	0.59	0.0100	512 B	0.69

Commit 51ae3fe

Applies to in and nin operators

Avoids reflection by using cached delegates for creating the parameter expression for the value

Method	Mean	Error	StdDev	Ratio	Gen0	Allocated	Alloc Ratio
InOld	3,643.0 ns	40.09 ns	37.50 ns	1.00	0.1068	5608 B	1.00
InNew	231.1 ns	2.65 ns	2.48 ns	0.06	0.0057	288 B	0.05

Commit 5631694

Applies to list operators All / Any / Some / None

Simply removes an array allocation by using a Expression overload which accepts the parameters directly

Method	Mean	Error	StdDev	Ratio	RatioSD	Gen0	Allocated	Alloc Ratio
AllOld	256.3 ns	2.33 ns	2.18 ns	1.00	0.00	0.0062	312 B	1.00
AllNew	245.8 ns	0.97 ns	0.81 ns	0.96	0.01	0.0052	272 B	0.87
AnyOld	261.8 ns	3.56 ns	3.33 ns	1.00	0.00	0.0062	312 B	1.00
AnyNew	250.5 ns	3.31 ns	2.93 ns	0.96	0.02	0.0052	272 B	0.87

Omitted benchmarks for None / Some since they are just All / Any wrapped with a Not expression

Commit 01642d2

Applies to all operators when the filter value is null

Eliminates lambda expression allocation by caching the expression used to represent null for the given type

Method	Mean	Error	StdDev	Ratio	Gen0	Allocated	Alloc Ratio
EqNullOld	156.89 ns	0.876 ns	0.684 ns	1.00	0.0050	256 B	1.00
EqNullNew	23.90 ns	0.089 ns	0.079 ns	0.15	0.0008	40 B	0.16

Other operators omitted for brevity

Commit 6d0b9e8

Applies to all operators when the filter value is true or false

Eliminates lambda expression allocation by using a static constant for true and false

Method	Mean	Error	StdDev	Ratio	Gen0	Allocated	Alloc Ratio
EqBoolOld	161.38 ns	1.459 ns	1.293 ns	1.00	0.0050	256 B	1.00
EqBoolNew	25.28 ns	0.094 ns	0.083 ns	0.16	0.0008	40 B	0.16

Other operators omitted for brevity

[nikolai-mb]

Added two additional commits that cache the default expression for representing a null value for a given type as well as constant expressions for representing boolean true and false. This avoids a lambda expression allocation for these values when used in a filter

[PascalSenn]

Thanks @nikolai-mb for the pull request, very clean PR.

Just changed a few formatting things
@michaelstaib LGTM, if tests are green, lets go

[michaelstaib]

will go in today

[michaelstaib]

Hey @nikolai-mb,

I send this trough our tests and it breaks 78 tests ...

Have you run the data tests?

[michaelstaib]

Tests Fail

[michaelstaib]

It looks like the parameters get removed

  false SQL:
  ---------------
- .param set @__p_0 0
- 
  SELECT "d"."Id", "d"."Bar"
  FROM "Data" AS "d"
- WHERE "d"."Bar" = @__p_0
+ WHERE NOT ("d"."Bar")
  ---------------

[michaelstaib]

while this statement would still work other tests just completely break with no usable SQL anymore. Also we must ensure that even in this case we have a parametrized SQL statement.

[michaelstaib]

In order to run the tests you need to have docker running on your system.

[nikolai-mb]

Have you run the data tests?

No, i assumed the same tests would run as part of the PR pipeline. I've tested the code against a real API running sql server and EF Core and so i was confident in the code submitted. I'll have a look at the data-tests specifically when i have a minute, sorry for the inconvenience

[nikolai-mb]

Pushed two fixes, the first one fixes the lack of parameterization from #7005 (comment)

Second one fixes a Nullable<bool> == bool comparison which resulted in expressions that were invalid. Resolved this by introducing two additional expressions for nullable true and false.

There's some noise in my tests and several snapshot tests are failing caused only by CRLF / LF diff, not sure if that's the case for you. I currently have 12 failing tests, which is the same as i have in the main branch so i think everything related to this PR passes.

Apologies for overlooking this, could you do another sanity check and run the tests on your end with the latest changes?

[michaelstaib]

its the same tests. But somehow the build agent looks green.

[michaelstaib]

[michaelstaib]

@nikolai-mb thank you for your contribution!