Skip to content

perf: RBAC role building and Enforcer optimizations#178

Merged
EmperorYP7 merged 3 commits into
apache:masterfrom
sheny1xuan:add_scope_clean
Dec 21, 2021
Merged

perf: RBAC role building and Enforcer optimizations#178
EmperorYP7 merged 3 commits into
apache:masterfrom
sheny1xuan:add_scope_clean

Conversation

@sheny1xuan
Copy link
Copy Markdown
Contributor

Signed-off-by: stonex 1479765922@qq.com

Fixes #177

Description

  • Rbac AddNamedGroupingPolicy build role link by add role.
  • Enforcer hold a scope pointer rather than initalize scope when enforce.
  • Remove bindings directory.

Screenshots/Testimonials

  • Now, normal Enforce performance is almost determined by duktape.
    image
  • Current benchmark in my machine is below. Compared to before, it can improve approximately 1/3. Compared to before, RBAC model' performance problem maybe can't be solved by this pr. And I will do more profiling about RBAC model.
    image

stonex added 2 commits December 17, 2021 17:13
perf: Enforcer hold a scope pointer rather than initalize scope when enforce.

Signed-off-by: stonex <1479765922@qq.com>
Signed-off-by: stonex <1479765922@qq.com>
@casbin-bot
Copy link
Copy Markdown

@EmperorYP7 @divy9881 @noob20000405 please review

Copy link
Copy Markdown
Contributor

@EmperorYP7 EmperorYP7 left a comment

Choose a reason for hiding this comment

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

The only bottleneck we're left with is duktape itself. We may look into faster alternatives or a different approach altogether.

Comment thread casbin/model/scope_config.cpp Outdated
}

void CleanScope(Scope scope) {
std::vector<std::string> props = {"obj", "sub", "act"};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The props depend on config. It should be dynamic.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your review. I move clean_scope as a private member function in enforecer, because it based on model's config. And I exposed DeletePropFromObject for Enforcer to edit object in duktape. In this way, the clean step will be dynamic.

Comment thread casbin/model/scope_config.cpp Outdated

void CleanScope(Scope scope) {
std::vector<std::string> props = {"obj", "sub", "act"};
std::vector<std::string> objects = {"r", "p"};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Again, multiple definitions can be accommodated in a single config. eg: r1, r2, r3, p1, p2, p3 etc.

Comment thread bindings/CMakeLists.txt
@EmperorYP7 EmperorYP7 changed the title perf: rbac AddNamedGroupingPolicy build role link by add role, Enforcer hold a scope pointer rather than initalize scope when enforce. perf: RBAC role building and Enforcer optimizations Dec 17, 2021
Signed-off-by: stonex <1479765922@qq.com>
Copy link
Copy Markdown
Contributor

@EmperorYP7 EmperorYP7 left a comment

Choose a reason for hiding this comment

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

LGTM

@EmperorYP7 EmperorYP7 merged commit e9628f3 into apache:master Dec 21, 2021
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Dec 21, 2021

CLA assistant check
All committers have signed the CLA.

@github-actions
Copy link
Copy Markdown

🎉 This PR is included in version 1.44.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@hsluoyz
Copy link
Copy Markdown
Member

hsluoyz commented Dec 22, 2021

@sheny1xuan
Copy link
Copy Markdown
Contributor Author

@sheny1xuan
Copy link
Copy Markdown
Contributor Author

sheny1xuan commented Dec 23, 2021

The only bottleneck we're left with is duktape itself. We may look into faster alternatives or a different approach altogether.

I have searched lightweight cpp evalator in google, but I can't find suitbale replacement of duktape. Exprtk is most methioned, but it doesn't support accessor operator. But we can wrapper it by parsing the matcher string to proper expression. For example, we can replace . to _ and define a object's properties as different variables. Or we can directly make a lightweight library like govaluate. And I have made a plan to for the eval expression, and I have done the string token parse in this repo. What do you think, do you have any suggestions. @EmperorYP7 @hsluoyz

@ArashPartow
Copy link
Copy Markdown
Contributor

ArashPartow commented Jan 28, 2022

but it doesn't support accessor operator

That is not correct. What you call an "accessor" is merely adding a variable to the symbol_table by its given instance name.

For example if you have a struct called my data:

struct my_data
{
   double avalue;
   std::string astring;
};

then subsequently have an instance of my_data and use it in an expression like so:

typedef exprtk::symbol_table<T> symbol_table_t;
typedef exprtk::expression<T>   expression_t;
typedef exprtk::parser<T>       parser_t

my_data x;

symbol_table_t symbol_table;
symbol_table.add_variable("x.avalue" ,x.avalue);
symbol_table.add_string  ("x.astring",x.astring);

expression_t expression;
expression.register_symbol_table(symbol_table);

const std::string expression_string =
                  "(x.avalue /2 > 3) or (x.astring like '*cat on a mat*')";

parser_t parser;
parser.compile(expression_string,expression);

...

expression.value();

x.avalue = 17;

expression.value();

x.astring = "there is a cat on a mat with a hat";

expression.value();

@sheny1xuan
Copy link
Copy Markdown
Contributor Author

sheny1xuan commented Jan 28, 2022

x.avalue = 17;

Oh, thanks for your reply. I don't know this trick usage of Exprtk. I'll try to use it as the evalutor of casbin-cpp. Thanks for your reply and your great code.

@hsluoyz
Copy link
Copy Markdown
Member

hsluoyz commented Jan 28, 2022

@ArashPartow the original author of Exprtk has shown up, kudos!

@ArashPartow
Copy link
Copy Markdown
Contributor

@sheny1xuan More information and examples can be found here:

  1. https://github.com/ArashPartow/exprtk/blob/master/readme.txt#L786
  2. https://github.com/ArashPartow/exprtk/blob/master/exprtk_simple_example_01.cpp#L47

@sheny1xuan sheny1xuan deleted the add_scope_clean branch March 14, 2022 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Profiling] Identifying the bottlenecks in Enforcer

6 participants