cue icon indicating copy to clipboard operation
cue copied to clipboard

evaluator: performance pathology when evaluating field comprehension, resolved by evalv3

Open acln0 opened this issue 1 year ago • 1 comments

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

$ cue version
cue version v0.9.0-alpha.5

go version go1.22.0
      -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.0

Does this issue reproduce with the latest stable release?

Yes, I discovered it on v0.8.2 initially, which behaves the same way.

What did you do?

Please see grpc_reproducer.txt. I discovered what seems to be a bug or a performance pathology in the evaluator when using the field comprehension (descriptor.packageName): descriptor on line 52 of grpc.cue (line 55 of the attached text file). Evaluation does not seem to finish in any reasonable amount of time, but I've only left it running for at most a couple of minutes, so I don't know if it eventually terminates. Commenting out the apiDescriptors declaration makes evaluation terminate quickly, as expected.

Note that evalv3 fixes the problem for the attached reproducer, but I am filing this issue anyway, just in case it is informative in some way. Apologies if it is not, or if this report is a duplicate, but I could not find something similar in a reasonable amount of time when I searched the issues.

What did you expect to see?

An invocation of cue eval -c which terminates in a reasonable amount of time. With CUE_EXPERIMENT=evalv3, this is indeed what I see.

What did you see instead?

cue eval -c hangs for a long time unless I am using evalv3.

acln0 avatar May 31 '24 06:05 acln0

Thanks for filing this issue! It is indeed useful and informative even if we can just close it as "resolved by the new evaluator" :) I will leave it open for the time being, as the new evaluator is not on by default yet.

mvdan avatar May 31 '24 10:05 mvdan

I'm going to close this one out just like the other "evalv3-win" issues we have labelled and closed. Unlike the others, I'm not adding a regression test, because the CUE involved is very long at nearly 4k lines of code, and it is too slow on evalv2, whereas we still test on both evaluator versions for the foreseeable future.

Thanks again for filing this!

mvdan avatar Sep 06 '24 14:09 mvdan