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

Checker performance improvements #346

Closed
TristonianJones opened this issue Apr 30, 2020 · 2 comments
Closed

Checker performance improvements #346

TristonianJones opened this issue Apr 30, 2020 · 2 comments
Assignees
Labels
enhancement New feature or request

Comments

@TristonianJones
Copy link
Collaborator

TristonianJones commented Apr 30, 2020

In assessing Checker performance there were three factors which dominated performance:

  • proto.Equal (46%)
  • ANTLR Lexer / Parser initialization (33%)
  • checker declaration conflict detection (17%)
  • other (4%)

The dominant use of proto.Equal was actually introduced in #250 to fix an infinite recursion bug. The second issue was introduced in #177 to address data races within the ANTLR runtime.

Upon further review, the use of proto.Equal in #250 could be much more narrowly scoped and the substitution logic simplified, thus significantly improving the expression compilation performance within larger applications like https://github.com/google/cel-policy-templates-go.

Before:

BenchmarkCompiler_Template/canonical-8         	      12	  96157074 ns/op	26354824 B/op	  683280 allocs/op

After:

BenchmarkCompiler_Template/canonical-8         	      19	  57289719 ns/op	21251336 B/op	  407321 allocs/op
@TristonianJones TristonianJones added the enhancement New feature or request label Apr 30, 2020
@TristonianJones TristonianJones self-assigned this Apr 30, 2020
@TristonianJones TristonianJones changed the title Checker performance degradation due to overly broad proto.Equal use Checker performance improvements May 1, 2020
@TristonianJones
Copy link
Collaborator Author

Note, I've submitted PR antlr/antlr4#2816 to make ANTLR's Go codegen like what we've hacked together in CEL, and to export a method that will allow us to use ANTLR Lexer / Parser objects within a sync.Pool in order amortize the construction cost while still being thread-safe.

@TristonianJones
Copy link
Collaborator Author

The bulk of performance improvements came with #520

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant