code-generator icon indicating copy to clipboard operation
code-generator copied to clipboard

performance drop with GO111MODULE="on"

Open BenTheElder opened this issue 5 years ago • 17 comments

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.

BenTheElder avatar May 08 '19 03:05 BenTheElder

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.

markmandel avatar May 09 '19 16:05 markmandel

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

BenTheElder avatar May 09 '19 16:05 BenTheElder

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

vincepri avatar Jun 24 '19 20:06 vincepri

@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 :sweat_smile: 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

BenTheElder avatar Jun 24 '19 20:06 BenTheElder

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.

vincepri avatar Jun 24 '19 20:06 vincepri

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

nikhita avatar Jun 25 '19 02:06 nikhita

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

vincepri avatar Jun 25 '19 03:06 vincepri

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

fejta-bot avatar Sep 23 '19 04:09 fejta-bot

/remove-lifecycle stale

nikhita avatar Sep 23 '19 04:09 nikhita

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

fejta-bot avatar Dec 22 '19 05:12 fejta-bot

/remove-lifecycle stale

maelvls avatar Dec 22 '19 10:12 maelvls

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

fejta-bot avatar Mar 21 '20 10:03 fejta-bot

/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...)

BenTheElder avatar Mar 23 '20 18:03 BenTheElder

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 ...

BenTheElder avatar Aug 06 '20 21:08 BenTheElder

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

ialidzhikov avatar Feb 05 '21 08:02 ialidzhikov

Could this change landing in Go 1.18 help here? 🤔 https://github.com/golang/go/issues/44435

MadhavJivrajani avatar Nov 08 '21 13:11 MadhavJivrajani

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.

BenTheElder avatar Nov 08 '21 15:11 BenTheElder

/close

thockin avatar Mar 12 '24 20:03 thockin

@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.

k8s-ci-robot avatar Mar 12 '24 20:03 k8s-ci-robot

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 ?

takirala avatar Apr 22 '24 10:04 takirala

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".

thockin avatar Apr 22 '24 15:04 thockin

thank you for clarification!

takirala avatar Apr 23 '24 03:04 takirala