part: allocate watches fully lazily - #198
Merged
Merged
Conversation
|
joamaki
force-pushed
the
pr/joamaki/improve-watches
branch
2 times, most recently
from
September 7, 2026 08:03
11aa908 to
4ffa355
Compare
giorio94
self-requested a review
September 11, 2026 12:52
giorio94
approved these changes
Sep 14, 2026
giorio94
left a comment
giorio94
left a comment
Member
There was a problem hiding this comment.
Changes look reasonable to me, although I definitely don't have a very deep knowledge of all the statedb internals. These changes are also making the logic a notch more difficult to read and reason about, but I guess that's the tradeoff to try to extract some more performance out of it.
The watch state was still allocated eagerly. This refactors the code
to fully allocate watches lazily. The nodes now contain a 'lazyWatchChannel'
which holds an atomic pointer to 'atomicWatchPointer', so essentially
each node has 'atomic.Pointer[atomic.Pointer[*hchan]]'. The indirection
is required as a logical watch may be shared when the tree forks. If
we'd just store 'atomic.Pointer[*hchan]' we wouldn't be able to avoid
a double-close when the tree forks as the two instances would not be
able to synchronize.
goos: linux
goarch: arm64
pkg: github.com/cilium/statedb/part
│ before │ after │
│ sec/op │ sec/op vs base │
_Insert 112.1µ 111.1µ ~ (p=0.280 n=10)
_WatchReplace 108.39µ 88.58µ -18.27% (p=0.001 n=10)
_GetWatch 17.91µ 19.75µ +10.28% (p=0.001 n=10)
│ before │ after │
│ B/op │ B/op vs base │
_Insert 82.28Ki 74.31Ki -9.68% (p=0.000 n=10)
_WatchReplace 63.81Ki 55.96Ki -12.30% (p=0.000 n=10)
_GetWatch 0.000 0.000 ~ (p=1.000 n=10)
│ before │ after │
│ allocs/op │ allocs/op vs base │
_Insert 3.067k 2.047k -33.26% (p=0.000 n=10)
_WatchReplace 2.011k 1.006k -49.98% (p=0.000 n=10)
_GetWatch 0.000 0.000 ~ (p=1.000 n=10)
Reconciler benchmark (best of 3) before:
1000000 objects reconciled in 1.38 seconds (batch size 1000)
Throughput 727151.51 objects per second
568MB total allocated, 6015247 in-use objects, 239MB bytes in use
After:
1000000 objects reconciled in 1.34 seconds (batch size 1000)
Throughput 748576.38 objects per second
552MB total allocated, 5010271 in-use objects, 231MB bytes in use
An isolated comparison of the pointer wrapper with the direct channel
pointer shows first channel creation improving from 51.44ns to 42.41ns
(-17.54%), 128B to 120B (-6.25%), and 3 to 2 allocations (-33.33%).
Accessing an existing channel changes from 1.999ns to 2.042ns (+2.15%).
A final comparison against an equivalent atomic.Value implementation on Go
1.25.10 used 15 samples of 300ms each pinned to one CPU. atomic.Value was
31.98% slower when creating the first channel, 6.48% slower when loading an
existing channel, 58.14% slower when closing before observation, and 31.54%
slower for channel creation followed by close. Allocation counts were equal,
but atomic.Value used 128B instead of 120B for channel creation and 16B
instead of 8B for close before observation.
AIL:3
Signed-off-by: Jussi Maki <jussi@isovalent.com>
Avoid the variable-length memequal call for the common zero- and one-byte compressed prefixes. Keep the existing optimized comparison for longer prefixes. name old time/op new time/op delta Benchmark_Get-6 14.17us 13.42us -5.24% Benchmark_GetWatch-6 19.97us 18.48us -7.46% Benchmark_GetInsert-6 83.38us 81.43us -2.33% Benchmark_Get throughput improves from 70.59M to 74.49M objects/sec (+5.53%), and Benchmark_GetWatch improves from 50.09M to 54.13M objects/sec (+8.07%). Allocation counts and bytes are unchanged. AIL:3 Signed-off-by: Jussi Maki <jussi@isovalent.com>
Use a dedicated search path for Tree.Get and Txn.Get that does not track the closest watch while traversing the tree. Keep the existing watch-aware path for GetWatch. name old time/op new time/op delta Benchmark_Get-6 16.59us 13.67us -17.58% Lookup throughput improves from 60.28M to 73.13M objects/sec (+21.33%). Both versions perform zero allocations. AIL:3 Signed-off-by: Jussi Maki <jussi@isovalent.com>
joamaki
force-pushed
the
pr/joamaki/improve-watches
branch
from
September 14, 2026 09:12
0b30d05 to
20c68e0
Compare
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 free
to 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.
The watch state was still allocated eagerly. This refactores the code to fully allocate watches lazily. The nodes now contain a 'lazyWatchChannel' which is an atomic pointer to 'atomicWatchPointer', so essentially each node has 'atomic.Pointer[atomic.Pointer[*runtime.hchan]]'. The indirection is required as a logical watch may be shared when the tree forks. If we'd just store 'atomic.Pointer[*runtime.hchan]' we wouldn't be able to avoid a double-close when the tree forks as the two instances would not be able to synchronize.
Reconciler benchmark (best of 3) before:
After:
An isolated comparison of the pointer wrapper with the direct channel pointer shows first channel creation improving from 51.44ns to 42.41ns (-17.54%), 128B to 120B (-6.25%), and 3 to 2 allocations (-33.33%). Accessing an existing channel changes from 1.999ns to 2.042ns (+2.15%).
I also compared against an equivalent atomic.Value implementation. atomic.Value was 31.98% slower when creating the first channel, 6.48% slower when loading an existing channel, 58.14% slower when closing before observation, and 31.54% slower for channel creation followed by close. Allocation counts were equal, but atomic.Value used 128B instead of 120B for channel creation and 16B instead of 8B for close before observation. So I would argue the trade-off of using
unsafe.Pointerto grab the*hchanis acceptable. Yes it means StateDB won't work on Go implementations/architectures wherechan Tis not a pointer, but considering the other uses ofunsafe.Pointerinpart/node.gothis isn't really an issue. It would also mean that if Go ever changes the internal representation things would break here, but tests will easily catch that if it were to happen and we can then change toatomic.Value.After this PR the
part.RootOnlyWatchoption doesn't help nearly as much anymore. However removing it completely did increase allocations from 552MB to 581MB in the reconciler benchmark without impact on throughput, so I'll leave it around for now. We can consider removing it in the future to simplify the implementation.AIL:3