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

evalv3: severe performance regression #3307

Closed
gotwarlost opened this issue Jul 18, 2024 · 5 comments
Closed

evalv3: severe performance regression #3307

gotwarlost opened this issue Jul 18, 2024 · 5 comments

Comments

@gotwarlost
Copy link

What version of CUE are you using (cue version)?

$ cue version
cue version v0.9.2

go version go1.22.5
      -buildmode exe
       -compiler gc
  DefaultGODEBUG httplaxcontentlength=1,httpmuxgo121=1,tls10server=1,tlsrsakex=1,tlsunsafeekm=1
     CGO_ENABLED 1
          GOARCH arm64
            GOOS darwin
cue.lang.version v0.9.2

Does this issue reproduce with the latest stable release?

Yes, but only with the new evaluator.

What did you do?

time cue eval ./repro

# real	0m0.058s
# user	0m0.025s
# sys	0m0.017s

time CUE_EXPERIMENT=evalv3 cue eval ./repro

#real	0m16.459s
#user	0m29.155s
#sys	0m11.458s

# This can be made arbitrarily slow by adding more fields to the structs
-- repro/one.cue --
package repro

if features.enabled {
	l0: l1: l2: l3: r1: {
		k1: "v1"
		k2: "v2"
		k3: "v3"
		if addK100 {
			k100: true
		}
		k4: "v4"
		k5: "v5"
		k6: "v6"
		k7: "v7"
		k8: "v8"
	}
	l0: l1: l2: l3: r2: {
		k1: "v1"
		k2: "v2"
		k3: "v3"
		if addK100 {
			k100: true
		}
		k4: "v4"
		k5: "v5"
		k6: "v6"
		k7: "v7"
	}
}
-- repro/two.cue --
package repro

#request: {...}
features: #request.features
addK100:  #request.k100

What did you expect to see?

Evaluation returning sub-second like the old evaluator.

What did you see instead?

Evaluation takes 16s for the files above. Adding more [k,v] fields can make this arbitrarily slow and not return.

To be clear, on our large package cue def --inline-imports . (which is the command I started with) never returns.

Even cue eval seems to have the same problem.

@gotwarlost gotwarlost added NeedsInvestigation Triage Requires triage/attention labels Jul 18, 2024
@myitcv myitcv added evaluator and removed Triage Requires triage/attention labels Jul 19, 2024
@myitcv
Copy link
Member

myitcv commented Jul 19, 2024

@gotwarlost thanks very much again for going to the effort of reducing the problem here, and for joining the dots to the original cause (cue def --inline-imports).

@mpvl
Copy link
Member

mpvl commented Aug 12, 2024

Interesting regression. Upon analysis, this evaluation does not result in a high number of core operation. A very low one indeed, actually. Some profiling is needed here.

@gotwarlost
Copy link
Author

I must be holding something wrong but I don't see the perf improve when running the latest code from master.

$ cue version
cue version v0.11.0-0.dev.0.20240815170408-67f508f9935b

go version go1.22.5
      -buildmode exe
       -compiler gc
     CGO_ENABLED 1
          GOARCH arm64
            GOOS darwin
cue.lang.version v0.11.0

$ time cue eval .

real	0m0.047s
user	0m0.020s
sys	0m0.014s

$ time CUE_EXPERIMENT=evalv3 cue eval .

real	0m20.444s
user	0m24.091s
sys	0m15.002s

@myitcv
Copy link
Member

myitcv commented Aug 19, 2024

I must be holding something wrong but I don't see the perf improve when running the latest code from master.

Neither do I. Hence re-opening this issue, @mpvl.

@myitcv myitcv reopened this Aug 19, 2024
@cuematthew
Copy link
Contributor

Yep, it was fixed briefly, but it turns out the fix was far too fragile. A better fix is in progress. Apologies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants