Skip to content

Fixed potential issue on single core machine. #58

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Feb 10, 2015
Merged

Fixed potential issue on single core machine. #58

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion sort/sort.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,13 @@ func MultithreadedSortComparators(comparators Comparators) Comparators {
copy(toBeSorted, comparators)

var wg sync.WaitGroup
chunks := chunk(toBeSorted, int64(runtime.NumCPU()))

numCPU := int64(runtime.NumCPU())
if numCPU%2 == 1 { // single core machine
numCPU++
}
Copy link
Contributor

@alexandercampbell-wk alexandercampbell-wk Feb 10, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why this works. Can you explain?

Either way, it would be nice to wrap our NumCPU() calls in some utility function that would do this for us.

Copy link
Contributor Author

@dustinhiatt-wf dustinhiatt-wf Feb 10, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're just checking to see if there are an odd number of CPUs. In most cases a user will have 1, 2 and then some multiple of 2 thereafter. If someone intentionally disabled a CPU to get an odd number (someone who has more than one core) this will also even that out.

Copy link
Contributor

@tannermiller-wf tannermiller-wf Feb 10, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, and AMD at least went through a phase where they sold triple-core processors. I actually have one at home in my desktop; fourth core unlocked and overclocked of course 😄


chunks := chunk(toBeSorted, numCPU)
wg.Add(len(chunks))
for i := 0; i < len(chunks); i++ {
go func(i int) {
Expand Down