Skip to content

Conversation

@anderseknert
Copy link
Member

Mainly making transactions cheaper to create, and read transactions much cheaper.

  • Add exported RootPath shorthand var
  • Don't return path on ParsePathEscaped failure
  • Allocate nothing for read transactions, other than the transaction itself
  • Lazy init of write update collections to avoid needless allocations
  • Add benchmarks

Before

BenchmarkNewTransaction/Read-16                     26707234            44.78 ns/op      144 B/op          3 allocs/op
BenchmarkNewTransaction/Write-16                    20344212            59.44 ns/op      192 B/op          4 allocs/op
BenchmarkReadOne/Go_store_(roundtrip)-16            21963003            54.41 ns/op      144 B/op          3 allocs/op
BenchmarkReadOne/Go_store_(no_roundtrip)-16         22217593            54.18 ns/op      144 B/op          3 allocs/op
BenchmarkReadOne/AST_store_(roundtrip)-16           15626653            76.52 ns/op      160 B/op          4 allocs/op
BenchmarkReadOne/AST_store_(no_roundtrip)-16        15820837            76.15 ns/op      160 B/op          4 allocs/op

After

BenchmarkNewTransaction/Read-16                     68091271            17.37 ns/op       48 B/op          1 allocs/op
BenchmarkNewTransaction/Write-16                    24928028            47.68 ns/op      144 B/op          3 allocs/op
BenchmarkReadOne/Go_store_(roundtrip)-16            42967630            28.10 ns/op       48 B/op          1 allocs/op
BenchmarkReadOne/Go_store_(no_roundtrip)-16         43825009            27.63 ns/op       48 B/op          1 allocs/op
BenchmarkReadOne/AST_store_(roundtrip)-16           24885938            48.06 ns/op       64 B/op          2 allocs/op
BenchmarkReadOne/AST_store_(no_roundtrip)-16        25012396            47.96 ns/op       64 B/op          2 allocs/op

@netlify
Copy link

netlify bot commented Sep 29, 2025

Deploy Preview for openpolicyagent ready!

Name Link
🔨 Latest commit 1e853c3
🔍 Latest deploy log https://app.netlify.com/projects/openpolicyagent/deploys/68dafdafccd34f000792dea7
😎 Deploy Preview https://deploy-preview-7944--openpolicyagent.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copy link
Contributor

@srenatus srenatus left a comment

Choose a reason for hiding this comment

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

LGTM

if write {

if txn.write {
txn.policies = map[string]policyUpdate{}
Copy link
Member Author

Choose a reason for hiding this comment

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

Looks like I forgot to lazy init this one! Will fix before merge

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed now, and we're down to 2 allocs / 96 B per write transaction, compared to 3 / 144 from before the change, 4 / 192 on main.

BenchmarkNewTransaction/Write-16            37049997            32.30 ns/op       96 B/op          2 allocs/op

Mainly making transactions cheaper to create, and read transactions
much cheaper.

- Add exported RootPath shorthand var
- Don't return path on ParsePathEscaped failure
- Allocate nothing for read transactions, other than the transaction itself
- Lazy init of write update collections to avoid needless allocations
- Add benchmarks

**Before**
```
BenchmarkNewTransaction/Read-16                     26707234            44.78 ns/op      144 B/op          3 allocs/op
BenchmarkNewTransaction/Write-16                    20344212            59.44 ns/op      192 B/op          4 allocs/op
BenchmarkReadOne/Go_store_(roundtrip)-16            21963003            54.41 ns/op      144 B/op          3 allocs/op
BenchmarkReadOne/Go_store_(no_roundtrip)-16         22217593            54.18 ns/op      144 B/op          3 allocs/op
BenchmarkReadOne/AST_store_(roundtrip)-16           15626653            76.52 ns/op      160 B/op          4 allocs/op
BenchmarkReadOne/AST_store_(no_roundtrip)-16        15820837            76.15 ns/op      160 B/op          4 allocs/op
```

**After**
```
BenchmarkNewTransaction/Read-16                     68091271            17.37 ns/op       48 B/op          1 allocs/op
BenchmarkNewTransaction/Write-16                    24928028            47.68 ns/op      144 B/op          3 allocs/op
BenchmarkReadOne/Go_store_(roundtrip)-16            42967630            28.10 ns/op       48 B/op          1 allocs/op
BenchmarkReadOne/Go_store_(no_roundtrip)-16         43825009            27.63 ns/op       48 B/op          1 allocs/op
BenchmarkReadOne/AST_store_(roundtrip)-16           24885938            48.06 ns/op       64 B/op          2 allocs/op
BenchmarkReadOne/AST_store_(no_roundtrip)-16        25012396            47.96 ns/op       64 B/op          2 allocs/op
```

Signed-off-by: Anders Eknert <[email protected]>
@anderseknert anderseknert merged commit e1e2bfb into open-policy-agent:main Sep 29, 2025
31 checks passed
@anderseknert anderseknert deleted the storage-improvements branch September 29, 2025 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants