feat: evaluate only the parameter/preset/tag closure when rendering - #221
feat: evaluate only the parameter/preset/tag closure when rendering#221PushTheLimit wants to merge 1 commit into
Conversation
Preview evaluates the entire Terraform module graph on every call, even though rendering a workspace form only needs coder_parameter, coder_workspace_preset and coder_workspace_tags (and what they reference). The resources a workspace would create cannot feed those blocks, so evaluating them is wasted work that dominates request latency on large templates. Pass OptionWithResourceClosure with the three target block types so the parser drops root-module resources that nothing in that closure references. On a real template this cuts EvaluateAll from ~2s to ~0.16s (~12x) with byte-identical parameters, presets and tags. Depends on the OptionWithResourceClosure addition in the trivy fork (coder/trivy#74). The trivy replace is temporarily pinned to that PR's commit; it will be moved to the merged coder/trivy commit before this merges.
f3a2bc9 to
adb5edc
Compare
Emyrk
left a comment
Emyrk
left a comment
There was a problem hiding this comment.
Have to block on the go mod update.
I'm curious how much of a speedup this is
| // Trivy has some issues that we're floating patches for, and will hopefully | ||
| // be upstreamed eventually. | ||
| replace github.com/aquasecurity/trivy => github.com/coder/trivy v0.0.0-20260309164037-c413f5a2f511 | ||
| replace github.com/aquasecurity/trivy => github.com/PushTheLimit/trivy v0.0.0-20260825213047-3916002b18a9 |
There was a problem hiding this comment.
We need to continue to use our fork. Is this just a matter of pulling their latest?
There was a problem hiding this comment.
@Emyrk I state this in the PR description:
Requires OptionWithResourceClosure from the trivy fork: coder/trivy#74. The trivy replace in go.mod is temporarily pinned to that PR's commit so this builds; it will be moved to the merged coder/trivy commit before this is taken out of draft.
There was a problem hiding this comment.
I do the same thing in the draft of the coder/coder PR: coder/coder#28602 that will also have to do the same thing if/once these upstream deps merge.
There was a problem hiding this comment.
👍
The PR here: https://github.com/coder/trivy/pull/74/changes
Does that only pull the change that adds OptionWithResourceClosure? Should we instead pull the latest trivy release and rebase the 2 coder commits ontop?
For our case with a template with a large Terraform module graph it went from ~4.5s per change to ~0.4s .. including the initial page load to the create workspace form. Big improvement. |
Oh that is huge and very worth it |
2 participants
What
Pass
OptionWithResourceClosure(["coder_parameter", "coder_workspace_preset", "coder_workspace_tags"])to the Terraform parser inPreview, so root-module resource blocks that nothing in the parameter/preset/tag closure references are skipped during evaluation.Why
Previewevaluates the entire module graph on every call, but rendering a workspace form only needs the parameter, preset and tag blocks and what they reference. The resources a workspace would create cannot feed those blocks, so evaluating them is wasted work that dominates request latency.Concretely this is what makes the Coder dynamic-parameters websocket slow: on a real Altana
nexus-stacktemplate,EvaluateAllis ~2s per request (prod ~4.5s), and it re-runs on every parameter change.Result (real nexus-stack template, local)
EvaluateAll~2.0s to ~0.16s (~12x)Dependency
Requires
OptionWithResourceClosurefrom the trivy fork: coder/trivy#74. The trivyreplaceingo.modis temporarily pinned to that PR's commit so this builds; it will be moved to the mergedcoder/trivycommit before this is taken out of draft.Testing
go build ./...and the non-E2E unit suite pass.coder/coderat v2.35.4:coderd/dynamicparameterstests pass against this change, with identical parameter output.