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

[Authorization Logic // C++] Performance Improvement in Translation to Datalog #267

Open
aferr opened this issue Jan 19, 2022 · 4 comments

Comments

@aferr
Copy link
Collaborator

aferr commented Jan 19, 2022

cc: @markww

Improve performance by eliminating extra copies of predicates when translating from authorization logic to Datalog. This can be done by extending the dlir.h with a predicate modifier class like:

class PredicateModifier {
  /* ... */
  private:
  absl::string_view modifier_;
  Predicate wrapped_predicate_;
};

and by replacing occurances of Predicate in the dlir.h tree with another new class with a private member of type std::variant<Predicate, PredicateModifier.

The implementation of PushOntoPred of dlir.h would then need to be updated to to produce PredicateModifiers instead of just predicates.

@bgogul
Copy link
Collaborator

bgogul commented Jan 20, 2022

I don't have the complete context of the new code. However, holding on to absl::string_view has lifetime implications.

@aferr
Copy link
Collaborator Author

aferr commented Jan 20, 2022

In case it is helpful, the rest of the context is this thread

@aferr
Copy link
Collaborator Author

aferr commented Jan 20, 2022

ARGH, that link is useless, and there doesn't appear to be a way to get a useful one :( :)

@markww
Copy link
Contributor

markww commented Jan 21, 2022

Just to address the confusion here: when I was suggesting absl::string_view, I was mostly suggesting it for the parameters that lead up to the creating of Predicates (or soon-to-be PredicateModifiers) so you didn't have to wrap string literals in std::string constructor calls. Once you get to actually creating a long-lived object that refers to a string, I think it is appropriate to make PredicateModifier hold onto its own std::string to eliminate lifetime concerns, as @bgogul suggests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants