Uh oh!
There was an error while loading. Please reload this page.
structdiff: construct paths on based on json tag - #2954
Merged
Conversation
~/work/cli-main/libs/structdiff % ./bench.sh + exec go test -bench=. -benchmem -run '^x' goos: darwin goarch: arm64 pkg: github.com/databricks/cli/libs/structdiff cpu: Apple M3 Max BenchmarkEqual-16 21391 55773 ns/op 21464 B/op 805 allocs/op --- BENCH: BenchmarkEqual-16 bench_test.go:30: Total: 0 / 1 bench_test.go:30: Total: 0 / 100 bench_test.go:30: Total: 0 / 10000 bench_test.go:30: Total: 0 / 21391 BenchmarkChanges-16 20712 57102 ns/op 24128 B/op 863 allocs/op --- BENCH: BenchmarkChanges-16 bench_test.go:30: Total: 10 / 1 bench_test.go:30: Total: 1000 / 100 bench_test.go:30: Total: 100000 / 10000 bench_test.go:30: Total: 207120 / 20712 BenchmarkZero-16 19105 63585 ns/op 33948 B/op 1097 allocs/op --- BENCH: BenchmarkZero-16 bench_test.go:30: Total: 75 / 1 bench_test.go:30: Total: 7500 / 100 bench_test.go:30: Total: 750000 / 10000 bench_test.go:30: Total: 1432875 / 19105 BenchmarkNils-16 25804 46156 ns/op 26740 B/op 768 allocs/op --- BENCH: BenchmarkNils-16 bench_test.go:30: Total: 77 / 1 bench_test.go:30: Total: 7700 / 100 bench_test.go:30: Total: 770000 / 10000 bench_test.go:30: Total: 1986908 / 25804 PASS ok github.com/databricks/cli/libs/structdiff 7.301s
+ exec go test -bench=. -benchmem -run '^x' goos: darwin goarch: arm64 pkg: github.com/databricks/cli/libs/structdiff cpu: Apple M3 Max BenchmarkEqual-16 24752 47089 ns/op 50185 B/op 827 allocs/op --- BENCH: BenchmarkEqual-16 bench_test.go:30: Total: 0 / 1 bench_test.go:30: Total: 0 / 100 bench_test.go:30: Total: 0 / 10000 bench_test.go:30: Total: 0 / 24752 BenchmarkChanges-16 24080 50690 ns/op 52849 B/op 885 allocs/op --- BENCH: BenchmarkChanges-16 bench_test.go:30: Total: 10 / 1 bench_test.go:30: Total: 1000 / 100 bench_test.go:30: Total: 100000 / 10000 bench_test.go:30: Total: 240800 / 24080 BenchmarkZero-16 20282 60779 ns/op 61068 B/op 1116 allocs/op --- BENCH: BenchmarkZero-16 bench_test.go:30: Total: 75 / 1 bench_test.go:30: Total: 7500 / 100 bench_test.go:30: Total: 750000 / 10000 bench_test.go:30: Total: 1521150 / 20282 BenchmarkNils-16 27357 43438 ns/op 47355 B/op 780 allocs/op --- BENCH: BenchmarkNils-16 bench_test.go:30: Total: 77 / 1 bench_test.go:30: Total: 7700 / 100 bench_test.go:30: Total: 770000 / 10000 bench_test.go:30: Total: 2106489 / 27357 PASS ok github.com/databricks/cli/libs/structdiff 7.127s
+ exec go test -bench=. -benchmem -run '^x' goos: darwin goarch: arm64 pkg: github.com/databricks/cli/libs/structdiff cpu: Apple M3 Max BenchmarkEqual-16 25681 46186 ns/op 50185 B/op 827 allocs/op --- BENCH: BenchmarkEqual-16 bench_test.go:30: Total: 0 / 1 bench_test.go:30: Total: 0 / 100 bench_test.go:30: Total: 0 / 10000 bench_test.go:30: Total: 0 / 25681 BenchmarkChanges-16 24361 49276 ns/op 52849 B/op 885 allocs/op --- BENCH: BenchmarkChanges-16 bench_test.go:30: Total: 10 / 1 bench_test.go:30: Total: 1000 / 100 bench_test.go:30: Total: 100000 / 10000 bench_test.go:30: Total: 243610 / 24361 BenchmarkZero-16 20415 58051 ns/op 61066 B/op 1116 allocs/op --- BENCH: BenchmarkZero-16 bench_test.go:30: Total: 75 / 1 bench_test.go:30: Total: 7500 / 100 bench_test.go:30: Total: 750000 / 10000 bench_test.go:30: Total: 1531125 / 20415 BenchmarkNils-16 27727 42859 ns/op 47355 B/op 780 allocs/op --- BENCH: BenchmarkNils-16 bench_test.go:30: Total: 77 / 1 bench_test.go:30: Total: 7700 / 100 bench_test.go:30: Total: 770000 / 10000 bench_test.go:30: Total: 2134979 / 27727 PASS ok github.com/databricks/cli/libs/structdiff 7.039s
~/work/cli-main/libs/structdiff % go test -bench=. -benchmem -run ^x -benchtime=10s goos: darwin goarch: arm64 pkg: github.com/databricks/cli/libs/structdiff cpu: Apple M3 Max BenchmarkEqual-16 321285 35888 ns/op 18040 B/op 540 allocs/op --- BENCH: BenchmarkEqual-16 bench_test.go:30: Total: 0 / 1 bench_test.go:30: Total: 0 / 100 bench_test.go:30: Total: 0 / 10000 bench_test.go:30: Total: 0 / 321285 BenchmarkChanges-16 312716 38235 ns/op 20704 B/op 598 allocs/op --- BENCH: BenchmarkChanges-16 bench_test.go:30: Total: 10 / 1 bench_test.go:30: Total: 1000 / 100 bench_test.go:30: Total: 100000 / 10000 bench_test.go:30: Total: 3127160 / 312716 BenchmarkZero-16 265086 46295 ns/op 30457 B/op 843 allocs/op --- BENCH: BenchmarkZero-16 bench_test.go:30: Total: 75 / 1 bench_test.go:30: Total: 7500 / 100 bench_test.go:30: Total: 750000 / 10000 bench_test.go:30: Total: 19881450 / 265086 BenchmarkNils-16 357333 33206 ns/op 24034 B/op 572 allocs/op --- BENCH: BenchmarkNils-16 bench_test.go:30: Total: 77 / 1 bench_test.go:30: Total: 7700 / 100 bench_test.go:30: Total: 770000 / 10000 bench_test.go:30: Total: 27514641 / 357333 PASS ok github.com/databricks/cli/libs/structdiff 49.502s
denik
requested review from
andrewnester, anton-107, pietern and shreyas-goenka
as code ownersMay 28, 2025 09:58
pietern
approved these changes
May 28, 2025
| // JSONTag represents a struct field's `json` tag as a string. | ||
| // It provides methods to lazily extract information from the tag. | ||
| type JSONTag string |
Contributor
There was a problem hiding this comment.
It seems worth computing the options we need at construction time.
Potentially can also cache these objects by reflect.Type. We do the same in libs/dyn/convert/struct_info.go.
ContributorAuthor
There was a problem hiding this comment.
It seems worth computing the options we need at construction time.
We should benchmark it. My intuition is - looking up things in a short string can be made very fast and allocation-free. Pre-parsing into struct requires you to allocate that struct at least.
denik added a commit
that referenced
this pull request
May 20, 2026
## Changes Switch structdiff to (lazily) compute jsontag-based path instead of Golang field names. Follow up to #2928 ## Why This is closer to what user is going to want to see in "bundle diff". This is also useful if we want to look up the path in dyn.Value world. ## Benchmark This PR: ``` ~/work/cli-main/libs/structdiff % go test -bench=. -benchmem -run ^x -benchtime=10s goos: darwin goarch: arm64 pkg: github.com/databricks/cli/libs/structdiff cpu: Apple M3 Max BenchmarkEqual-16 315444 36290 ns/op 18040 B/op 540 allocs/op BenchmarkChanges-16 307059 38747 ns/op 20704 B/op 598 allocs/op BenchmarkZero-16 260900 45907 ns/op 30456 B/op 843 allocs/op BenchmarkNils-16 365086 33155 ns/op 24034 B/op 572 allocs/op ``` main: ``` ~/work/cli-main/libs/structdiff % go test -bench=. -benchmem -run ^x -benchtime=10s goos: darwin goarch: arm64 pkg: github.com/databricks/cli/libs/structdiff cpu: Apple M3 Max BenchmarkEqual-16 311192 36944 ns/op 13368 B/op 540 allocs/op BenchmarkChanges-16 310232 39032 ns/op 16008 B/op 598 allocs/op BenchmarkZero-16 264555 45187 ns/op 26011 B/op 843 allocs/op BenchmarkNils-16 363981 32819 ns/op 20542 B/op 572 allocs/op ```
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
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Changes
Switch structdiff to (lazily) compute jsontag-based path instead of Golang field names.
Follow up to #2928
Why
This is closer to what user is going to want to see in "bundle diff".
This is also useful if we want to look up the path in dyn.Value world.
Benchmark
This PR:
main: