cache_req: Allocate topreq only if needed

From a discussion here: varnishcache#4269 (comment)
nigoroll · Feb 12, 2025 · dc397e0 · dc397e0
1 parent 4fe462a
commit dc397e0
Show file tree

Hide file tree

Showing 8 changed files with 18 additions and 16 deletions.
diff --git a/bin/varnishd/acceptor/cache_acceptor_tcp.c b/bin/varnishd/acceptor/cache_acceptor_tcp.c
@@ -406,7 +406,7 @@ vca_tcp_make_session(struct worker *wrk, void *arg)
 
 	vca_tcp_sockopt_set(wa->acceptlsock, sp);
 
-	req = Req_New(sp);
+	req = Req_New(sp, NULL);
 	CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
 	req->htc->rfd = &sp->fd;
 

diff --git a/bin/varnishd/acceptor/cache_acceptor_uds.c b/bin/varnishd/acceptor/cache_acceptor_uds.c
@@ -356,7 +356,7 @@ vca_uds_make_session(struct worker *wrk, void *arg)
 
 	vca_uds_sockopt_set(wa->acceptlsock, sp);
 
-	req = Req_New(sp);
+	req = Req_New(sp, NULL);
 	CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
 	req->htc->rfd = &sp->fd;
 

diff --git a/bin/varnishd/cache/cache_esi_deliver.c b/bin/varnishd/cache/cache_esi_deliver.c
@@ -132,7 +132,7 @@ ved_include(struct req *preq, const char *src, const char *host,
 		return;
 	}
 
-	req = Req_New(sp);
+	req = Req_New(sp, preq);
 	AN(req);
 	THR_SetRequest(req);
 	assert(IS_NO_VXID(req->vsl->wid));
@@ -148,9 +148,6 @@ ved_include(struct req *preq, const char *src, const char *host,
 
 	VSLb_ts_req(req, "Start", W_TIM_real(wrk));
 
-	memset(req->top, 0, sizeof *req->top);
-	req->top = preq->top;
-
 	HTTP_Setup(req->http, req->ws, req->vsl, SLT_ReqMethod);
 	HTTP_Dup(req->http, preq->http0);
 
@@ -182,7 +179,7 @@ ved_include(struct req *preq, const char *src, const char *host,
 	req->req_body_status = BS_NONE;
 
 	AZ(req->vcl);
-	AN(req->top);
+	assert(req->top == preq->top);
 	if (req->top->vcl0)
 		req->vcl = req->top->vcl0;
 	else

diff --git a/bin/varnishd/cache/cache_req.c b/bin/varnishd/cache/cache_req.c
@@ -123,7 +123,7 @@ Req_LogStart(const struct worker *wrk, struct req *req)
  */
 
 struct req *
-Req_New(struct sess *sp)
+Req_New(struct sess *sp, struct req *preq)
 {
 	struct pool *pp;
 	struct req *req;
@@ -134,6 +134,7 @@ Req_New(struct sess *sp)
 	CHECK_OBJ_NOTNULL(sp, SESS_MAGIC);
 	pp = sp->pool;
 	CHECK_OBJ_NOTNULL(pp, POOL_MAGIC);
+	CHECK_OBJ_ORNULL(preq, REQ_MAGIC);
 
 	req = MPL_Get(pp->mpl_req, &sz);
 	AN(req);
@@ -181,10 +182,14 @@ Req_New(struct sess *sp)
 	req->htc->doclose = SC_NULL;
 	p = (void*)PRNDUP(p + sizeof(*req->htc));
 
-	req->top = (void*)p;
-	INIT_OBJ(req->top, REQTOP_MAGIC);
-	req->top->topreq = req;
-	p = (void*)PRNDUP(p + sizeof(*req->top));
+	if (UNLIKELY(preq != NULL))
+		req->top = preq->top;
+	else {
+		req->top = (void*)p;
+		INIT_OBJ(req->top, REQTOP_MAGIC);
+		req->top->topreq = req;
+		p = (void*)PRNDUP(p + sizeof(*req->top));
+	}
 
 	assert(p < e);
 

diff --git a/bin/varnishd/cache/cache_varnishd.h b/bin/varnishd/cache/cache_varnishd.h
@@ -411,7 +411,7 @@ void pan_pool(struct vsb *);
 int VRG_CheckBo(struct busyobj *);
 
 /* cache_req.c */
-struct req *Req_New(struct sess *);
+struct req *Req_New(struct sess *, struct req *);
 void Req_Release(struct req *);
 void Req_Rollback(VRT_CTX);
 void Req_Cleanup(struct sess *sp, struct worker *wrk, struct req *req);

diff --git a/bin/varnishd/http1/cache_http1_fsm.c b/bin/varnishd/http1/cache_http1_fsm.c
@@ -141,7 +141,7 @@ http1_unwait(struct worker *wrk, void *arg)
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
 	CAST_OBJ_NOTNULL(sp, arg, SESS_MAGIC);
 	WS_Release(sp->ws, 0);
-	req = Req_New(sp);
+	req = Req_New(sp, NULL);
 	CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
 	req->htc->rfd = &sp->fd;
 	HTC_RxInit(req->htc, req->ws);

diff --git a/bin/varnishd/http2/cache_http2_proto.c b/bin/varnishd/http2/cache_http2_proto.c
@@ -150,7 +150,7 @@ h2_new_req(struct h2_sess *h2, unsigned stream, struct req *req)
 
 	ASSERT_RXTHR(h2);
 	if (req == NULL)
-		req = Req_New(h2->sess);
+		req = Req_New(h2->sess, NULL);
 	CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
 
 	r2 = WS_Alloc(req->ws, sizeof *r2);

diff --git a/bin/varnishd/http2/cache_http2_session.c b/bin/varnishd/http2/cache_http2_session.c
@@ -132,7 +132,7 @@ h2_init_sess(struct sess *sp,
 	assert(*up == 0);
 
 	if (srq == NULL)
-		srq = Req_New(sp);
+		srq = Req_New(sp, NULL);
 	AN(srq);
 	h2 = h2s;
 	AN(h2);