Skip to content
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

hand written IstioEndpoint deepcopy #52485

Merged
merged 3 commits into from
Aug 5, 2024
Merged

hand written IstioEndpoint deepcopy #52485

merged 3 commits into from
Aug 5, 2024
+36 −14

Conversation

wulianglongrd
Copy link
Member

@wulianglongrd wulianglongrd commented Aug 2, 2024 •
edited by istio-policy-bot
Loading

fix #52446

benchmark result:

goos: darwin
goarch: amd64
pkg: istio.io/istio/pilot/pkg/model
cpu: Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
                    │    old.txt     │               new.txt               │
                    │     sec/op     │    sec/op     vs base               │
EndpointDeepCopy-12   14235.5n ± 16%   271.2n ± 10%  -98.10% (p=0.002 n=6)

                    │   old.txt   │              new.txt              │
                    │    B/op     │    B/op     vs base               │
EndpointDeepCopy-12   6688.0 ± 0%   624.0 ± 0%  -90.67% (p=0.002 n=6)

                    │   old.txt    │              new.txt              │
                    │  allocs/op   │ allocs/op   vs base               │
EndpointDeepCopy-12   189.000 ± 0%   4.000 ± 0%  -97.88% (p=0.002 n=6)

@wulianglongrd wulianglongrd requested a review from a team as a code owner August 2, 2024 02:27
@istio-policy-bot istio-policy-bot added area/networking area/perf and scalability release-notes-none Indicates a PR that does not require release notes. labels Aug 2, 2024
@istio-testing istio-testing added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Aug 2, 2024
howardjohn
howardjohn reviewed Aug 2, 2024
View reviewed changes
Copy link
Member

@howardjohn howardjohn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wow a lot faster than I expected! and simple. nice!
would be good to run #52447 against this to verify (I can do it tomorrow if you have issues)

@zirain
Copy link
Member

zirain commented Aug 2, 2024 •
edited
Loading

should we use // +k8s:deepcopy-gen=true to generate it instead of hand writting?

@wulianglongrd
Copy link
Member Author

should we use // +k8s:deepcopy-gen=true to generate it instead of hand writting?

I am not familiar with this usage, can it be used for any structure? In istio, it is common to write DeepCopy by hand.

@istio-testing istio-testing merged commit ee1754a into istio:master Aug 5, 2024
27 checks passed
@wulianglongrd wulianglongrd deleted the istio-ep-deepcopy branch August 5, 2024 04:31
luksa pushed a commit to luksa/istio that referenced this pull request Oct 14, 2024
Automator: merge upstream changes to openshift-service-mesh/istio@master
ffba480 
* upstream/master: (67 commits)
  Add release notes for dual-stack support promotion (istio#52524)
  Automator: update proxy@master in istio/istio@master (istio#52515)
  add name (istio#52501)
  Allow setting resources to null in gateway chart (istio#52514)
  Fix replacements errors in helm charts and templates (istio#52459)
  hand written IstioEndpoint deepcopy (istio#52485)
  operator: remove `profile` commands (istio#52468)
  Automator: update proxy@master in istio/istio@master (istio#52506)
  Automator: update proxy@master in istio/istio@master (istio#52505)
  Automator: update proxy@master in istio/istio@master (istio#52504)
  Automator: update ztunnel@master in istio/istio@master (istio#52502)
  Fix typo of InsertDataToConfigMap func description (istio#52491)
  Fix stale GVK in benchmark test (istio#52482)
  Bug/stale cert expiration logs (istio#52466)
  Automator: update ztunnel@master in istio/istio@master (istio#52496)
  Automator: update proxy@master in istio/istio@master (istio#52495)
  operator: move proto from api to this repo (istio#52472)
  remove unneeded func isAllowedKubernetesAudience (istio#52489)
  operator: misc code cleanup (istio#52467)
  ambient: do not allow service waypoint to have a waypoint (istio#52480)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@howardjohn howardjohn

@hzxuzhonghu hzxuzhonghu

Assignees
No one assigned
Labels
area/networking area/perf and scalability release-notes-none Indicates a PR that does not require release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Move off copystructure
6 participants
@wulianglongrd @zirain @howardjohn @hzxuzhonghu @istio-testing @istio-policy-bot