Only use os.cpus() and filter out core like physical-cpu-count did #142
Conversation
This file contains 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
The problem is with heroku and some CI, the physical-cpu-count linux branch requires a binary and access not provided by these environments.
|
It will fix #133 |
devongovett
reviewed
Dec 9, 2017
src/WorkerFarm.js
Outdated
| @@ -86,4 +87,13 @@ for (let key in EventEmitter.prototype) { | |||
| WorkerFarm.prototype[key] = EventEmitter.prototype[key]; | |||
| } | |||
|
|
|||
| function getNumWorkers() { | |||
| const cores = os.cpus().filter(function(cpu, index) { | |||
| const hasHyperthreading = cpu.model.includes('Intel'); | |||
There was a problem hiding this comment.
Is this really a good assumption? Do all Intel CPUs have hyperthreading?
There was a problem hiding this comment.
That's the else catchall branch of physical-cpu-count.
I don't know
There was a problem hiding this comment.
also i’m pretty sure AMD’s newer Ryzen cpus have SMT, which is what Intel calls hyperthreading
There was a problem hiding this comment.
Here is a Gist of the aforementioned physical-cpu-count for reference. The Intel check seems to be the last-ditch effort.
|
Since function getNumWorkers() {
let cores;
try {
cores = require('physical-cpu-count');
} catch (err) {
cores = os.cpus();
}
return cores;
} |
|
Does it throw on Heroku or returns something else? |
albinotonnina
added a commit
to albinotonnina/parcel
that referenced
this pull request
Dec 10, 2017
|
You will still have that warning, but it does not break? |
|
@reel my bad, I lied! It works! my little heroku app gets built correctly: https://shrouded-ocean-46253.herokuapp.com/ Like you said, warning is still there but it doesn't break. log |
|
But, it was crashing before or just writing a msg? As this PR might not be required. |
|
Crashing! Sent with GitHawk |
devongovett
reviewed
Dec 11, 2017
src/WorkerFarm.js
Outdated
| try { | ||
| cores = require('physical-cpu-count'); | ||
| } catch (err) { | ||
| cores = os.cpus(); |
There was a problem hiding this comment.
should probably be os.cpus().length
devongovett
approved these changes
Dec 11, 2017
rakannimer
pushed a commit
to rakannimer/parcel
that referenced
this pull request
Dec 11, 2017
…arcel-bundler#142) * Only use os.cpus() and filter out core like physical-cpu-count did The problem is with heroku and some CI, the physical-cpu-count linux branch requires a binary and access not provided by these environments. * using physical-cpu-count and defaulting to os.cpus() * get the actual length, should be good
|
Has this been solved? I am trying to create dist folder on heroku and it gives the same message . |
|
@sohaibnehal I think 9d319af solves the issue |
devongovett
pushed a commit
that referenced
this pull request
Oct 15, 2018
) * Only use os.cpus() and filter out core like physical-cpu-count did The problem is with heroku and some CI, the physical-cpu-count linux branch requires a binary and access not provided by these environments. * using physical-cpu-count and defaulting to os.cpus() * get the actual length, should be good
devongovett
pushed a commit
that referenced
this pull request
Oct 15, 2018
) * Only use os.cpus() and filter out core like physical-cpu-count did The problem is with heroku and some CI, the physical-cpu-count linux branch requires a binary and access not provided by these environments. * using physical-cpu-count and defaulting to os.cpus() * get the actual length, should be good
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The problem is with heroku and some CI, the physical-cpu-count linux branch requires a binary and access not provided by these environments.