Closed sagilio closed 1 year ago
@sagilio fix:
@AsakusaRinne here is an issue about parallel enforce, plz have a look.
@sociometry Plz review the new changes.
@AsakusaRinne here is an issue about parallel enforce, plz have a look.
It's the GFunctionCache
that throws an exception saying "Operations that change non-concurrent collections must have exclusive access. A concurrent update was performed on this collection and corrupted its state. The collection's state is no longer correct". I'm a little confused that why the test cannot pass after the change of this commit. It seems that the logic about chache of enforcing was not changed.
I think there may be two ways to solve it:
GFunctionCache
to concurrent dictionary. There may be an impact on the efficiency.Parallel For
and cache the result together after Parallel For
. Some extra complexity will be introduced.The impact from using concurrent dictionary is listed below. Though it's small, the impact actually exists.
Benchmark with dictionary (.NET 7.0): | Method | Mean | Error | StdDev | Gen0 | Allocated |
---|---|---|---|---|---|---|
AbacModel | 156.2 ns | 1.80 ns | 1.19 ns | - | 32 B | |
BasicModel | 204.3 ns | 3.09 ns | 1.84 ns | - | 32 B | |
KeyMatchModel | 343.7 ns | 3.02 ns | 2.00 ns | 0.0010 | 336 B | |
PriorityModel | 246.9 ns | 2.48 ns | 1.30 ns | - | 120 B | |
RbacModel | 475.3 ns | 5.61 ns | 3.71 ns | 0.0010 | 296 B | |
RbacModelWithSmallScale | 11,559.2 ns | 95.83 ns | 57.02 ns | 0.0305 | 10016 B | |
RbacModelWithMediumScale | 113,268.4 ns | 1,778.27 ns | 1,176.21 ns | 0.3662 | 96424 B | |
RbacModelWithLargeScale | 1,172,893.2 ns | 17,688.30 ns | 11,699.71 ns | 1.9531 | 1039625 B | |
RbacModelWithResourceRoles | 281.5 ns | 2.75 ns | 1.82 ns | 0.0005 | 208 B | |
RbacModelWithDomains | 300.7 ns | 30.41 ns | 18.10 ns | 0.0005 | 144 B | |
RbacModelWithDeny | 717.7 ns | 10.85 ns | 7.18 ns | 0.0010 | 480 B |
Benchmark with concurrent dictionary (.NET 7.0): | Method | Mean | Error | StdDev | Gen0 | Allocated |
---|---|---|---|---|---|---|
AbacModel | 155.8 ns | 1.99 ns | 1.31 ns | - | 32 B | |
BasicModel | 204.0 ns | 2.74 ns | 1.81 ns | - | 32 B | |
KeyMatchModel | 342.9 ns | 2.93 ns | 1.94 ns | 0.0010 | 336 B | |
PriorityModel | 247.9 ns | 3.40 ns | 2.25 ns | - | 120 B | |
RbacModel | 470.4 ns | 3.98 ns | 2.63 ns | 0.0010 | 296 B | |
RbacModelWithSmallScale | 11,902.6 ns | 122.37 ns | 80.94 ns | 0.0305 | 10016 B | |
RbacModelWithMediumScale | 115,028.2 ns | 897.78 ns | 593.82 ns | 0.3662 | 96424 B | |
RbacModelWithLargeScale | 1,272,455.1 ns | 10,652.52 ns | 7,045.99 ns | 3.9063 | 1039625 B | |
RbacModelWithResourceRoles | 287.3 ns | 5.29 ns | 2.77 ns | 0.0005 | 208 B | |
RbacModelWithDomains | 291.0 ns | 5.14 ns | 3.40 ns | 0.0005 | 144 B | |
RbacModelWithDeny | 998.7 ns | 124.33 ns | 82.24 ns | 0.0010 | 480 B |
@sagilio I don't think we should merge this PR, given it still has CI erros: https://github.com/casbin/Casbin.NET/tree/preview
@sagilio I don't think we should merge this PR, given it still has CI erros: https://github.com/casbin/Casbin.NET/tree/preview
This is a compatibility issue caused by .NET 7 RC1 released 3 days ago (.NET 7 is preview now, not released yet), with no effects on other.NET versions.
And it will be fixed by this PR.
:tada: This PR is included in version 2.0.0-preview.5 :tada:
The release is available on GitHub release
Your semantic-release bot :package::rocket:
:tada: This PR is included in version 2.0.0 :tada:
The release is available on GitHub release
Your semantic-release bot :package::rocket:
:tada: This PR is included in version 2.0.0 :tada:
The release is available on GitHub release
Your semantic-release bot :package::rocket:
Continue with #275