-
-
Notifications
You must be signed in to change notification settings - Fork 3.2k
script: implement navigator.hardwareConcurrency #31268
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
Conversation
components/script/Cargo.toml
Outdated
@@ -75,6 +75,7 @@ mime = { workspace = true } | |||
mime_guess = { workspace = true } | |||
msg = { workspace = true } | |||
net_traits = { workspace = true } | |||
num_cpus = "1.1.0" |
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.
Maybe is good to create an issue and link this PR
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.
Do you mean I should create an issue in num_cpus
? I don't see why this PR would warrant creating an issue.
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.
no I meant on this repo
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.
Nice! Just a few comments.
@@ -24,6 +27,13 @@ use crate::dom::window::Window; | |||
use crate::dom::xrsystem::XRSystem; | |||
use crate::script_runtime::JSContext; | |||
|
|||
pub(super) fn hardware_concurrency() -> u64 { |
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.
Why is this function public? Could it simply be moved into the body of HardwareConcurrency(&self)
? Since the code is not repeated, I'm not sure an extra function buys us much unless I'm misunderstanding.
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.
It's also called in components/script/dom/workernavigator.rs
- I split it out to another function to avoid duplicating the logic in the Navigator
and WorkerNavigator
.
Signed-off-by: syvb <me@iter.ca>
01f96a8
to
382cf13
Compare
Signed-off-by: syvb <me@iter.ca>
Implements
navigator.hardwareConcurrency
, which returns the number of logical processors available.I used the
num_cpus
library (which is already used by Servo in other places), and cache the result since it can be slow to query that information on some operating systems../mach build -d
does not report any errors./mach test-tidy
does not report any errors