8000 remove unnecessary loop and condition evaluation by ramaraochavali · Pull Request #56533 · istio/istio · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

remove unnecessary loop and condition evaluation #56533

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

remove unnecessary loop and condition evaluation #56533

wants to merge 3 commits into from
+7 −21

Conversation

ramaraochavali
Copy link
Contributor
@ramaraochavali ramaraochavali commented Jun 6, 2025 •
edited by istio-policy-bot
Loading

Please provide a description of this PR:

@ramaraochavali ramaraochavali requested a review from a team as a code owner June 6, 2025 09:09
@istio-policy-bot istio-policy-bot added the release-notes-none Indicates a PR that does not require release notes. label Jun 6, 2025
@linux-foundation-easycla Linux Foundation: EasyCLA
Copy link
linux-foundation-easycla bot commented Jun 6, 2025 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

@istio-testing istio-testing added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jun 6, 2025
@ramaraochavali
remove unnecessary loop and condition evaluation
36beb83 
Signed-off-by: Rama Chavali <ramaraochavali@gmail.com>
@ramaraochavali
Copy link
Contributor Author

/test integ-ambient-mc

@ramaraochavali
empty push
762b974 
Signed-off-by: Rama Chavali <rama.rao@salesforce.com>
@ramaraochavali
add comment
161acd0 
Signed-off-by: Rama Chavali <rama.rao@salesforce.com>
@Stevenjin8
Copy link
Contributor
Stevenjin8 commented Jun 9, 2025 •
edited
Loading

Not super familiar with the code, but I do think this changes how stats are emitted. For example, suppose the last cluster patch

cp.Operation == networking.EnvoyFilter_Patch_REMOVE &&
			commonConditionMatch(pctx, cp) &&
			clusterMatch(c, cp, hosts)

evaluates to true. Before, IncrementEnvoyFilterMetric(cp.Key(), Cluster, applied) would never be called. But with this PR, it could get called even if the cluster is eventually removed.

IMO this change in behavior is pretty inconsequential, but thought I would call it out.

@ramaraochavali
Copy link
Contributor Author

IMO this change in behavior is pretty inconsequential, but thought I would call it out.

Yes. I agree. Thanks for calling this out

@ramaraochavali
Copy link
Contributor Author

/test integ-ambient-mc

@ramaraochavali
Copy link
Contributor Author

@istio/wg-networking-maintainers PTAL

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
release-notes-none Indicates a PR that does not require release notes. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
4676
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ramaraochavali @Stevenjin8 @istio-testing @istio-policy-bot
0