-
Notifications
You must be signed in to change notification settings - Fork 155
Add multi-threading for pack/unpack #1029
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add multi-threading for pack/unpack #1029
Conversation
Signed-off-by: Keerthan KK <[email protected]>
…rresponding goroutine Signed-off-by: Keerthan KK <[email protected]>
Signed-off-by: Keerthan KK <[email protected]>
…ches cant be called concurrently Signed-off-by: Keerthan KK <[email protected]>
gorkem
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some critical issues to address and also missing
- Race detector tests (go test -race)
- Concurrent layer ordering tests
- Performance benchmarks
|
|
||
| func (pm *ignorePaths) Matches(path, layerPath string) (bool, error) { | ||
| pm.mu.Lock() | ||
| defer pm.mu.Unlock() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
saveContentLayer -> packLayerToTar -> writeLayerToTar
writeLayerToTar calls ignore.Matches` for each file With the mutex, all file traversal becomes serialized. Parallel implementation may be SLOWER than sequential due to lock contention.
| if err != nil { | ||
| return err | ||
| } | ||
| kitfile.Model.Parts[index].LayerInfo = layerInfo |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is introducing concurrent writes to shared Kitfile fields without synchronization, causing potential data races and corruption. The same pattern is repeated on other layers too.
| } | ||
|
|
||
| if err = eg.Wait(); err != nil { | ||
| close(results) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This closes the results channel and returns. However, some goroutines might still be running and will panic when trying to send to the closed channel.
Description
This PR introduces parallel processing of pack and unpack operations. Previously the layers were packed and unpacked sequentially.
Linked issues
closes #254
AI-Assisted Code