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

feat: support cancellation/timeouts on condition evaluation #1237

Merged
merged 16 commits into from
Jan 3, 2024

Conversation

jpadilla
Copy link
Member

@jpadilla jpadilla commented Dec 15, 2023

Description

In aa117f1, I added two temporary benchmark suites to get an idea of how long evaluations might take based on the expression + parameters supplied. Notice the p99(ns) column.

$ go test ./internal/condition -bench ^BenchmarkEvaluate -test.count=10 -timeout 0 -run=XXX -benchmem
goos: darwin
goarch: arm64
pkg: github.com/openfga/openfga/internal/condition
BenchmarkEvaluate-10                           	 1497010	       811.8 ns/op	     66392 p99(ns)	     440 B/op	       7 allocs/op
BenchmarkEvaluate-10                           	 1488909	       797.9 ns/op	     62086 p99(ns)	     440 B/op	       7 allocs/op
BenchmarkEvaluate-10                           	 1515388	       793.4 ns/op	     59103 p99(ns)	     440 B/op	       7 allocs/op
BenchmarkEvaluate-10                           	 1519147	       792.3 ns/op	     60268 p99(ns)	     440 B/op	       7 allocs/op
BenchmarkEvaluate-10                           	 1495501	       791.6 ns/op	     61239 p99(ns)	     440 B/op	       7 allocs/op
BenchmarkEvaluate-10                           	 1519246	       793.6 ns/op	     59887 p99(ns)	     440 B/op	       7 allocs/op
BenchmarkEvaluate-10                           	 1508084	       805.4 ns/op	     65923 p99(ns)	     440 B/op	       7 allocs/op
BenchmarkEvaluate-10                           	 1502704	       797.7 ns/op	     63425 p99(ns)	     440 B/op	       7 allocs/op
BenchmarkEvaluate-10                           	 1487602	       795.0 ns/op	     61058 p99(ns)	     440 B/op	       7 allocs/op
BenchmarkEvaluate-10                           	 1498320	       799.1 ns/op	     62269 p99(ns)	     440 B/op	       7 allocs/op
BenchmarkEvaluateComprehensionIterations-10    	   28660	     38706 ns/op	    331240 p99(ns)	   24792 B/op	     732 allocs/op
BenchmarkEvaluateComprehensionIterations-10    	   31036	     38456 ns/op	    242090 p99(ns)	   24791 B/op	     732 allocs/op
BenchmarkEvaluateComprehensionIterations-10    	   31383	     38306 ns/op	    227997 p99(ns)	   24791 B/op	     732 allocs/op
BenchmarkEvaluateComprehensionIterations-10    	   31207	     38479 ns/op	    302734 p99(ns)	   24791 B/op	     732 allocs/op
BenchmarkEvaluateComprehensionIterations-10    	   31215	     38321 ns/op	    194209 p99(ns)	   24792 B/op	     732 allocs/op
BenchmarkEvaluateComprehensionIterations-10    	   31353	     38387 ns/op	    218893 p99(ns)	   24791 B/op	     732 allocs/op
BenchmarkEvaluateComprehensionIterations-10    	   31309	     39251 ns/op	   1090966 p99(ns)	   24791 B/op	     732 allocs/op
BenchmarkEvaluateComprehensionIterations-10    	   31428	     39416 ns/op	   1257040 p99(ns)	   24791 B/op	     732 allocs/op
BenchmarkEvaluateComprehensionIterations-10    	   30876	     39960 ns/op	   1729277 p99(ns)	   24791 B/op	     732 allocs/op
BenchmarkEvaluateComprehensionIterations-10    	   31045	     38503 ns/op	    271264 p99(ns)	   24791 B/op	     732 allocs/op
PASS
ok  	github.com/openfga/openfga/internal/condition	36.263s

References

Review Checklist

  • I have clicked on "allow edits by maintainers".
  • I have added documentation for new/changed functionality in this PR or in a PR to openfga.dev [Provide a link to any relevant PRs in the references section above]
  • The correct base branch is being used, if not main
  • I have added tests to validate that the change in functionality is working as expected

@jpadilla jpadilla requested a review from a team as a code owner December 15, 2023 17:26
Copy link

codecov bot commented Dec 15, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (68dfc32) 82.70% compared to head (76bf23e) 82.67%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1237      +/-   ##
==========================================
- Coverage   82.70%   82.67%   -0.02%     
==========================================
  Files          84       84              
  Lines        9746     9749       +3     
==========================================
  Hits         8059     8059              
- Misses       1332     1334       +2     
- Partials      355      356       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jpadilla jpadilla requested a review from a team as a code owner December 20, 2023 18:40
@jon-whit jon-whit merged commit ee1cdd6 into main Jan 3, 2024
13 of 14 checks passed
@jon-whit jon-whit deleted the condition-eval-with-context branch January 3, 2024 21:43
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.

None yet

3 participants