From dc397e015a8e6513171e2c0b7a3a517c04b75744 Mon Sep 17 00:00:00 2001 From: Nils Goroll Date: Wed, 12 Feb 2025 20:04:29 +0100 Subject: [PATCH] cache_req: Allocate topreq only if needed From a discussion here: https://github.com/varnishcache/varnish-cache/pull/4269#issuecomment-2654577140 --- bin/varnishd/acceptor/cache_acceptor_tcp.c | 2 +- bin/varnishd/acceptor/cache_acceptor_uds.c | 2 +- bin/varnishd/cache/cache_esi_deliver.c | 7 ++----- bin/varnishd/cache/cache_req.c | 15 ++++++++++----- bin/varnishd/cache/cache_varnishd.h | 2 +- bin/varnishd/http1/cache_http1_fsm.c | 2 +- bin/varnishd/http2/cache_http2_proto.c | 2 +- bin/varnishd/http2/cache_http2_session.c | 2 +- 8 files changed, 18 insertions(+), 16 deletions(-) diff --git a/bin/varnishd/acceptor/cache_acceptor_tcp.c b/bin/varnishd/acceptor/cache_acceptor_tcp.c index 66f15b8e7a..af2ceda901 100644 --- 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 index b71cd1cc47..75462416b9 100644 --- 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 index f739b22c38..ccb44d32d3 100644 --- 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 index 0511ae09b6..dd441b33ea 100644 --- 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 index 34a616c8d9..4154f05e55 100644 --- 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 index 9763078633..ac1dc012cf 100644 --- 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 index c3800ded81..ba21ea5c60 100644 --- 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 index 075cba1838..46b02dc094 100644 --- 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);