From d7581d06546582762090539a5741237726fb0e75 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Thu, 13 Feb 2025 11:53:16 +0100 Subject: [PATCH] client-go watch: implement interface with pointer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I wasn't entirely sure whether this should return a value or a pointer to satisfy the interface. Both works, so I benchmarked it elsewhere (REST mapper). Mem allocs are the same (one alloc/call), but returning a value is 10% slower when calling one method. What I then benchmarked is whether pointer vs value receiver in the wrapper makes a difference. Converting from value receiver (what I had before) to pointer receiver reduced call overhead by 6%. That's because with a value receiver, Go has to auto-generate a variant with pointer receiver and calls the value receiver through that. That can be seen in a debugger (call stack) and when setting breakpoints: (dlv) b restMapperWrapper.KindForWithContext Command failed: Location "restMapperWrapper.KindForWithContext" ambiguous: k8s.io/apimachinery/pkg/api/meta.restMapperWrapper.KindForWithContext, k8s.io/apimachinery/pkg/api/meta.(*restMapperWrapper).KindForWithContext… Conventional wisdom is to define types with value receiver because those can be called also on unmutable instances, making them more flexible. But for types which will only ever be used via a pointer, I think pointer receiver is better for the reasons above (small performance difference, easier to debug). Kubernetes-commit: b21dcbcaa1ccf4995bf486afc37dc0321c5bdf0b --- tools/cache/listwatch.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/cache/listwatch.go b/tools/cache/listwatch.go index 2c4065f01..48f3e7a9e 100644 --- a/tools/cache/listwatch.go +++ b/tools/cache/listwatch.go @@ -90,7 +90,7 @@ func ToWatcherWithContext(w Watcher) WatcherWithContext { if w, ok := w.(WatcherWithContext); ok { return w } - return watcherWrapper{ + return &watcherWrapper{ parent: w, } } @@ -99,7 +99,7 @@ type watcherWrapper struct { parent Watcher } -func (l watcherWrapper) WatchWithContext(ctx context.Context, options metav1.ListOptions) (watch.Interface, error) { +func (l *watcherWrapper) WatchWithContext(ctx context.Context, options metav1.ListOptions) (watch.Interface, error) { return l.parent.Watch(options) } @@ -121,7 +121,7 @@ func ToListerWatcherWithContext(lw ListerWatcher) ListerWatcherWithContext { if lw, ok := lw.(ListerWatcherWithContext); ok { return lw } - return listerWatcherWrapper{ + return &listerWatcherWrapper{ ListerWithContext: ToListerWithContext(lw), WatcherWithContext: ToWatcherWithContext(lw), }