Open
Conversation
With this change it will no longer start for each file a goroutine it will start fewer goroutines and never more than the system cpu cores count. This will increase the performance on single core systems because it won't create unnessesary goroutines and multi core systems will still benefit from the reduced amount of goroutines.
Refactor test to be easier to read.
Now if you give it 10 files and tell it to split it into 3 parts it will not longer split it into 2 parts. It will split it into 3 parts and add the remaining files to the first part -> the first part will have in this case one file more than the rest. This and some other cases are now covered by tests.
The name is now more descriptive and easier to understand.
gsquire
reviewed
Apr 22, 2021
| cpuCount := runtime.NumCPU() | ||
| filesPerCore := splitArrayIntoParts(files, cpuCount) | ||
|
|
||
| go func() { |
Owner
There was a problem hiding this comment.
I think a simpler approach would be to just spawn a new goroutine for each file and add each slice of *reports to another slice guarded by a mutex. Then when the WaitGroup is done(), iterate through the slice of all reports and dump that to stdout.
I don't know how much of an advantage the splitting gives us either as I usually let the Go runtime schedule goroutines for me. :)
Other than that I like the approach. I think if we pare it down to what I tried to describe, it'd be perfect.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is my implementation for the concurrent version that I offered inside #8.
Feel free to criticize my work and to propose changes 😃