reduce DeepCopy in gateway-api layer by arkodg · Pull Request #6940 · envoyproxy/gateway

arkodg · 2025-09-11T02:06:21Z

 benchstat old.txt new.txt 
goos: darwin
goarch: amd64
pkg: github.com/envoyproxy/gateway/test/gobench
cpu: Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
                          │   old.txt    │                new.txt                │
                          │    sec/op    │    sec/op     vs base                 │
GatewayAPItoXDS/small-12    3.852m ± ∞ ¹   2.691m ± ∞ ¹        ~ (p=0.700 n=3) ²
GatewayAPItoXDS/medium-12   51.09m ± ∞ ¹   43.25m ± ∞ ¹        ~ (p=0.700 n=3) ²
GatewayAPItoXDS/large-12    397.4m ± ∞ ¹   385.0m ± ∞ ¹        ~ (p=0.700 n=3) ²
geomean                     42.76m         35.52m        -16.94%
¹ need >= 6 samples for confidence interval at level 0.95
² need >= 4 samples to detect a difference at alpha level 0.05

                          │    old.txt    │                new.txt                │
                          │     B/op      │     B/op       vs base                │
GatewayAPItoXDS/small-12    727.2Ki ± ∞ ¹   723.0Ki ± ∞ ¹       ~ (p=0.400 n=3) ²
GatewayAPItoXDS/medium-12   15.90Mi ± ∞ ¹   15.63Mi ± ∞ ¹       ~ (p=0.100 n=3) ²
GatewayAPItoXDS/large-12    153.9Mi ± ∞ ¹   151.3Mi ± ∞ ¹       ~ (p=0.100 n=3) ²
geomean                     12.02Mi         11.86Mi        -1.31%
¹ need >= 6 samples for confidence interval at level 0.95
² need >= 4 samples to detect a difference at alpha level 0.05

                          │   old.txt    │               new.txt                │
                          │  allocs/op   │  allocs/op    vs base                │
GatewayAPItoXDS/small-12    11.80k ± ∞ ¹   11.72k ± ∞ ¹       ~ (p=0.400 n=3) ²
GatewayAPItoXDS/medium-12   261.3k ± ∞ ¹   257.3k ± ∞ ¹       ~ (p=0.100 n=3) ²
GatewayAPItoXDS/large-12    2.505M ± ∞ ¹   2.467M ± ∞ ¹       ~ (p=0.100 n=3) ²
geomean                     197.7k         195.2k        -1.26%
¹ need >= 6 samples for confidence interval at level 0.95
² need >= 4 samples to detect a difference at alpha level 0.05

also fixed the DeepCopy implementation for ControllerResources
which was performing a Shallow Copy resulting it lack of isolation
b/w provider and gateway-api layer

Relates to #6919

codecov · 2025-09-11T04:30:56Z

Codecov Report

❌ Patch coverage is 96.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 71.02%. Comparing base (53c24c8) to head (e4a08a4).
⚠️ Report is 5 commits behind head on main.

Files with missing lines	Patch %	Lines
internal/gatewayapi/backendtrafficpolicy.go	66.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6940      +/-   ##
==========================================
- Coverage   71.03%   71.02%   -0.02%     
==========================================
  Files         226      226              
  Lines       40037    40038       +1     
==========================================
- Hits        28441    28436       -5     
- Misses       9915     9921       +6     
  Partials     1681     1681

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Xunzhuo

Nice hacking

rudrakhp · 2025-09-11T09:33:56Z

internal/gatewayapi/backendtrafficpolicy.go

Curious why needed a DeepCopy before, and why even do the shallow copy now? Are we modifying policy during the process?

looks like its been there from the beginning
we dont need a deepCopy here because the gateway-api layer is free to update the object received from watchable and modify it, which it does ( modifies status)
note, there was a bug in DeepCopy implementation of ControllerResources that I fixed in this PR too which was keeping some shallow copy references around, that was needed to be changed to a pure DeepCopy for isolation between the provider layer and gateway-api layer

the gateway-api layer is free to update the object received from watchable and modify it

Why not use currPolicy everywhere in that case? Can we remove the shallow copy policy = currPolicy? Unless this is intentional to avoid multiple changes. Approved as this is minor.

Sure will try doing another round of clean up

* also fixed the DeepCopy implementation for ControllerResources which was performing a Shallow Copy resulting it lack of isolation b/w provider and gateway-api layer Relates to envoyproxy#6919 Signed-off-by: Arko Dasgupta <arko@tetrate.io>

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

zirain · 2025-09-12T10:42:45Z

/retest

* reduce deep copy in gateway-api layer * also fixed the DeepCopy implementation for ControllerResources which was performing a Shallow Copy resulting it lack of isolation b/w provider and gateway-api layer Relates to envoyproxy#6919 Signed-off-by: Arko Dasgupta <arko@tetrate.io> * fix lint Signed-off-by: Arko Dasgupta <arko@tetrate.io> --------- Signed-off-by: Arko Dasgupta <arko@tetrate.io> Signed-off-by: zirain <zirain2009@gmail.com>

* fix: cluster stat name: lowercase Kind (#6780) cluster stat name: lowercase Kind Signed-off-by: Guy Daich <guy.daich@sap.com> Signed-off-by: zirain <zirain2009@gmail.com> * fix: envoy service cluster name for zone-aware routing (#6763) * fix!: fix envoy service cluster name for zone-aware routing Signed-off-by: y-rabie <youssef.rabie@procore.com> * extend e2e tests for zone aware routing Signed-off-by: y-rabie <youssef.rabie@procore.com> * extend unit tests for zone aware routing Signed-off-by: y-rabie <youssef.rabie@procore.com> --------- Signed-off-by: y-rabie <youssef.rabie@procore.com> Signed-off-by: zirain <zirain2009@gmail.com> * conformance: update experimental test report (#6782) * conformance: update experimental test report Signed-off-by: zirain <zirain2009@gmail.com> * fix version Signed-off-by: zirain <zirain2009@gmail.com> * fix(api): image validation regex, support port in repository (#6819) fix: match repository in image with port Signed-off-by: Windfarer <windfarer@gmail.com> Signed-off-by: zirain <zirain2009@gmail.com> * fix: Actually update xdsIR with maxAcceptPerSocketEvent (#6834) * Actually update xdsIR with maxAcceptPerSocketEvent Signed-off-by: jukie <10012479+Jukie@users.noreply.github.com> * release note Signed-off-by: jukie <10012479+Jukie@users.noreply.github.com> * newline lint Signed-off-by: jukie <10012479+Jukie@users.noreply.github.com> --------- Signed-off-by: jukie <10012479+Jukie@users.noreply.github.com> Signed-off-by: zirain <zirain2009@gmail.com> * bugfix: fix the topologyInjectorDisabled and the local cluster was not defined (#6847) * bugfix: fix the topologyInjectorDisabled and the local cluster was not defined. Signed-off-by: qicz <qiczzhu@gmail.com> * fix ut Signed-off-by: qicz <qiczzhu@gmail.com> * add topology-injector-enabled ut Signed-off-by: qicz <qiczzhu@gmail.com> * add release note Signed-off-by: qi <qiczzhu@gmail.com> --------- Signed-off-by: qicz <qiczzhu@gmail.com> Signed-off-by: qi <qiczzhu@gmail.com> Signed-off-by: zirain <zirain2009@gmail.com> * fix(logging): correct log formatting to avoid DPANIC in controller-runtime logger (#6846) * Update filters.go Signed-off-by: TomerJLevy <TomerJlevy@gmail.com> * add release notes Signed-off-by: TomerJLevy <TomerJlevy@gmail.com> --------- Signed-off-by: TomerJLevy <TomerJlevy@gmail.com> Signed-off-by: zirain <zirain2009@gmail.com> * fix: handle context errors as transient errors (#6850) * handle context errors as transient errors Signed-off-by: TomerJLevy <TomerJlevy@gmail.com> * add test cases Signed-off-by: TomerJLevy <TomerJlevy@gmail.com> * no need the new line Signed-off-by: TomerJLevy <TomerJlevy@gmail.com> * add release notes Signed-off-by: TomerJLevy <TomerJlevy@gmail.com> * Return the error as is Signed-off-by: TomerJLevy <TomerJlevy@gmail.com> * revert redundant changes Signed-off-by: TomerJLevy <TomerJlevy@gmail.com> * revert unrelated changes Signed-off-by: TomerJLevy <TomerJlevy@gmail.com> * revert more changes... Signed-off-by: TomerJLevy <TomerJlevy@gmail.com> --------- Signed-off-by: TomerJLevy <TomerJlevy@gmail.com> Co-authored-by: zirain <zirain2009@gmail.com> Signed-off-by: zirain <zirain2009@gmail.com> * bugfix: the controller cannot read the EnvoyProxy attached gatewayclass only. (#6838) Signed-off-by: zirain <zirain2009@gmail.com> * chore: fix CVE (#6903) Signed-off-by: Shahar Harari <shahar.harari@sap.com> Signed-off-by: zirain <zirain2009@gmail.com> * fix: nil pointer dereference in btp configmap indexer (#6921) Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> Signed-off-by: zirain <zirain2009@gmail.com> * improve targetRef selection for targetSelectors (#6917) * improve targetRef selection for targetSelectors * only select refs in the same namespace as the policy Signed-off-by: Arko Dasgupta <arko@tetrate.io> * fix lint Signed-off-by: Arko Dasgupta <arko@tetrate.io> --------- Signed-off-by: Arko Dasgupta <arko@tetrate.io> Signed-off-by: zirain <zirain2009@gmail.com> * fix: suppress lua validation logs (#6929) Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> Signed-off-by: zirain <zirain2009@gmail.com> * fix: rm incorrectly set exclusiveMaximum field in CRD (#6926) * fix: rm incorrectly set exclusiveMaximum field in CRD * Also fix maximum value to 599 which includes 599 as a valid num Fixes: #6925 Signed-off-by: Arko Dasgupta <arko@tetrate.io> Signed-off-by: zirain <zirain2009@gmail.com> * fix: rm Strict SameSite default (#6941) * by default it should be unset which implies `Lax` Relates to #6347 Signed-off-by: Arko Dasgupta <arko@tetrate.io> Signed-off-by: zirain <zirain2009@gmail.com> * Optimize pod cache (#6936) * Optimize pod cache Signed-off-by: jukie <10012479+Jukie@users.noreply.github.com> * release note Signed-off-by: jukie <10012479+Jukie@users.noreply.github.com> * Remove retry Signed-off-by: jukie <10012479+Jukie@users.noreply.github.com> * cleanup Signed-off-by: jukie <10012479+Jukie@users.noreply.github.com> --------- Signed-off-by: jukie <10012479+Jukie@users.noreply.github.com> Signed-off-by: Isaac <10012479+jukie@users.noreply.github.com> Co-authored-by: zirain <zirain2009@gmail.com> Signed-off-by: zirain <zirain2009@gmail.com> * reduce DeepCopy in gateway-api layer (#6940) * reduce deep copy in gateway-api layer * also fixed the DeepCopy implementation for ControllerResources which was performing a Shallow Copy resulting it lack of isolation b/w provider and gateway-api layer Relates to #6919 Signed-off-by: Arko Dasgupta <arko@tetrate.io> * fix lint Signed-off-by: Arko Dasgupta <arko@tetrate.io> --------- Signed-off-by: Arko Dasgupta <arko@tetrate.io> Signed-off-by: zirain <zirain2009@gmail.com> * fix: validation for grpc routes with extension ref filters (#6949) Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> Signed-off-by: zirain <zirain2009@gmail.com> * fix: cleanup dangling route status conditions (#6812) Signed-off-by: y-rabie <youssef.rabie@procore.com> Signed-off-by: zirain <zirain2009@gmail.com> * Fix: Add missing patch annotations to Compression struct for proper Merge (#6951) * fix: merge compression annotation Signed-off-by: sudipto baral <sudiptobaral.me@gmail.com> * test: add more compression merge test cases Signed-off-by: sudipto baral <sudiptobaral.me@gmail.com> --------- Signed-off-by: sudipto baral <sudiptobaral.me@gmail.com> Signed-off-by: zirain <zirain2009@gmail.com> * fix: update distroless image to resolve glibc CVEs (#6953) Signed-off-by: Shahar Harari <shahar.harari@sap.com> Signed-off-by: zirain <zirain2009@gmail.com> * chore: bump golang to 1.24.7 (#6959) chore: bump golang Signed-off-by: zirain <zirain2009@gmail.com> * fix: Make sure proxy protocol filter is the first listener filter (#6972) Fixes: #6873 Signed-off-by: Arko Dasgupta <arko@tetrate.io> Co-authored-by: Jacob Neil Taylor <me@jacobtaylor.id.au> Signed-off-by: zirain <zirain2009@gmail.com> * release notes Signed-off-by: zirain <zirain2009@gmail.com> * Removes reflection from RouteContext to reduce allocations (#6820) * bench: adds APIToXDS bench & small opt Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com> * no refect goos: darwin goarch: arm64 pkg: github.com/envoyproxy/gateway/test/gobench cpu: Apple M1 Pro │ old.txt │ new.txt │ │ sec/op │ sec/op vs base │ GatewayAPItoXDS/small-10 881.2µ ± 7% 803.4µ ± 1% -8.82% (p=0.000 n=10) GatewayAPItoXDS/medium-10 4.130m ± 26% 2.959m ± 4% -28.36% (p=0.000 n=10) GatewayAPItoXDS/large-10 5.375 ± 2% 4.553 ± 1% -15.28% (p=0.000 n=10) geomean 26.94m 22.12m -17.90% │ old.txt │ new.txt │ │ B/op │ B/op vs base │ GatewayAPItoXDS/small-10 507.2Ki ± 0% 492.9Ki ± 0% -2.83% (p=0.000 n=10) GatewayAPItoXDS/medium-10 2.545Mi ± 7% 1.954Mi ± 2% -23.21% (p=0.000 n=10) GatewayAPItoXDS/large-10 2.832Gi ± 0% 2.831Gi ± 0% ~ (p=0.529 n=10) geomean 15.40Mi 13.97Mi -9.31% │ old.txt │ new.txt │ │ allocs/op │ allocs/op vs base │ GatewayAPItoXDS/small-10 8.328k ± 0% 8.017k ± 0% -3.73% (p=0.000 n=10) GatewayAPItoXDS/medium-10 39.45k ± 6% 29.74k ± 2% -24.60% (p=0.000 n=10) GatewayAPItoXDS/large-10 38.75M ± 0% 38.71M ± 0% -0.11% (p=0.000 n=10) geomean 233.5k 209.8k -10.16% Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com> * removes garbage Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com> * more tests Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com> * more tests Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com> --------- Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com> --------- Signed-off-by: Guy Daich <guy.daich@sap.com> Signed-off-by: zirain <zirain2009@gmail.com> Signed-off-by: y-rabie <youssef.rabie@procore.com> Signed-off-by: Windfarer <windfarer@gmail.com> Signed-off-by: jukie <10012479+Jukie@users.noreply.github.com> Signed-off-by: qicz <qiczzhu@gmail.com> Signed-off-by: qi <qiczzhu@gmail.com> Signed-off-by: TomerJLevy <TomerJlevy@gmail.com> Signed-off-by: Shahar Harari <shahar.harari@sap.com> Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> Signed-off-by: Arko Dasgupta <arko@tetrate.io> Signed-off-by: Isaac <10012479+jukie@users.noreply.github.com> Signed-off-by: sudipto baral <sudiptobaral.me@gmail.com> Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com> Co-authored-by: Guy Daich <guy.daich@sap.com> Co-authored-by: Youssef Rabie <youssef.rabie@procore.com> Co-authored-by: Windfarer <windfarer@gmail.com> Co-authored-by: Isaac <10012479+jukie@users.noreply.github.com> Co-authored-by: qi <qiczzhu@gmail.com> Co-authored-by: TomerJLevy <TomerJlevy@gmail.com> Co-authored-by: shahar-h <shahar.harari@sap.com> Co-authored-by: Rudrakh Panigrahi <rudrakh97@gmail.com> Co-authored-by: Arko Dasgupta <arkodg@users.noreply.github.com> Co-authored-by: Sudipto Baral <sudiptobaral.me@gmail.com> Co-authored-by: Jacob Neil Taylor <me@jacobtaylor.id.au> Co-authored-by: Takeshi Yoneda <t.y.mathetake@gmail.com>

arkodg requested a review from a team as a code owner September 11, 2025 02:06

arkodg marked this pull request as draft September 11, 2025 02:06

arkodg changed the title ~~reduce deep copy in gateway-api layer~~ reduce DeepCopy in gateway-api layer Sep 11, 2025

arkodg force-pushed the reduce-deepCopy-gwawpi branch from 32f2a9c to c62b833 Compare September 11, 2025 04:05

arkodg marked this pull request as ready for review September 11, 2025 04:05

arkodg requested review from a team and mathetake September 11, 2025 04:06

arkodg added the cherrypick/release-v1.5.1 label Sep 11, 2025

mathetake approved these changes Sep 11, 2025

View reviewed changes

zirain force-pushed the reduce-deepCopy-gwawpi branch from c62b833 to 636d203 Compare September 11, 2025 04:21

Xunzhuo previously approved these changes Sep 11, 2025

View reviewed changes

rudrakhp reviewed Sep 11, 2025

View reviewed changes

arkodg marked this pull request as draft September 11, 2025 16:15

arkodg dismissed Xunzhuo’s stale review via 451f781 September 11, 2025 18:47

arkodg force-pushed the reduce-deepCopy-gwawpi branch 2 times, most recently from 451f781 to 89b997e Compare September 11, 2025 18:54

arkodg removed the cherrypick/release-v1.5.1 label Sep 11, 2025

arkodg marked this pull request as ready for review September 11, 2025 19:10

arkodg force-pushed the reduce-deepCopy-gwawpi branch from df57c90 to ad483f1 Compare September 11, 2025 20:08

arkodg requested review from Xunzhuo and rudrakhp September 11, 2025 20:21

zirain approved these changes Sep 11, 2025

View reviewed changes

rudrakhp approved these changes Sep 12, 2025

View reviewed changes

Arko Dasgupta added 2 commits September 12, 2025 17:14

reduce deep copy in gateway-api layer

8e2f81c

* also fixed the DeepCopy implementation for ControllerResources which was performing a Shallow Copy resulting it lack of isolation b/w provider and gateway-api layer Relates to envoyproxy#6919 Signed-off-by: Arko Dasgupta <arko@tetrate.io>

fix lint

e4a08a4

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

zirain force-pushed the reduce-deepCopy-gwawpi branch from ad483f1 to e4a08a4 Compare September 12, 2025 09:14

zirain enabled auto-merge (squash) September 12, 2025 09:14

zirain merged commit 106dba3 into envoyproxy:main Sep 12, 2025
51 of 53 checks passed

arkodg deleted the reduce-deepCopy-gwawpi branch September 15, 2025 03:56

arkodg added the cherrypick/release-v1.5.1 label Sep 15, 2025

zirain mentioned this pull request Sep 16, 2025

[release/v1.5] cherry pick for v1.5.1 #6974

Closed

zirain mentioned this pull request Sep 16, 2025

[release/v1.5] cherry pick for v1.5.1 #6975

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

reduce DeepCopy in gateway-api layer#6940

reduce DeepCopy in gateway-api layer#6940
zirain merged 2 commits intoenvoyproxy:mainfrom
arkodg:reduce-deepCopy-gwawpi

arkodg commented Sep 11, 2025 •

edited

Loading

Uh oh!

codecov bot commented Sep 11, 2025 •

edited

Loading

Uh oh!

Xunzhuo left a comment

Uh oh!

rudrakhp Sep 11, 2025

Uh oh!

arkodg Sep 11, 2025

Uh oh!

rudrakhp Sep 12, 2025 •

edited

Loading

Uh oh!

arkodg Sep 12, 2025 •

edited

Loading

Uh oh!

zirain commented Sep 12, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

arkodg commented Sep 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Sep 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Xunzhuo left a comment

Choose a reason for hiding this comment

Uh oh!

rudrakhp Sep 11, 2025

Choose a reason for hiding this comment

Uh oh!

arkodg Sep 11, 2025

Choose a reason for hiding this comment

Uh oh!

rudrakhp Sep 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arkodg Sep 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zirain commented Sep 12, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

arkodg commented Sep 11, 2025 •

edited

Loading

codecov bot commented Sep 11, 2025 •

edited

Loading

rudrakhp Sep 12, 2025 •

edited

Loading

arkodg Sep 12, 2025 •

edited

Loading