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

Fix predefined rule value caching behavior #188

Merged
merged 1 commit into from
Feb 12, 2025
Merged

Conversation

jchadwick-buf
Copy link
Member

At least in some cases, the value of rule can get inappropriately cached. This PR attempts to fix the issue and refactor the code to make it a bit less likely to appear in the future.

This should allow the rule value for predefined rules to still get resolved during compile time, but will hopefully ensure that the environment can't get polluted by the cache.

Closes #187.

Copy link

The latest Buf updates on your PR. Results from workflow Buf / validate-protos (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedFeb 12, 2025, 1:43 AM

@jchadwick-buf
Copy link
Member Author

Benchmark before/after:


$ git checkout main && go test -bench=. -benchmem
goos: darwin
goarch: arm64
pkg: github.com/bufbuild/protovalidate-go
cpu: Apple M1 Max
BenchmarkValidator/ColdStart-10                     1641            708476 ns/op         2682114 B/op      22067 allocs/op
BenchmarkValidator/Lazy/Valid-10                 2023678               600.2 ns/op             8 B/op          1 allocs/op
BenchmarkValidator/Lazy/Invalid-10               2845416               432.5 ns/op          1280 B/op         33 allocs/op
BenchmarkValidator/Lazy/FailFast-10             11363515               100.0 ns/op           248 B/op          7 allocs/op
BenchmarkValidator/PreWarmed/Valid-10            2051448               589.4 ns/op             8 B/op          1 allocs/op
BenchmarkValidator/PreWarmed/Invalid-10          2758561               445.7 ns/op          1280 B/op         33 allocs/op
BenchmarkValidator/PreWarmed/FailFast-10                12198266                99.44 ns/op          248 B/op          7 allocs/op
PASS
ok      github.com/bufbuild/protovalidate-go    11.811s

$ git checkout jchadwick/issue-187 && go test -bench=. -benchmem
goos: darwin
goarch: arm64
pkg: github.com/bufbuild/protovalidate-go
cpu: Apple M1 Max
BenchmarkValidator/ColdStart-10                     1530            673609 ns/op         2683355 B/op      22084 allocs/op
BenchmarkValidator/Lazy/Valid-10                 2033733               591.0 ns/op             8 B/op          1 allocs/op
BenchmarkValidator/Lazy/Invalid-10               2857659               416.9 ns/op          1280 B/op         33 allocs/op
BenchmarkValidator/Lazy/FailFast-10             11410802               108.7 ns/op           248 B/op          7 allocs/op
BenchmarkValidator/PreWarmed/Valid-10            2014646               595.8 ns/op             8 B/op          1 allocs/op
BenchmarkValidator/PreWarmed/Invalid-10          2702216               427.6 ns/op          1280 B/op         33 allocs/op
BenchmarkValidator/PreWarmed/FailFast-10                11902192                97.62 ns/op          248 B/op          7 allocs/op
PASS
ok      github.com/bufbuild/protovalidate-go    12.012s

No significant changes.

@jchadwick-buf jchadwick-buf merged commit 1f18b86 into main Feb 12, 2025
9 checks passed
@jchadwick-buf jchadwick-buf deleted the jchadwick/issue-187 branch February 12, 2025 16:55
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.

[BUG] Rule not honored in expression
2 participants