perf: speed up EncodeSourceVersion by multiple optimizations
#4533
Conversation
📝 WalkthroughWalkthroughThe code updates the source directory traversal in the Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Source
participant OS
Caller->>Source: EncodeSourceVersion()
Source->>OS: WalkDir(sourceDir, callback)
loop For each entry
OS-->>Source: path, DirEntry, err
alt Entry is directory and name is .terragrunt-cache or .terraform
Source->>OS: SkipDir
else Entry is file
Source->>OS: Stat(path)
OS-->>Source: FileInfo
Source->>Source: Update hash with file mod time
end
end
Source-->>Caller: hash value
Caller->>Source: WriteVersionFile()
Source->>OS: Write file with 0o640 permissions
OS-->>Source: Success/Failure
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
1. Use `WalkDir` to only call `os.Stat` when needed (i.e. when access to file modification time is needed). Previously, it was called on all entries. This cut `EncodeSourceVersion` time in one of our private workspaces from 43s to 34s. 2. Skip walking cache directories completely by returning `filepath.SkipDir` when hitting one. Previously, we just returned nil which meant we still had to walk everything in the file tree. This cut `EncodeSourceVersion` time in one of our private workspaces from 34s to ~nil.
healthy-pod
force-pushed
the
250711.optimize-encode-source-version
branch
from
83e47b2 to
6801ed6
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tf/source.go(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
🧠 Learnings (1)
tf/source.go (3)
Learnt from: yhakbar
PR: gruntwork-io/terragrunt#4169
File: cli/commands/hcl/validate/cli.go:29-60
Timestamp: 2025-04-17T13:02:28.098Z
Learning: Avoid shadowing imported packages with local variables in Go code, such as using a variable named `flags` when the `github.com/gruntwork-io/terragrunt/cli/flags` package is imported. Use more specific variable names like `flagSet` instead.
Learnt from: wakeful
PR: gruntwork-io/terragrunt#4491
File: test/integration_test.go:4265-4265
Timestamp: 2025-07-03T22:05:07.356Z
Learning: Constants defined in one Go file are accessible from other files in the same package without explicit imports. When reviewing Go code, consider the package-level scope rather than just the individual file scope.
Learnt from: levkohimins
PR: gruntwork-io/terragrunt#3723
File: cli/commands/stack/action.go:160-160
Timestamp: 2025-02-10T13:36:19.542Z
Learning: The project uses a custom error package `github.com/gruntwork-io/terragrunt/internal/errors` which provides similar functionality to `fmt.Errorf` but includes stack traces. Prefer using this package's error functions (e.g., `errors.Errorf`, `errors.New`) over the standard library's error handling.
🧬 Code Graph Analysis (1)
tf/source.go (1)
util/file.go (2)
IsDir(184-187)TerragruntCacheDir(27-27)
🔇 Additional comments (4)
tf/source.go (4)
7-7: LGTM: Import addition necessary for fs.DirEntryThe
io/fsimport is required for thefs.DirEntrytype used in thefilepath.WalkDircallback function.
91-91: LGTM: Excellent performance optimizationSwitching from
filepath.Walktofilepath.WalkDiris a great optimization that avoids unnecessaryos.Statcalls on every directory entry. This aligns perfectly with the PR objective of reducing stat calls.
110-114: LGTM: Proper file info handling with error checkingThe code correctly calls
os.Stat(path)only when file information is actually needed (for files, not directories) and properly handles any errors that might occur. This maintains the same functionality while improving performance.
152-152: LGTM: Modernized octal literal syntaxGood modernization to use Go 1.13+ octal literal syntax
0o640instead of the older0640format.
| if entry.IsDir() { | ||
| // avoid checking files in .terragrunt-cache directory since contents is auto-generated | ||
| if strings.Contains(path, util.TerragruntCacheDir) { | ||
| return filepath.SkipDir | ||
| } | ||
| // avoid checking files in .terraform directory since contents is auto-generated | ||
| if entry.Name() == util.TerraformLockFile { | ||
| return filepath.SkipDir | ||
| } | ||
| // We don't use any info from directories to calculate our hash | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Fix the .terraform directory check logic
The directory skipping logic correctly uses filepath.SkipDir for better performance, but there's an issue with the .terraform directory check.
The code checks entry.Name() == util.TerraformLockFile, but based on the comment "avoid checking files in .terraform directory", this should be checking for the directory name ".terraform" instead of the lock file name.
Apply this diff to fix the directory check:
- // avoid checking files in .terraform directory since contents is auto-generated
- if entry.Name() == util.TerraformLockFile {
- return filepath.SkipDir
- }
+ // avoid checking files in .terraform directory since contents is auto-generated
+ if entry.Name() == ".terraform" {
+ return filepath.SkipDir
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if entry.IsDir() { | |
| // avoid checking files in .terragrunt-cache directory since contents is auto-generated | |
| if strings.Contains(path, util.TerragruntCacheDir) { | |
| return filepath.SkipDir | |
| } | |
| // avoid checking files in .terraform directory since contents is auto-generated | |
| if entry.Name() == util.TerraformLockFile { | |
| return filepath.SkipDir | |
| } | |
| // We don't use any info from directories to calculate our hash | |
| return nil | |
| } | |
| if entry.IsDir() { | |
| // avoid checking files in .terragrunt-cache directory since contents is auto-generated | |
| if strings.Contains(path, util.TerragruntCacheDir) { | |
| return filepath.SkipDir | |
| } | |
| // avoid checking files in .terraform directory since contents is auto-generated | |
| if entry.Name() == ".terraform" { | |
| return filepath.SkipDir | |
| } | |
| // We don't use any info from directories to calculate our hash | |
| return nil | |
| } |
🤖 Prompt for AI Agents
In tf/source.go around lines 97 to 108, the code incorrectly checks for the
.terraform directory by comparing entry.Name() to util.TerraformLockFile, which
is a file name, not a directory. To fix this, change the condition to check if
entry.Name() equals the string ".terraform" to properly skip the .terraform
directory using filepath.SkipDir.
|
Hi, Looks like TestInitSkipCache is failing |
Description
Use
WalkDirto only callos.Statwhen needed (i.e. when access to file modification time is needed). Previously, it was called on all entries. This cutEncodeSourceVersiontime in one of our private workspaces from 43s to 34s.Skip walking cache directories completely by returning
filepath.SkipDirwhen hitting one. Previously, we just returned nil which meant we still had to walk everything in the file tree.This cut
EncodeSourceVersiontime in one of our private workspaces from 34s to ~nil.TODOs
Read the Gruntwork contribution guidelines.
Release Notes (draft)
Updated
EncodeSourceVersionto make lessstatcalls and skip walking cache dirs.Migration Guide
Summary by CodeRabbit
Bug Fixes
.terragrunt-cacheand.terraformdirectories when processing local sources.Chores