Go binary size regression #26232
Comments
|
istio-agent basically imports literally everything now since @costinm put an XDS server in it |
|
I think that's strange that the client agent is larger than the service. Go doesn't do a good job at code elimination as well. |
|
A minimalistic xDS server (e.g. go-control-plane test server) is around 22MB as a point of reference. |
|
We also import literally every proto in xds (including v2 and v4) so we can handle envoyfilters which adds a lot |
istio-policy-bot
added
lifecycle/needs-triage
and removed
lifecycle/needs-triage
labels
|
@brian-avery Checking against 1.7 branch. |
|
Here is the actual effect on memory. Unshared ~= resident - shared.
Unshared memory costs increase per pod, whereas shared memory costs increase per node. |
|
Can you share the commands you ran? Then we can compare some code changes
and see what makes improvements
…On Tue, Aug 11, 2020, 5:29 PM mandarjog ***@***.***> wrote:
Here is the actual effect on memory. Unshared ~= resident - shared.
TLDR;
1.7 Is using ~15 MB per pod more for istio-agent, which I don't think is
release blocking.
Istio Version File size Resident Memory Unshared memory Overage
1.5 68M 55M 15M 0
1.6 74M 55M 17M 2M
1.7 110M 80M 30M 15M
Unshared memory costs increase *per pod*, whereas shared memory costs
increase *per node*.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#26232 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAEYGXI4MZOSMBEHK3WQOVDSAHO7HANCNFSM4PW6A7SQ>
.
|
|
btw - go 1.14: 101M so some small wins.. |
We currently set `STATIC=0 LDFLAGS='-extldflags -static -s -w'` as part of the build. This does not make much sense, since STATIC sets LDFLAGS to empty, so we don't pass `-static -s -w`. I believe we do want to pass these flags, as they were originally added for a reason. Binary size changes for istioctl, agent, and pilot in mb: ``` 114 -> 81 104 -> 75 108 -> 77 ``` Helps with istio#26232
|
Notes to self: useful tools for analyzing: |
We currently set `STATIC=0 LDFLAGS='-extldflags -static -s -w'` as part of the build. This does not make much sense, since STATIC sets LDFLAGS to empty, so we don't pass `-static -s -w`. I believe we do want to pass these flags, as they were originally added for a reason. Binary size changes for istioctl, agent, and pilot in mb: ``` 114 -> 81 104 -> 75 108 -> 77 ``` Helps with #26232
Hi folks - as someone using istio with a lot of pods, a 25% increase in proxy usage, after we've just took a 10% increase 1.5 -> 1.6 for SDS, is certainly upgrade blocking for us. I have to be honest, as an end user I struggle to see how it wasn't release blocking either or as an absolute minimum, all over the upgrade notes in red to give folks a heads up - as it feels pretty substantial. |
Part of istio#26232 75mb -> 64mb We could have also done massive refactoring to avoid using build tags, but I think in the near future we will need build tags for dropping k8s import, so it seems useful to implement this at least for the short term.
|
Doing a bunch of work on https://github.com/howardjohn/istio/pull/new/agent/tiny-binary-full-branch to trim the deps a lot. Down to 37mb currently. 30mb seems feasible. It contains about 15 changes so I will be splitting it out into many smaller prs |
I am not sure what changed between this being added and now, but I am fairly sure this is not needed know, as its not possible to import collections without importing the packages. This is not strictly neccesary, but allows splitting the kubernetes imports out for istio#26232 much more easily.
|
That's awesome @howardjohn - guessing we're looking at 1.9 for most of them however? |
|
Yeah unfortunately so. The bulk of the changes are from dropping Kubernetes imports which is not feasible on older versions. |
* Trim agent binary by 11mb by conditionally compiled XDS filters Part of #26232 75mb -> 64mb We could have also done massive refactoring to avoid using build tags, but I think in the near future we will need build tags for dropping k8s import, so it seems useful to implement this at least for the short term. * Trim another 4mb
I am not sure what changed between this being added and now, but I am fairly sure this is not needed know, as its not possible to import collections without importing the packages. This is not strictly neccesary, but allows splitting the kubernetes imports out for #26232 much more easily.
|
Note sure how much this will impact once we are done trimming, but golang 1.16 will net us a few wins as well: |
…o#28670) * Trim agent binary by 11mb by conditionally compiled XDS filters Part of istio#26232 75mb -> 64mb We could have also done massive refactoring to avoid using build tags, but I think in the near future we will need build tags for dropping k8s import, so it seems useful to implement this at least for the short term. * Trim another 4mb (cherry picked from commit 19c8ea4)
…o#28670) * Trim agent binary by 11mb by conditionally compiled XDS filters Part of istio#26232 75mb -> 64mb We could have also done massive refactoring to avoid using build tags, but I think in the near future we will need build tags for dropping k8s import, so it seems useful to implement this at least for the short term. * Trim another 4mb (cherry picked from commit 19c8ea4)
…) (#28718) * Trim agent binary by 11mb by conditionally compiled XDS filters Part of #26232 75mb -> 64mb We could have also done massive refactoring to avoid using build tags, but I think in the near future we will need build tags for dropping k8s import, so it seems useful to implement this at least for the short term. * Trim another 4mb (cherry picked from commit 19c8ea4)
…) (#28717) * Trim agent binary by 11mb by conditionally compiled XDS filters Part of #26232 75mb -> 64mb We could have also done massive refactoring to avoid using build tags, but I think in the near future we will need build tags for dropping k8s import, so it seems useful to implement this at least for the short term. * Trim another 4mb (cherry picked from commit 19c8ea4)
* Move mesh configmap watcher out of the core `mesh` package * Move security kubernetes `util`s to its own package This drops the Kubernetes dependency for the agent completely. This essentially completes istio#26232 for the agent (but not Istiod).
* Move mesh configmap watcher out of the core `mesh` package * Move security kubernetes `util`s to its own package This drops the Kubernetes dependency for the agent completely. This essentially completes #26232 for the agent (but not Istiod).
…o#28670) * Trim agent binary by 11mb by conditionally compiled XDS filters Part of istio#26232 75mb -> 64mb We could have also done massive refactoring to avoid using build tags, but I think in the near future we will need build tags for dropping k8s import, so it seems useful to implement this at least for the short term. * Trim another 4mb
|
I am going with the assumption that we don't really care about istiod binary size and the agent is now a fraction of its size (38M), and considering this fixed |
Agent:
master: 110m
1.6: 74m
1.5: 68m
1.4: 38m
Pilot:
master: 108m
1.6: 90m
1.5: 84m
1.4: 65m
Measured with
My theory is the 1.4 -> 1.5 regression in agent is for gateway SDS (pulls in k8s) and then 1.6 -> master regression is
google.golang.org/api/compute/v1cc @rshriram @kyessenov
The text was updated successfully, but these errors were encountered: