chore: Benchmarks for ListObjects #2584
Open
+278
−0
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughA comprehensive benchmark suite was added to Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2584 +/- ##
==========================================
+ Coverage 90.39% 90.42% +0.04%
==========================================
Files 140 140
Lines 19683 19683
==========================================
+ Hits 17790 17796 +6
+ Misses 1442 1438 -4
+ Partials 451 449 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
86487b2 to
cb5e366
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pkg/server/commands/list_objects_test.go(3 hunks)
🧰 Additional context used
🪛 GitHub Actions: Pull Request
pkg/server/commands/list_objects_test.go
[error] 734-734: Benchmark test 'BenchmarkListObjects/weight_three_with_optimization' failed. See error trace at line 734.
[error] 743-743: Benchmark test 'BenchmarkListObjects/weight_three_without_optimization' failed. See error trace at line 743.
🔇 Additional comments (3)
pkg/server/commands/list_objects_test.go (3)
9-9: LGTM: Import additions are appropriate.The new imports are necessary for the benchmark functionality:
ulid/v2for unique ID generationparserfor DSL-to-proto transformationtuplefor creating TupleKey objectsAlso applies to: 17-17, 29-29
583-641: Benchmark setup looks comprehensive and well-structured.The setup correctly:
- Creates an in-memory datastore with proper cleanup
- Defines a realistic authorization model with different relationship weights
- Uses ULIDs for unique identifiers
- Sets up typesystem context and check resolver
- Configures unlimited max results for consistent benchmarking
755-772: Good approach to handle unsupported recursive optimizations.The commented-out recursive benchmark with optimizations is appropriate since the optimization currently falls back to non-optimized code. The TODO comment clearly indicates future work, and running only the non-optimized version provides a baseline for comparison.
Comment on lines
+583
to
+856
| // b.Run("recursive_ttu_with_optimizations", func(b *testing.B) { | ||
| // query.optimizationsEnabled = true | ||
| // for i := 0; i < b.N; i++ { | ||
| // _, err := query.Execute(ctx, recursiveRequest) | ||
| // require.NoError(b, err) | ||
| // } | ||
| // }) | ||
|
|
||
| b.Run("recursive_ttu_without_optimizations", func(b *testing.B) { | ||
| query.optimizationsEnabled = false | ||
| for i := 0; i < b.N; i++ { | ||
| res, err := query.Execute(ctx, recursiveRequest) | ||
| require.NoError(b, err) | ||
| require.Len(b, res.Objects, n) | ||
| } | ||
| }) | ||
| } | ||
|
|
||
| // This helper writes tuples for user:justin with relation "member" to org:0...org:10000. | ||
| func createDirectWeightOneRelations( | ||
| b *testing.B, | ||
| ctx context.Context, | ||
| datastore storage.OpenFGADatastore, | ||
| storeID string, | ||
| numTuples int, | ||
| ) { | ||
| b.Helper() | ||
| for objID := 0; objID < numTuples; objID++ { | ||
| tuples := make([]*openfgav1.TupleKey, datastore.MaxTuplesPerWrite()) | ||
|
|
||
| for j := 0; j < datastore.MaxTuplesPerWrite(); j++ { | ||
| obj := "org:" + strconv.Itoa(objID) | ||
| tuples[j] = tuple.NewTupleKey(obj, "member", "user:justin") | ||
| objID++ | ||
| } | ||
| err := datastore.Write(ctx, storeID, nil, tuples) | ||
| require.NoError(b, err) | ||
| } | ||
| } | ||
|
|
||
| func createWeightTwoRelations(b *testing.B, ctx context.Context, datastore storage.OpenFGADatastore, storeID string, numTuples int) { | ||
| b.Helper() | ||
| for objID := 0; objID < numTuples; objID++ { | ||
| tuples := make([]*openfgav1.TupleKey, datastore.MaxTuplesPerWrite()) | ||
|
|
||
| for j := 0; j < datastore.MaxTuplesPerWrite(); j++ { | ||
| // These IDs can be the same as we already created org:0 - org:numTuples | ||
| obj := "company:" + strconv.Itoa(objID) | ||
| user := "org:" + strconv.Itoa(objID) | ||
| tuples[j] = tuple.NewTupleKey(obj, "owner", user) | ||
| objID++ | ||
| } | ||
| err := datastore.Write(ctx, storeID, nil, tuples) | ||
| require.NoError(b, err) | ||
| } | ||
| } | ||
|
|
||
| func createWeightThreeRelations(b *testing.B, ctx context.Context, datastore storage.OpenFGADatastore, storeID string, numTuples int) { | ||
| b.Helper() | ||
| for objID := 0; objID < numTuples; objID++ { | ||
| tuples := make([]*openfgav1.TupleKey, datastore.MaxTuplesPerWrite()) | ||
|
|
||
| for j := 0; j < datastore.MaxTuplesPerWrite(); j++ { | ||
| // These IDs can be the same as we already created org:0 - org:numTuples | ||
| obj := "office:" + strconv.Itoa(objID) | ||
| user := "company:" + strconv.Itoa(objID) | ||
| tuples[j] = tuple.NewTupleKey(obj, "parent", user) | ||
| objID++ | ||
| } | ||
| err := datastore.Write(ctx, storeID, nil, tuples) | ||
| require.NoError(b, err) | ||
| } | ||
| } | ||
|
|
||
| func createRecursiveRelations(b *testing.B, ctx context.Context, datastore storage.OpenFGADatastore, storeID string, numTuples int) { | ||
| b.Helper() | ||
| for objID := 0; objID < numTuples; objID++ { | ||
| tuples := make([]*openfgav1.TupleKey, datastore.MaxTuplesPerWrite()) | ||
|
|
||
| for j := 0; j < datastore.MaxTuplesPerWrite(); j++ { | ||
| // Every 10th item, make user:justin the leaf | ||
| if j%10 == 0 { | ||
| obj := "org:" + strconv.Itoa(objID) | ||
| tk := tuple.NewTupleKey(obj, "recursive", "user:justin") | ||
| tuples[j] = tk | ||
| tuples = append(tuples, tk) | ||
| objID++ | ||
| continue | ||
| } | ||
|
|
||
| // otherwise, chain the org#parent#org relation | ||
| obj := "org:" + strconv.Itoa(objID) | ||
| user := "org:" + strconv.Itoa(objID-1) | ||
| tuples[j] = tuple.NewTupleKey(obj, "parent", user) | ||
| objID++ | ||
| } | ||
| err := datastore.Write(ctx, storeID, nil, tuples) | ||
| require.NoError(b, err) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Excellent benchmark design with implementation issues to fix.
The benchmark suite design is comprehensive and valuable for performance testing:
✅ Strengths:
- Tests different relationship complexity weights (1, 2, 3, INF)
- Compares optimized vs non-optimized implementations
- Uses realistic authorization models
- Proper benchmark patterns with setup/cleanup
- Clear documentation and comments
❌ Critical Issues:
- Loop logic errors in all helper functions causing incorrect tuple counts
- This explains the pipeline failures in weight_three benchmarks
Once the helper function fixes are applied, this will provide excellent benchmark coverage for ListObjects performance optimization work.
🧰 Tools
🪛 GitHub Actions: Pull Request
[error] 734-734: Benchmark test 'BenchmarkListObjects/weight_three_with_optimization' failed. See error trace at line 734.
[error] 743-743: Benchmark test 'BenchmarkListObjects/weight_three_without_optimization' failed. See error trace at line 743.
🤖 Prompt for AI Agents
In pkg/server/commands/list_objects_test.go from lines 583 to 856, the helper
functions creating tuples for the benchmark have loop logic errors causing
incorrect tuple counts and pipeline failures. To fix this, remove the increment
of objID inside the inner loop so that objID increments only once per outer loop
iteration. Also, ensure tuples slice is correctly sized and not appended to
inside the loop. This will correctly batch tuples per write and produce the
intended number of tuples for each benchmark weight.
Comment on lines
+721
to
+745
| weightThreeRequest := &openfgav1.ListObjectsRequest{ | ||
| StoreId: storeID, | ||
| AuthorizationModelId: model.GetId(), | ||
| Type: "office", | ||
| Relation: "weight_three", | ||
| User: "user:justin", | ||
| } | ||
|
|
||
| b.Run("weight_three_with_optimization", func(b *testing.B) { | ||
| query.optimizationsEnabled = true | ||
| for i := 0; i < b.N; i++ { | ||
| res, err := query.Execute(ctx, weightThreeRequest) | ||
| require.NoError(b, err) | ||
| require.Len(b, res.Objects, n) | ||
| } | ||
| }) | ||
|
|
||
| b.Run("weight_three_without_optimization", func(b *testing.B) { | ||
| query.optimizationsEnabled = false | ||
| for i := 0; i < b.N; i++ { | ||
| res, err := query.Execute(ctx, weightThreeRequest) | ||
| require.NoError(b, err) | ||
| require.Len(b, res.Objects, n) | ||
| } | ||
| }) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Investigate weight_three benchmark failures.
The pipeline indicates these benchmarks are failing. This likely stems from issues in the createWeightThreeRelations helper function or the authorization model definition.
Based on pipeline failures, please verify the weight_three benchmark setup:
🏁 Script executed:
#!/bin/bash
# Verify the authorization model and tuple creation logic for weight_three relations
# Check the model definition for office type
ast-grep --pattern $'type office
relations
$$$'
# Look for any issues in createWeightThreeRelations function
ast-grep --pattern $'func createWeightThreeRelations($$$) {
$$$
}'Length of output: 1479
I wasn’t able to locate a “type office” definition in the test—only the helper that writes tuples for weight_three relations appeared. It looks like the authorization model for “office” never declares the “weight_three” relation (or “parent” at all), so the tuples you’re writing don’t match anything and the benchmark fails.
Please add or correct the type office { … relations { weight_three: user } } block in the model setup before invoking createWeightThreeRelations.
🧰 Tools
🪛 GitHub Actions: Pull Request
[error] 734-734: Benchmark test 'BenchmarkListObjects/weight_three_with_optimization' failed. See error trace at line 734.
[error] 743-743: Benchmark test 'BenchmarkListObjects/weight_three_without_optimization' failed. See error trace at line 743.
🤖 Prompt for AI Agents
In pkg/server/commands/list_objects_test.go around lines 721 to 745, the
benchmark test uses a "type office" with a "weight_three" relation, but the
authorization model setup lacks a definition for this type and relation. To fix
this, update the model setup to include a "type office" declaration with a
"weight_three: user" relation before calling createWeightThreeRelations,
ensuring the tuples written match the model and the benchmark runs correctly.
Comment on lines
+775
to
+795
| // This helper writes tuples for user:justin with relation "member" to org:0...org:10000. | ||
| func createDirectWeightOneRelations( | ||
| b *testing.B, | ||
| ctx context.Context, | ||
| datastore storage.OpenFGADatastore, | ||
| storeID string, | ||
| numTuples int, | ||
| ) { | ||
| b.Helper() | ||
| for objID := 0; objID < numTuples; objID++ { | ||
| tuples := make([]*openfgav1.TupleKey, datastore.MaxTuplesPerWrite()) | ||
|
|
||
| for j := 0; j < datastore.MaxTuplesPerWrite(); j++ { | ||
| obj := "org:" + strconv.Itoa(objID) | ||
| tuples[j] = tuple.NewTupleKey(obj, "member", "user:justin") | ||
| objID++ | ||
| } | ||
| err := datastore.Write(ctx, storeID, nil, tuples) | ||
| require.NoError(b, err) | ||
| } | ||
| } |
There was a problem hiding this comment.
Critical issue: Incorrect loop logic in tuple creation.
The nested loop structure has a fundamental flaw. The objID is incremented in both the outer loop condition and the inner loop body, causing:
- Fewer tuples to be created than expected
- Inconsistent object ID sequences
- Potential out-of-bounds conditions
Apply this fix to correct the loop logic:
-func createDirectWeightOneRelations(
- b *testing.B,
- ctx context.Context,
- datastore storage.OpenFGADatastore,
- storeID string,
- numTuples int,
-) {
- b.Helper()
- for objID := 0; objID < numTuples; objID++ {
- tuples := make([]*openfgav1.TupleKey, datastore.MaxTuplesPerWrite())
-
- for j := 0; j < datastore.MaxTuplesPerWrite(); j++ {
- obj := "org:" + strconv.Itoa(objID)
- tuples[j] = tuple.NewTupleKey(obj, "member", "user:justin")
- objID++
- }
- err := datastore.Write(ctx, storeID, nil, tuples)
- require.NoError(b, err)
- }
-}
+func createDirectWeightOneRelations(
+ b *testing.B,
+ ctx context.Context,
+ datastore storage.OpenFGADatastore,
+ storeID string,
+ numTuples int,
+) {
+ b.Helper()
+ objID := 0
+ for objID < numTuples {
+ batchSize := min(datastore.MaxTuplesPerWrite(), numTuples-objID)
+ tuples := make([]*openfgav1.TupleKey, batchSize)
+
+ for j := 0; j < batchSize; j++ {
+ obj := "org:" + strconv.Itoa(objID)
+ tuples[j] = tuple.NewTupleKey(obj, "member", "user:justin")
+ objID++
+ }
+ err := datastore.Write(ctx, storeID, nil, tuples)
+ require.NoError(b, err)
+ }
+}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In pkg/server/commands/list_objects_test.go around lines 775 to 795, the nested
loops incorrectly increment objID in both the outer and inner loops, leading to
fewer tuples and inconsistent object IDs. Fix this by removing the objID
increment from the inner loop and only incrementing it in the outer loop,
ensuring the correct number of tuples are created with sequential object IDs.
Comment on lines
+797
to
+812
| func createWeightTwoRelations(b *testing.B, ctx context.Context, datastore storage.OpenFGADatastore, storeID string, numTuples int) { | ||
| b.Helper() | ||
| for objID := 0; objID < numTuples; objID++ { | ||
| tuples := make([]*openfgav1.TupleKey, datastore.MaxTuplesPerWrite()) | ||
|
|
||
| for j := 0; j < datastore.MaxTuplesPerWrite(); j++ { | ||
| // These IDs can be the same as we already created org:0 - org:numTuples | ||
| obj := "company:" + strconv.Itoa(objID) | ||
| user := "org:" + strconv.Itoa(objID) | ||
| tuples[j] = tuple.NewTupleKey(obj, "owner", user) | ||
| objID++ | ||
| } | ||
| err := datastore.Write(ctx, storeID, nil, tuples) | ||
| require.NoError(b, err) | ||
| } | ||
| } |
There was a problem hiding this comment.
Same loop logic issue as previous helper function.
This function has the identical nested loop problem with objID being incremented incorrectly.
Apply the same fix pattern:
-func createWeightTwoRelations(b *testing.B, ctx context.Context, datastore storage.OpenFGADatastore, storeID string, numTuples int) {
- b.Helper()
- for objID := 0; objID < numTuples; objID++ {
- tuples := make([]*openfgav1.TupleKey, datastore.MaxTuplesPerWrite())
-
- for j := 0; j < datastore.MaxTuplesPerWrite(); j++ {
- // These IDs can be the same as we already created org:0 - org:numTuples
- obj := "company:" + strconv.Itoa(objID)
- user := "org:" + strconv.Itoa(objID)
- tuples[j] = tuple.NewTupleKey(obj, "owner", user)
- objID++
- }
- err := datastore.Write(ctx, storeID, nil, tuples)
- require.NoError(b, err)
- }
-}
+func createWeightTwoRelations(b *testing.B, ctx context.Context, datastore storage.OpenFGADatastore, storeID string, numTuples int) {
+ b.Helper()
+ objID := 0
+ for objID < numTuples {
+ batchSize := min(datastore.MaxTuplesPerWrite(), numTuples-objID)
+ tuples := make([]*openfgav1.TupleKey, batchSize)
+
+ for j := 0; j < batchSize; j++ {
+ obj := "company:" + strconv.Itoa(objID)
+ user := "org:" + strconv.Itoa(objID)
+ tuples[j] = tuple.NewTupleKey(obj, "owner", user)
+ objID++
+ }
+ err := datastore.Write(ctx, storeID, nil, tuples)
+ require.NoError(b, err)
+ }
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| func createWeightTwoRelations(b *testing.B, ctx context.Context, datastore storage.OpenFGADatastore, storeID string, numTuples int) { | |
| b.Helper() | |
| for objID := 0; objID < numTuples; objID++ { | |
| tuples := make([]*openfgav1.TupleKey, datastore.MaxTuplesPerWrite()) | |
| for j := 0; j < datastore.MaxTuplesPerWrite(); j++ { | |
| // These IDs can be the same as we already created org:0 - org:numTuples | |
| obj := "company:" + strconv.Itoa(objID) | |
| user := "org:" + strconv.Itoa(objID) | |
| tuples[j] = tuple.NewTupleKey(obj, "owner", user) | |
| objID++ | |
| } | |
| err := datastore.Write(ctx, storeID, nil, tuples) | |
| require.NoError(b, err) | |
| } | |
| } | |
| func createWeightTwoRelations(b *testing.B, ctx context.Context, datastore storage.OpenFGADatastore, storeID string, numTuples int) { | |
| b.Helper() | |
| objID := 0 | |
| for objID < numTuples { | |
| batchSize := min(datastore.MaxTuplesPerWrite(), numTuples-objID) | |
| tuples := make([]*openfgav1.TupleKey, batchSize) | |
| for j := 0; j < batchSize; j++ { | |
| obj := "company:" + strconv.Itoa(objID) | |
| user := "org:" + strconv.Itoa(objID) | |
| tuples[j] = tuple.NewTupleKey(obj, "owner", user) | |
| objID++ | |
| } | |
| err := datastore.Write(ctx, storeID, nil, tuples) | |
| require.NoError(b, err) | |
| } | |
| } |
🤖 Prompt for AI Agents
In pkg/server/commands/list_objects_test.go around lines 797 to 812, the nested
loop incorrectly increments objID inside the inner loop, causing it to skip
values and misalign tuple creation. To fix this, remove the objID++ increment
from inside the inner loop and instead increment objID only once per outer loop
iteration, ensuring each objID is used correctly for tuple creation.
Comment on lines
+814
to
+829
| func createWeightThreeRelations(b *testing.B, ctx context.Context, datastore storage.OpenFGADatastore, storeID string, numTuples int) { | ||
| b.Helper() | ||
| for objID := 0; objID < numTuples; objID++ { | ||
| tuples := make([]*openfgav1.TupleKey, datastore.MaxTuplesPerWrite()) | ||
|
|
||
| for j := 0; j < datastore.MaxTuplesPerWrite(); j++ { | ||
| // These IDs can be the same as we already created org:0 - org:numTuples | ||
| obj := "office:" + strconv.Itoa(objID) | ||
| user := "company:" + strconv.Itoa(objID) | ||
| tuples[j] = tuple.NewTupleKey(obj, "parent", user) | ||
| objID++ | ||
| } | ||
| err := datastore.Write(ctx, storeID, nil, tuples) | ||
| require.NoError(b, err) | ||
| } | ||
| } |
There was a problem hiding this comment.
Root cause of pipeline failures: Incorrect loop logic.
This function contains the same loop logic error that's causing the weight_three benchmark failures. The incorrect objID incrementing means fewer than 10,000 tuples are created, causing the require.Len(b, res.Objects, n) assertions to fail.
Apply the corrected loop logic:
-func createWeightThreeRelations(b *testing.B, ctx context.Context, datastore storage.OpenFGADatastore, storeID string, numTuples int) {
- b.Helper()
- for objID := 0; objID < numTuples; objID++ {
- tuples := make([]*openfgav1.TupleKey, datastore.MaxTuplesPerWrite())
-
- for j := 0; j < datastore.MaxTuplesPerWrite(); j++ {
- // These IDs can be the same as we already created org:0 - org:numTuples
- obj := "office:" + strconv.Itoa(objID)
- user := "company:" + strconv.Itoa(objID)
- tuples[j] = tuple.NewTupleKey(obj, "parent", user)
- objID++
- }
- err := datastore.Write(ctx, storeID, nil, tuples)
- require.NoError(b, err)
- }
-}
+func createWeightThreeRelations(b *testing.B, ctx context.Context, datastore storage.OpenFGADatastore, storeID string, numTuples int) {
+ b.Helper()
+ objID := 0
+ for objID < numTuples {
+ batchSize := min(datastore.MaxTuplesPerWrite(), numTuples-objID)
+ tuples := make([]*openfgav1.TupleKey, batchSize)
+
+ for j := 0; j < batchSize; j++ {
+ obj := "office:" + strconv.Itoa(objID)
+ user := "company:" + strconv.Itoa(objID)
+ tuples[j] = tuple.NewTupleKey(obj, "parent", user)
+ objID++
+ }
+ err := datastore.Write(ctx, storeID, nil, tuples)
+ require.NoError(b, err)
+ }
+}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In pkg/server/commands/list_objects_test.go around lines 814 to 829, the
function createWeightThreeRelations incorrectly increments objID inside the
inner loop, causing fewer tuples than expected to be created. To fix this,
remove the objID++ increment from inside the inner loop and instead increment
objID only in the outer loop, ensuring the correct total number of tuples are
generated and the benchmark assertions pass.
Comment on lines
+831
to
+856
| func createRecursiveRelations(b *testing.B, ctx context.Context, datastore storage.OpenFGADatastore, storeID string, numTuples int) { | ||
| b.Helper() | ||
| for objID := 0; objID < numTuples; objID++ { | ||
| tuples := make([]*openfgav1.TupleKey, datastore.MaxTuplesPerWrite()) | ||
|
|
||
| for j := 0; j < datastore.MaxTuplesPerWrite(); j++ { | ||
| // Every 10th item, make user:justin the leaf | ||
| if j%10 == 0 { | ||
| obj := "org:" + strconv.Itoa(objID) | ||
| tk := tuple.NewTupleKey(obj, "recursive", "user:justin") | ||
| tuples[j] = tk | ||
| tuples = append(tuples, tk) | ||
| objID++ | ||
| continue | ||
| } | ||
|
|
||
| // otherwise, chain the org#parent#org relation | ||
| obj := "org:" + strconv.Itoa(objID) | ||
| user := "org:" + strconv.Itoa(objID-1) | ||
| tuples[j] = tuple.NewTupleKey(obj, "parent", user) | ||
| objID++ | ||
| } | ||
| err := datastore.Write(ctx, storeID, nil, tuples) | ||
| require.NoError(b, err) | ||
| } | ||
| } |
There was a problem hiding this comment.
Multiple critical issues in recursive relations helper.
This function has several problems:
- Same loop logic issue as other helpers with incorrect
objIDincrementing - Slice capacity violation: Line 842 appends to a slice already at
MaxTuplesPerWrite()capacity - Logic error: The
continueon line 844 skips the inner loop increment, causing index misalignment
Apply this comprehensive fix:
-func createRecursiveRelations(b *testing.B, ctx context.Context, datastore storage.OpenFGADatastore, storeID string, numTuples int) {
- b.Helper()
- for objID := 0; objID < numTuples; objID++ {
- tuples := make([]*openfgav1.TupleKey, datastore.MaxTuplesPerWrite())
-
- for j := 0; j < datastore.MaxTuplesPerWrite(); j++ {
- // Every 10th item, make user:justin the leaf
- if j%10 == 0 {
- obj := "org:" + strconv.Itoa(objID)
- tk := tuple.NewTupleKey(obj, "recursive", "user:justin")
- tuples[j] = tk
- tuples = append(tuples, tk)
- objID++
- continue
- }
-
- // otherwise, chain the org#parent#org relation
- obj := "org:" + strconv.Itoa(objID)
- user := "org:" + strconv.Itoa(objID-1)
- tuples[j] = tuple.NewTupleKey(obj, "parent", user)
- objID++
- }
- err := datastore.Write(ctx, storeID, nil, tuples)
- require.NoError(b, err)
- }
-}
+func createRecursiveRelations(b *testing.B, ctx context.Context, datastore storage.OpenFGADatastore, storeID string, numTuples int) {
+ b.Helper()
+ objID := 0
+ for objID < numTuples {
+ batchSize := min(datastore.MaxTuplesPerWrite(), numTuples-objID)
+ tuples := make([]*openfgav1.TupleKey, 0, batchSize)
+
+ for j := 0; j < batchSize && objID < numTuples; j++ {
+ // Every 10th item, make user:justin the leaf
+ if objID%10 == 0 {
+ obj := "org:" + strconv.Itoa(objID)
+ tk := tuple.NewTupleKey(obj, "recursive", "user:justin")
+ tuples = append(tuples, tk)
+ } else {
+ // otherwise, chain the org#parent#org relation
+ obj := "org:" + strconv.Itoa(objID)
+ user := "org:" + strconv.Itoa(objID-1)
+ tuples = append(tuples, tuple.NewTupleKey(obj, "parent", user))
+ }
+ objID++
+ }
+ err := datastore.Write(ctx, storeID, nil, tuples)
+ require.NoError(b, err)
+ }
+}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In pkg/server/commands/list_objects_test.go lines 831 to 856, the
createRecursiveRelations function incorrectly increments objID inside the inner
loop causing logic errors, appends to a slice already at max capacity causing
slice capacity violation, and uses continue which skips necessary increments
leading to index misalignment. To fix, remove the append operation to the tuples
slice to avoid capacity overflow, adjust objID increments to happen only once
per iteration in a controlled manner, and replace continue with proper
conditional logic to ensure all loop indices and increments align correctly
without skipping steps.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
What problem is being solved?
We don't currently have great benchmark coverage of ListObjects. We have an existing benchmark, but it doesn't distinguish between the various weights of relationships, and in the future our different optimization strategies will be based on the weight of the query being run.
How is it being solved?
This benchmark suite creates a single model with relations of weights 1, 2, 3, and INF. It then writes 10k tuples for each of those relationship weights and runs a specific assertion against each relation weight. It does the whole thing twice, once
WithListObjectsOptimizationsEnabled(true)and onceWithListObjectsOptimizationsEnabled(false).Notes from local testing:
These benchmarks are run without a
maxResults, and every case is designed to return 10k tuples; significantly more than the default ListObjects item return limit of 1k objects.Summary
Weighted graph implementation is equal to or faster than non-weighted for all cases, and the weighted graph implementation uses significantly fewer resources in all cases.
Full results below, running with the same setup as our
make test-bench. To reproduce rungo test -bench BenchmarkListObjects ./pkg/server/commands/. -benchtime 5s -cpu 1 -benchmem -count 2Review Checklist
mainSummary by CodeRabbit