perf: skip comment assignment when no comments exist (#1473)
In a large repo this made a _minor_ difference in memory consumption on
gazelle invocations. This is minor enough it may not be worth it, or
maybe only the first commit is worth the extra code.
The first commit is simpler and has the largest gain in my large repo,
but doesn't cover as wide of a range of cases.
Robot summary of results:
```
┌────────────────────────────────────────────────────┬───────────────┬─────────┐
│ build │ order() alloc │ vs base │
├────────────────────────────────────────────────────┼───────────────┼─────────┤
│ base (no buildtools change) │ 137.6 MB │ — │
├────────────────────────────────────────────────────┼───────────────┼─────────┤
│ commit 1 (guard: skip when no comments) │ 60.7 MB │ −56% │
├────────────────────────────────────────────────────┼───────────────┼─────────┤
│ commit 1 + 2 (guard + finer-grained split) │ 42.1 MB │ −69% │
└────────────────────────────────────────────────────┴───────────────┴─────────┘
```
## Buildtools PR checklist
- [x] The code in this PR is covered by unit/integration tests.
- [x] I have tested these changes and provide testing instructions
below.
- [x] I have either responded to, or resolved all Gemini comments on the
PR.
- [x] I have read Google Eng Practices on [Small
Changes](https://google.github.io/eng-practices/review/developer/small-cls.html),
this PR either follows these guidelines or the description provides
reasoning for why they can not be followed.
## Description
`assignComments` previously walked the full syntax tree on every parse,
building preorder and postorder node lists used to attach line and
suffix comments.
This change skips the walking and `pre/post` construction when no
comments exists.
### (optional) These changes were tested using the following steps
bazel tests, patching the go.mod when invoking gazelle in a large repo
while profiling memory/GC
diff --git a/build/lex.go b/build/lex.go
index f56fe2f..0fba2e3 100644
--- a/build/lex.go
+++ b/build/lex.go
@@ -738,7 +738,7 @@
// order walks the expression adding it and its subexpressions to the
// preorder and postorder lists.
func (in *input) order(v Expr) {
- if v != nil {
+ if len(in.lineComments) > 0 && v != nil {
in.pre = append(in.pre, v)
}
switch v := v.(type) {
@@ -874,17 +874,29 @@
in.order(s)
}
}
- if v != nil {
+ if len(in.suffixComments) > 0 && v != nil {
in.post = append(in.post, v)
}
}
// assignComments attaches comments to nearby syntax.
func (in *input) assignComments() {
- // Generate preorder and postorder lists.
+ // Line comments are attached using the preorder list, suffix comments using
+ // the postorder list (order() builds only the list(s) for the comment kinds
+ // present). If the file has neither kind, skip the whole-tree walk entirely.
+ hasLine := len(in.lineComments) > 0
+ hasSuffix := len(in.suffixComments) > 0
+ if !hasLine && !hasSuffix {
+ return
+ }
+
in.order(in.file)
- in.assignSuffixComments()
- in.assignLineComments()
+ if hasSuffix {
+ in.assignSuffixComments()
+ }
+ if hasLine {
+ in.assignLineComments()
+ }
}
func (in *input) assignSuffixComments() {