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.

Sign up for GitHub

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

Jump to bottom

Improve performance when using large amount policy #39

Merged
merged 1 commit into from Jun 15, 2020
Merged

Improve performance when using large amount policy #39

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 9 additions & 4 deletions NetCasbin/CoreEnforcer.cs
View file
Open in desktop
Expand Up @@ -154,8 +154,13 @@ public void ClearPolicy()
/// </summary>
public void LoadPolicy()
{
if (adapter is null)
{
return;
}
model.ClearPolicy();
adapter.LoadPolicy(model);
model.RefreshPolicyStringSet();
if (autoBuildRoleLinks)
{
BuildRoleLinks();
Expand All @@ -174,7 +179,7 @@ public bool LoadFilteredPolicy(Filter filter)
{
throw new Exception("filtered policies are not supported by this adapter");
}
(adapter as IFilteredAdapter).LoadFilteredPolicy(model, filter);
(adapter as IFilteredAdapter)?.LoadFilteredPolicy(model, filter);
if (autoBuildRoleLinks)
{
BuildRoleLinks();
Expand All @@ -188,9 +193,9 @@ public bool LoadFilteredPolicy(Filter filter)
/// <returns>if the loaded policy has been filtered.</returns>
public bool IsFiltered()
{
if (adapter is IFilteredAdapter)
if (adapter is IFilteredAdapter filteredAdapter)
{
return (adapter as IFilteredAdapter).IsFiltered;
return filteredAdapter.IsFiltered;
}
return false;
}
Expand Down Expand Up @@ -397,7 +402,7 @@ private Interpreter GetAndInitializeInterpreter()
{
var key = entry.Key;
var ast = entry.Value;
var rm = ast.RM;
var rm = ast.RoleManager;
functions.Add(key, BuiltInFunctions.GenerateGFunction(key, rm));
}
}
Expand Down
14 changes: 5 additions & 9 deletions NetCasbin/Enforcer.cs
View file
Open in desktop
Expand Up @@ -35,11 +35,7 @@ public Enforcer(Model.Model m, IAdapter adapter)
fm = FunctionMap.LoadFunctionMap();

Initialize();

if (this.adapter != null)
{
LoadPolicy();
}
LoadPolicy();
}

public Enforcer(Model.Model m) :
Expand All @@ -58,19 +54,19 @@ public Enforcer(string modelPath, string policyFile, bool enableLog) : this(mode

public List<string> GetRolesForUser(string name)
{
return model.Model["g"]["g"].RM.GetRoles(name);
return model.Model["g"]["g"].RoleManager.GetRoles(name);
}

public List<string> GetUsersForRole(string name)
{
return model.Model["g"]["g"].RM.GetUsers(name);
return model.Model["g"]["g"].RoleManager.GetUsers(name);
}

public List<string> GetUsersForRoles(string[] names)
{
var userIds = new List<string>();
foreach (var name in names)
userIds.AddRange(model.Model["g"]["g"].RM.GetUsers(name));
userIds.AddRange(model.Model["g"]["g"].RoleManager.GetUsers(name));
return userIds;
}

Expand Down Expand Up @@ -238,7 +234,7 @@ public bool HasPermissionForUser(string user, List<string> permission)

public List<string> GetRolesForUserInDomain(string name, string domain)
{
return model.Model["g"]["g"].RM.GetRoles(name, domain);
return model.Model["g"]["g"].RoleManager.GetRoles(name, domain);
}

public List<List<string>> GetPermissionsForUserInDomain(string user, string domain)
Expand Down
36 changes: 20 additions & 16 deletions NetCasbin/Model/Assertion.cs
View file
Open in desktop
Expand Up @@ -2,6 +2,7 @@
using System;
using System.Collections.Generic;
using System.Linq;
using NetCasbin.Util;

namespace NetCasbin.Model
{
Expand All @@ -16,29 +17,33 @@ public class Assertion

public string[] Tokens { set; get; }

private List<List<string>> _policy;
public IRoleManager RoleManager { get; private set; }

private IRoleManager _rm;
public List<List<string>> Policy { set; get; }

public IRoleManager RM => _rm;
internal HashSet<string> PolicyStringSet { get; }

public List<List<string>> Policy
public Assertion()
{
set => _policy = value;
get => _policy;
Policy = new List<List<string>>();
PolicyStringSet = new HashSet<string>();
Copy link

Choose a reason for hiding this comment

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

The PolicyStringSet just used to store rule? what if a rule contains same string, like

p, alice, alice, read

Copy link
Member Author

@sagilio sagilio Jun 15, 2020 •
edited

Choose a reason for hiding this comment

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

It stores the string which connected by rule List<string>, I think it will keep the same operation as before.
this is operation before :

for (var i = 0; i < a.Count; i++)
{
    if (!a[i].Equals(b[i]))
    {
        return false;
    }
}
return true;

If a rule contains the same string and the HasPolicy will can not return false at before too. Whether the HashSet<string> is safe enough need the rule parsing is complete.

Copy link

Choose a reason for hiding this comment

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

Ok, understand, you turn an array into a string, so no need to worry about this.

Copy link
Member

Choose a reason for hiding this comment

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

Why we don't use HashSet<List<string>? somethings like:

 class ListPolicy<T> : List<T>
    {
        public override bool Equals(object obj)
        {
           // I'm not sure how to write.
            return obj is ListPolicy<T> policy &&
                   Capacity == policy.Capacity &&
                   Count == policy.Count;
        }

        public override int GetHashCode()
        {
            var hash = new HashCode();
            foreach (var item in this)
            {
                hash.Add(item);
            }
            return hash.ToHashCode();
        }
    }



    class MainClass
    {
        public static void Main(string[] args)
        {

            var policy = new HashSet<ListPolicy<string>>
            {
                new ListPolicy<string> { "user","data1","read" },
                new ListPolicy<string> { "user","data1","read" }
            };

            Console.WriteLine(policy.Count); // => 1
        }
    }

Copy link
Member Author

Choose a reason for hiding this comment

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

Why we don't use HashSet<List<string>? somethings like:

 class ListPolicy<T> : List<T>
    {
        public override bool Equals(object obj)
        {
           // I'm not sure how to write.
            return obj is ListPolicy<T> policy &&
                   Capacity == policy.Capacity &&
                   Count == policy.Count;
        }

        public override int GetHashCode()
        {
            var hash = new HashCode();
            foreach (var item in this)
            {
                hash.Add(item);
            }
            return hash.ToHashCode();
        }
    }



    class MainClass
    {
        public static void Main(string[] args)
        {

            var policy = new HashSet<ListPolicy<string>>
            {
                new ListPolicy<string> { "user","data1","read" },
                new ListPolicy<string> { "user","data1","read" }
            };

            Console.WriteLine(policy.Count); // => 1
        }
    }

I mentioned it in the next review. here is the content:

If changed List<List<string>> to Hash<Rule> (the Rule inherit List<string> and overwrite the Equals and GetHashCode), It will change almost all the public API and can not gain more performance. It will cost much time when Add the list because the Equal or Add has O(n) complexity.

Additionally, The HashSet can not get by index and will shuffle the list (But we can add the index property to handle it).

Copy link
Member Author

Choose a reason for hiding this comment

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

@nodece @xcaptain Here is the benchmark result of using HashSet<Rule>, the code is here change-to-hashset branch :

BenchmarkDotNet=v0.12.1, OS=Windows 10.0.19645
Intel Core i7-7700HQ CPU 2.80GHz (Kaby Lake), 1 CPU, 8 logical and 4 physical cores
.NET Core SDK=5.0.100-preview.5.20279.10
  [Host]     : .NET Core 3.1.2 (CoreCLR 4.700.20.6602, CoreFX 4.700.20.6702), X64 RyuJIT
  Job-WRAGMF : .NET Core 3.1.2 (CoreCLR 4.700.20.6602, CoreFX 4.700.20.6702), X64 RyuJIT

IterationCount=3  RunStrategy=Throughput
Method Times PolicyCount Mean Error StdDev Min Max Median
HasPolicy 1000 500 511.904 μs 331.9158 μs 18.1934 μs 492.169 μs 528.009 μs 515.534 μs
AddPolicy 1000 500 10,286.494 μs 1,392.4007 μs 76.3222 μs 10,239.692 μs 10,374.566 μs 10,245.225 μs
HasPolicy 1000 5000 523.930 μs 411.9189 μs 22.5787 μs 498.292 μs 540.851 μs 532.646 μs
AddPolicy 1000 5000 10,284.017 μs 5,746.3454 μs 314.9766 μs 9,943.827 μs 10,565.527 μs 10,342.697 μs

RoleManager = new DefaultRoleManager(0);
}

public Assertion()
public void RefreshPolicyStringSet()
{
_policy = new List<List<string>>();
_rm = new DefaultRoleManager(0);
PolicyStringSet.Clear();
foreach (var rule in Policy)
{
PolicyStringSet.Add(Utility.ArrayToString(rule));
}
}

public void BuildRoleLinks(IRoleManager rm)
public void BuildRoleLinks(IRoleManager roleManager)
{
_rm = rm;
RoleManager = roleManager;
var count = Value.ToCharArray().Count(x => x == '_');
foreach (var rule in _policy)
foreach (var rule in Policy)
{
if (count < 2)
{
Expand All @@ -51,18 +56,17 @@ public void BuildRoleLinks(IRoleManager rm)

if (count == 2)
{
rm.AddLink(rule[0], rule[1]);
roleManager.AddLink(rule[0], rule[1]);
}
else if (count == 3)
{
rm.AddLink(rule[0], rule[1], rule[2]);
roleManager.AddLink(rule[0], rule[1], rule[2]);
}
else if (count == 4)
{
rm.AddLink(rule[0], rule[1], rule[2], rule[3]);
roleManager.AddLink(rule[0], rule[1], rule[2], rule[3]);
}
}

}
}
}
65 changes: 41 additions & 24 deletions NetCasbin/Model/Policy.cs
View file
Open in desktop
@@ -1,6 +1,5 @@
using NetCasbin.Rbac;
using NetCasbin.Util;
using System;
using System.Collections.Generic;
using System.Linq;

Expand All @@ -14,28 +13,39 @@ public void BuildRoleLinks(IRoleManager rm)
{
if (Model.ContainsKey("g"))
{
foreach (var ast in Model["g"].Values)
foreach (Assertion ast in Model["g"].Values)
{
ast.BuildRoleLinks(rm);
}
}
}

public void RefreshPolicyStringSet()
{
foreach (Assertion ast in Model.Values
.SelectMany(pair => pair.Values))
{
ast.RefreshPolicyStringSet();
}
}

public void ClearPolicy()
{
if (Model.ContainsKey("p"))
{
foreach (var ast in Model["p"].Values)
foreach (Assertion ast in Model["p"].Values)
{
ast.Policy = new List<List<string>>();
ast.RefreshPolicyStringSet();
}
}

if (Model.ContainsKey("g"))
{
foreach (var ast in Model["p"].Values)
foreach (Assertion ast in Model["p"].Values)
{
ast.Policy = new List<List<string>>();
ast.RefreshPolicyStringSet();
}
}
}
Expand Down Expand Up @@ -73,29 +83,37 @@ public List<List<string>> GetFilteredPolicy(string sec, string ptype, int fieldI

public bool HasPolicy(string sec, string ptype, List<string> rule)
{
return Model[sec][ptype]!=null && Model[sec][ptype].Policy.Any(x => Utility.ArrayEquals(rule, x));
return Model[sec][ptype] != null && Model[sec][ptype].PolicyStringSet.Contains(Utility.ArrayToString(rule));
}

public bool AddPolicy(string sec, string ptype, List<string> rule)
{
if (!HasPolicy(sec, ptype, rule))
if (HasPolicy(sec, ptype, rule))
{
Model[sec][ptype].Policy.Add(rule);
return true;
return false;
}
return false;

Assertion assertion = Model[sec][ptype];
assertion.Policy.Add(rule);
Copy link

Choose a reason for hiding this comment

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

Can we merge these 2 steps into 1? We can compare 2 List, why change to string

Copy link
Member Author

Choose a reason for hiding this comment

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

If changed List<List<string>> to Hash<Rule> (the Rule inherit List<string> and overwrite the Equals and GetHashCode), It will change almost all the public API and can not gain more performance. It will cost much time when Add the list because the Equal or Add has O(n) complexity.

Copy link

@xcaptain xcaptain Jun 15, 2020 •
edited

Choose a reason for hiding this comment

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

If we have a lot of policies in memory, will this increase memory usage because of storing twice.

Copy link
Member Author

Choose a reason for hiding this comment

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

You are right, But it will not much increase memory and to file adapter it can be like a cache.
We can try to make it better, I think we can convert it to an array when using it.

Copy link
Member

Choose a reason for hiding this comment

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

the assertion.PolicyStringSet proptry does not need to be public.

  1. add assertion.AddPolicy(rule) method in Assertion Class replace these 2 steps:
assertion.Policy.Add(rule);
assertion.PolicyStringSet.Add(Utility.ArrayToString(rule));

  1. add assertion.Contains(rule) mehod in Assertion Class replace Model[sec][ptype].PolicyStringSet.Contains(Utility.ArrayToString(rule))

  2. add assertion.RemovePolicyAt(i) method replace these 2 steps:

assertion.Policy.RemoveAt(i);
assertion.PolicyStringSet.Remove(Utility.ArrayToString(rule));

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, The assertion.PolicyStringSet is Internal now, I will add them at next commit.

assertion.PolicyStringSet.Add(Utility.ArrayToString(rule));
return true;
}

public bool RemovePolicy(string sec, string ptype, List<string> rule)
{
for (var i = 0; i < Model[sec][ptype].Policy.Count; i++)
if (!HasPolicy(sec, ptype, rule))
{
var r = Model[sec][ptype].Policy[i];
if (Utility.ArrayEquals(rule, r))
{
Model[sec][ptype].Policy.RemoveAt(i);
return true;
}
return true;
}

Assertion assertion = Model[sec][ptype];
for (var i = 0; i < assertion.Policy.Count; i++)
{
var r = assertion.Policy[i];
if (!Utility.ArrayEquals(rule, r)) continue;
assertion.Policy.RemoveAt(i);
assertion.PolicyStringSet.Remove(Utility.ArrayToString(rule));
return true;
}
return false;
}
Expand All @@ -105,7 +123,8 @@ public bool RemoveFilteredPolicy(string sec, string ptype, int fieldIndex, param
var tmp = new List<List<string>>();
var res = false;

foreach (var rule in Model[sec][ptype].Policy)
Assertion assertion = Model[sec][ptype];
foreach (var rule in assertion.Policy)
{
var matched = true;
for (var i = 0; i < fieldValues.Length; i++)
Expand All @@ -128,18 +147,16 @@ public bool RemoveFilteredPolicy(string sec, string ptype, int fieldIndex, param
}
}

Model[sec][ptype].Policy = tmp;
assertion.Policy = tmp;
assertion.RefreshPolicyStringSet();
return res;
}

public List<string> GetValuesForFieldInPolicy(string sec, string ptype, int fieldIndex)
{
var values = new List<string>();

foreach (var rule in Model[sec][ptype].Policy)
{
values.Add(rule[fieldIndex]);
}
var values = Model[sec][ptype].Policy
.Select(rule => rule[fieldIndex])
.ToList();

Utility.ArrayRemoveDuplicates(values);
return values;
Expand Down
22 changes: 11 additions & 11 deletions NetCasbin/Persist/FileAdapter/DefaultFileAdapter.cs
View file
Open in desktop
@@ -1,10 +1,11 @@
using NetCasbin.Util;
using System;
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using NetCasbin.Model;
using NetCasbin.Util;

namespace NetCasbin.Persist.FileAdapter
{
Expand Down Expand Up @@ -85,20 +86,19 @@ public void RemoveFilteredPolicy(string sec, string ptype, int fieldIndex, param
throw new NotImplementedException("not implemented");
}

private List<string> GetModelPolicy(Model.Model model, string ptype)
private static IEnumerable<string> GetModelPolicy(Model.Model model, string ptype)
{
var policy = new List<string>();
model.Model[ptype].ToList().ForEach(item =>
foreach (var pair in model.Model[ptype])
{
var k = item.Key;
var v = item.Value;
var p = v.Policy.Select(x => $"{k}, {Utility.ArrayToString(x)}").ToList();
policy.AddRange(p);
});
var key = pair.Key;
Assertion value = pair.Value;
policy.AddRange(value.Policy.Select(p => $"{key}, {Utility.ArrayToString(p)}"));
}
return policy;
}

private void LoadPolicyData(Model.Model model, Helper.LoadPolicyLineHandler<string, Model.Model> handler, StreamReader inputStream)
private static void LoadPolicyData(Model.Model model, Helper.LoadPolicyLineHandler<string, Model.Model> handler, StreamReader inputStream)
{
while (!inputStream.EndOfStream)
{
Expand All @@ -107,7 +107,7 @@ private void LoadPolicyData(Model.Model model, Helper.LoadPolicyLineHandler<stri
}
}

private IList<string> ConvertToPolicy(Model.Model model)
private IEnumerable<string> ConvertToPolicy(Model.Model model)
{
if (_byteArrayInputStream != null && _readOnly)
{
Expand Down