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.

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

performance drop with GO111MODULE="on" #69

Closed
BenTheElder opened this issue May 8, 2019 · 22 comments
Closed

performance drop with GO111MODULE="on" #69

BenTheElder opened this issue May 8, 2019 · 22 comments
Labels
kind/bug Categorizes issue or PR as related to a bug. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery.

Comments

@BenTheElder
Copy link
Member

export GO111MODULE="on" and then run any of the code generators (so far defaulter-gen, deepcopy-gen, conversion-gen) for a pretty drastic increase in time to run vs export GO111MODULE="off" first.

I've been converting https://sigs.k8s.io/kind over to modules fully, and observed this performance issue, I've not yet had a chance to dig into why this occurs, but ensuring that module mode is set to off while calling the code generators seems to dramatically improve peformance.

@markmandel
Copy link
Contributor

This might be because go mod will download all the dependencies of the project, rather than looking at the vendor directory (if it hasn't pulled them down already?). May want to try with -mod=vendor enabled?

Just a thought.

@BenTheElder
Copy link
Member Author

I don't think that's it but I'm not 100% certain. The performance difference can be observed with a populated vendor and module cache just between running GO111MODULE="on" <code-generator> and GO111MODULE="off" <code-generator>. The code generator code performance seems to be tied to this.

So far a viable work around to embrace modules is:

  • build the code generators while module mode is enabled (so you can install the version your module tracks)
  • go mod vendor to populate vendor locally from your module cache
  • disable module mode export GO111MODULE="off"
  • fake a GOPATH with a tempdir and a symlink to the repo
  • run the generators
  • cleanup the fake gopath

This is a bit hacky, but allows everything to use modules and doesn't have the big performance penalty.

Implementation here:
https://github.com/kubernetes-sigs/kind/blob/10642f530782963ed23b3d4a14587950bbacaa63/hack/update-generated.sh

@vincepri
Copy link
Member

@BenTheElder Can you try with GO111MODULE="on" GOFLAGS="-mod=vendor" <code-generator> as well? We're about to hit this in cluster-api soon :)

@BenTheElder
Copy link
Member Author

BenTheElder commented Jun 24, 2019

@vincepri feel free to give it a shot in the kind repo, we're still using the workaround detailed above for now. I might not get to this for a bit myself 😅
The fake gopath part is pretty trivial and either way we need the rest of the wrapper to populate vendor etc. if it's going to use vendor instead of the module cache

@vincepri
Copy link
Member

Oops, sorry I missed the other comment above yours which suggested the same. I wonder how this will perform with the latest build of code-generator, which should be using tools/packages to retrieve the modules.

I'll update when I can test in CAPI or kind.

@nikhita
Copy link
Member

nikhita commented Jun 25, 2019

latest build of code-generator, which should be using tools/packages to retrieve the modules.

@vincepri do you have a link to an issue/PR about this? Afaik gengo, the main generation tool behind code-generator, would need to move to using tools/packages first and I don't think that's going to happen soon-ish because that would result in breaking lots of existing interfaces.

cc @sttts

@vincepri
Copy link
Member

@nikhita I think I might have talked too soon 😄. We should definitely get this on the roadmap, maybe with a feature flag and documentation to opt in?

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 23, 2019
@nikhita
Copy link
Member

nikhita commented Sep 23, 2019

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 23, 2019
@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 22, 2019
@maelvls
Copy link

maelvls commented Dec 22, 2019

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 22, 2019
@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Mar 21, 2020
@BenTheElder
Copy link
Member Author

/lifecycle frozen
this issue is not going away on it's own and it will probably bite us in the future, FYI @liggitt @dims (be aware of this when touching k/k build scripts...)

@k8s-ci-robot k8s-ci-robot added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Mar 23, 2020
@BenTheElder
Copy link
Member Author

FWIW i'm no longer using the vendor / GOPATH workaround in kind, but I've also changed a lot of things:

  • less code
  • less dependencies
  • less use of generators
  • go1.15rc1
    ...

abayer added a commit to abayer/lighthouse that referenced this issue Aug 19, 2020
Like from 5m37s to 7s on my laptop. Turning off `GO111MODULE` makes
for a huge improvement - see kubernetes/code-generator#69

Signed-off-by: Andrew Bayer <andrew.bayer@gmail.com>
jenkins-x-bot pushed a commit to jenkins-x/lighthouse that referenced this issue Aug 20, 2020
Like from 5m37s to 7s on my laptop. Turning off `GO111MODULE` makes
for a huge improvement - see kubernetes/code-generator#69

Signed-off-by: Andrew Bayer <andrew.bayer@gmail.com>
@ialidzhikov
Copy link
Contributor

/kind bug
/priority important-soon
/sig api-machinery

@k8s-ci-robot k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. labels Feb 5, 2021
ironcladlou added a commit to ironcladlou/hypershift that referenced this issue Oct 28, 2021
Optimize the API docs generation by disabling the use of Go modules
when running the generation tool. This is a workaround for performance
issues in the upstream libraries:

    kubernetes/gengo#147
    kubernetes/code-generator#69

Before this workaround:

    make api-docs  93.87s user 171.98s system 426% cpu 1:02.30 total

After the workaround:

    make api-docs  2.91s user 0.83s system 135% cpu 2.752 total

The hack seems worth the 97% improvement for iterating on docs.
ironcladlou added a commit to ironcladlou/hypershift that referenced this issue Oct 28, 2021
Optimize the API docs generation by disabling the use of Go modules
when running the generation tool. This is a workaround for performance
issues in the upstream libraries:

- kubernetes/gengo#147
- kubernetes/code-generator#69

Before this workaround:

    make api-docs  93.87s user 171.98s system 426% cpu 1:02.30 total

After the workaround:

    make api-docs  2.91s user 0.83s system 135% cpu 2.752 total

The hack seems worth the 97% improvement for iterating on docs.
ironcladlou added a commit to ironcladlou/hypershift that referenced this issue Oct 28, 2021
Optimize the API docs generation by disabling the use of Go modules
when running the generation tool. This is a workaround for performance
issues in the upstream libraries:

- kubernetes/gengo#147
- kubernetes/code-generator#69

Before this workaround:

    make api-docs  93.87s user 171.98s system 426% cpu 1:02.30 total

After the workaround:

    make api-docs  2.91s user 0.83s system 135% cpu 2.752 total

The hack seems worth the 97% improvement for iterating on docs.
@MadhavJivrajani
Copy link
Contributor

Could this change landing in Go 1.18 help here? 🤔
golang/go#44435

@BenTheElder
Copy link
Member Author

I don't think so. That issue seems to relate to populating dependencies in the module cache more quickly. Module mode slows the generators significantly independently of that step.

@thockin
Copy link
Member

thockin commented Mar 12, 2024

/close

@k8s-ci-robot
Copy link

@thockin: Closing this issue.

In response to this:

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@takirala
Copy link

any context around how/why this is closed ? Seems like there was no activity for three years and it was closed - is that it or it was "fixed" somehow ?

@thockin
Copy link
Member

thockin commented Apr 22, 2024

An answer in parts.

  1. Yes, module mode is slower because Go's libs internally call exec("go", "list", "-json") and then parse the results.
  2. It's not as much slower now as it was when this was filed.
  3. Continuing to support non-modules mode in k/k was a burden, which we have decided not to carry. We stopped running codegen on every make and we adopted go workspaces.

IOW, Go's packages and related libs could get faster, but we no longer consider it "a problem".

@takirala
Copy link

thank you for clarification!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery.
Projects
None yet
Development

No branches or pull requests