Collapse revision context into specialized context to avoid allocatio…

…ns (knative#11009)

* Collapse revision context into specialized context to avoid allocations

* Avoid allocation due to non-pointer interface{}

* Simplify to value ctx storing struct

cmd/activator/request_log_test.go

@@ -25,6 +25,7 @@ import (
 	"github.com/google/go-cmp/cmp"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/uuid"
 	ltesting "knative.dev/pkg/logging/testing"
 	"knative.dev/pkg/metrics"
@@ -126,7 +127,10 @@ func TestUpdateRequestLogFromConfigMap(t *testing.T) {
 			resp := httptest.NewRecorder()
 			rs := string(uuid.NewUUID())
 			req := httptest.NewRequest(http.MethodPost, test.url, bytes.NewBufferString(rs))
-			ctx := activatorhandler.WithRevision(req.Context(), revision(true))
+
+			rev := revision(true)
+			revID := types.NamespacedName{Namespace: rev.Namespace, Name: rev.Name}
+			ctx := activatorhandler.WithRevisionAndID(req.Context(), rev, revID)
 			handler.ServeHTTP(resp, req.WithContext(ctx))
 
 			if got := buf.String(); got != test.want {
@@ -173,7 +177,8 @@ func TestRequestLogTemplateInputGetter(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			ctx := activatorhandler.WithRevision(test.request.Context(), test.revision)
+			revID := types.NamespacedName{Namespace: test.revision.Namespace, Name: test.revision.Name}
+			ctx := activatorhandler.WithRevisionAndID(test.request.Context(), test.revision, revID)
 			request := test.request.WithContext(ctx)
 
 			got := requestLogTemplateInputGetter(request, test.response)

pkg/activator/handler/concurrency_reporter_test.go

@@ -437,7 +437,7 @@ func TestConcurrencyReporterHandler(t *testing.T) {
 
 	resp := httptest.NewRecorder()
 	req := httptest.NewRequest(http.MethodPost, "http://example.com", nil)
-	rCtx := WithRevID(context.Background(), rev1)
+	rCtx := WithRevisionAndID(context.Background(), nil, rev1)
 
 	// Send a few requests.
 	handler.ServeHTTP(resp, req.WithContext(rCtx))
@@ -644,7 +644,7 @@ func BenchmarkConcurrencyReporterHandler(b *testing.B) {
 			Name:      testRevName + strconv.Itoa(i),
 		}
 		req := httptest.NewRequest(http.MethodGet, "http://example.com", nil)
-		reqs = append(reqs, req.WithContext(WithRevID(context.Background(), key)))
+		reqs = append(reqs, req.WithContext(WithRevisionAndID(context.Background(), nil, key)))
 
 		// Create revisions in the fake clients to trigger report logic.
 		rev := revision(key.Namespace, key.Name)

pkg/activator/handler/context.go

@@ -29,26 +29,28 @@ import (
 )
 
 type (
-	revisionKey struct{}
-	revIDKey    struct{}
+	revCtxKey struct{}
 )
 
-// WithRevision attaches the Revision object to the context.
-func WithRevision(ctx context.Context, rev *v1.Revision) context.Context {
-	return context.WithValue(ctx, revisionKey{}, rev)
+type revCtx struct {
+	revision *v1.Revision
+	revID    types.NamespacedName
 }
 
-// RevisionFrom retrieves the Revision object from the context.
-func RevisionFrom(ctx context.Context) *v1.Revision {
-	return ctx.Value(revisionKey{}).(*v1.Revision)
+// WithRevisionAndID attaches the Revision and the ID to the context.
+func WithRevisionAndID(ctx context.Context, rev *v1.Revision, revID types.NamespacedName) context.Context {
+	return context.WithValue(ctx, revCtxKey{}, &revCtx{
+		revision: rev,
+		revID:    revID,
+	})
 }
 
-// WithRevID attaches the the revisionID to the context.
-func WithRevID(ctx context.Context, revID types.NamespacedName) context.Context {
-	return context.WithValue(ctx, revIDKey{}, revID)
+// RevisionFrom retrieves the Revision object from the context.
+func RevisionFrom(ctx context.Context) *v1.Revision {
+	return ctx.Value(revCtxKey{}).(*revCtx).revision
 }
 
 // RevIDFrom retrieves the the revisionID from the context.
 func RevIDFrom(ctx context.Context) types.NamespacedName {
-	return ctx.Value(revIDKey{}).(types.NamespacedName)
+	return ctx.Value(revCtxKey{}).(*revCtx).revID
 }

pkg/activator/handler/context_handler.go

@@ -74,8 +74,7 @@ func (h *contextHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	}
 
 	ctx := r.Context()
-	ctx = WithRevision(ctx, revision)
-	ctx = WithRevID(ctx, revID)
+	ctx = WithRevisionAndID(ctx, revision, revID)
 
 	h.nextHandler.ServeHTTP(w, r.WithContext(ctx))
 }

pkg/activator/handler/handler_test.go

@@ -134,7 +134,7 @@ func TestActivationHandler(t *testing.T) {
 			// Set up config store to populate context.
 			configStore := setupConfigStore(t, logging.FromContext(ctx))
 			ctx = configStore.ToContext(ctx)
-			ctx = WithRevID(ctx, types.NamespacedName{Namespace: testNamespace, Name: testRevName})
+			ctx = WithRevisionAndID(ctx, nil, types.NamespacedName{Namespace: testNamespace, Name: testRevName})
 
 			handler.ServeHTTP(resp, req.WithContext(ctx))
 
@@ -172,7 +172,7 @@ func TestActivationHandlerProxyHeader(t *testing.T) {
 	// Set up config store to populate context.
 	configStore := setupConfigStore(t, logging.FromContext(ctx))
 	ctx = configStore.ToContext(req.Context())
-	ctx = WithRevID(ctx, types.NamespacedName{Namespace: testNamespace, Name: testRevName})
+	ctx = WithRevisionAndID(ctx, nil, types.NamespacedName{Namespace: testNamespace, Name: testRevName})
 
 	handler.ServeHTTP(writer, req.WithContext(ctx))
 
@@ -267,7 +267,7 @@ func sendRequest(namespace, revName string, handler http.Handler, store *activat
 	resp := httptest.NewRecorder()
 	req := httptest.NewRequest(http.MethodPost, "http://example.com/", nil)
 	ctx := store.ToContext(req.Context())
-	ctx = WithRevID(ctx, types.NamespacedName{Namespace: namespace, Name: revName})
+	ctx = WithRevisionAndID(ctx, nil, types.NamespacedName{Namespace: namespace, Name: revName})
 	handler.ServeHTTP(resp, req.WithContext(ctx))
 	return resp
 }
@@ -318,7 +318,7 @@ func BenchmarkHandler(b *testing.B) {
 			req.Host = "test-host"
 
 			reqCtx := configStore.ToContext(context.Background())
-			reqCtx = WithRevID(reqCtx, types.NamespacedName{Namespace: testNamespace, Name: testRevName})
+			reqCtx = WithRevisionAndID(reqCtx, nil, types.NamespacedName{Namespace: testNamespace, Name: testRevName})
 			return req.WithContext(reqCtx)
 		}
 

pkg/activator/handler/metric_handler_test.go

@@ -114,8 +114,7 @@ func TestRequestMetricHandler(t *testing.T) {
 				metricstest.AssertMetricExists(t, responseTimeInMsecM.Name())
 			}()
 
-			reqCtx := WithRevision(context.Background(), rev)
-			reqCtx = WithRevID(reqCtx, types.NamespacedName{Namespace: testNamespace, Name: testRevName})
+			reqCtx := WithRevisionAndID(context.Background(), rev, types.NamespacedName{Namespace: testNamespace, Name: testRevName})
 			handler.ServeHTTP(resp, req.WithContext(reqCtx))
 		})
 	}
@@ -128,7 +127,7 @@ func reset() {
 
 func BenchmarkMetricHandler(b *testing.B) {
 	baseHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})
-	reqCtx := WithRevision(context.Background(), revision(testNamespace, testRevName))
+	reqCtx := WithRevisionAndID(context.Background(), revision(testNamespace, testRevName), types.NamespacedName{Namespace: testNamespace, Name: testRevName})
 
 	handler := NewMetricHandler("benchPod", baseHandler)