feat: check headers concurrently #257
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds concurrent header checking to improve performance by processing files in parallel using goroutines and errgroup. The change updates the Check function to check license headers across files concurrently with a limit based on GOMAXPROCS.
Changes:
- Introduced concurrent file processing using
golang.org/x/sync/errgroup - Set concurrency limit to
runtime.GOMAXPROCS(0) - Modified the file checking loop to spawn goroutines for parallel execution
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| g := new(errgroup.Group) | ||
| g.SetLimit(runtime.GOMAXPROCS(0)) | ||
|
|
||
| for _, file := range fileList { | ||
| if err := CheckFile(file, config, result); err != nil { | ||
| return err | ||
| } | ||
| f := file | ||
| g.Go(func() error { | ||
| return CheckFile(f, config, result) | ||
| }) | ||
| } |
There was a problem hiding this comment.
The Result struct methods (Succeed, Fail, Ignore) perform unsynchronized slice appends in concurrent goroutines, causing race conditions. The Result type (lines 32-42 in result.go) uses direct slice appends without any synchronization mechanism like sync.Mutex. This will lead to data races and potentially lost results or corrupted data when multiple goroutines write to the same Result instance simultaneously.
There was a problem hiding this comment.
It is. I didn't look closely enough into the Result struct. I pushed a version which adds a mutex to make the Result struct's associated methods thread-safe. Accessing the inner slices directly is still not thread-safe but the code does not do anything with the slices directly while the concurrent checks are happening so I think it should be ok
ZacBlanco
force-pushed
the
check-parallel
branch
from
be68d36 to
f46e7ee
Compare
3 participants
Closes apache/skywalking#13667