From fed9e937d9f629d8672249d3d67763d4e783042c Mon Sep 17 00:00:00 2001 From: ecyrbe Date: Sun, 1 Mar 2020 04:17:37 +0100 Subject: [PATCH 1/4] fest(std/node): implement os.getPriority() and os.setPriority() --- Cargo.lock | 1 + cli/Cargo.toml | 1 + cli/js/deno.ts | 2 + cli/js/lib.deno.ns.d.ts | 6 ++ cli/js/os.ts | 10 +++ cli/js/os_test.ts | 5 ++ cli/lib.rs | 1 + cli/ops/os.rs | 34 +++++++++ cli/priority.rs | 159 ++++++++++++++++++++++++++++++++++++++++ std/node/os.ts | 24 ++++-- std/node/os_test.ts | 66 +++++++++++++---- 11 files changed, 289 insertions(+), 20 deletions(-) create mode 100644 cli/priority.rs diff --git a/Cargo.lock b/Cargo.lock index 16b00365a3cf6d..4d3c0ebe7e4cea 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -443,6 +443,7 @@ dependencies = [ "dirs", "dlopen", "dprint-plugin-typescript", + "errno", "futures 0.3.4", "fwdansi", "glob", diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 0115938f7bd8fa..a49d9b044db31b 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -34,6 +34,7 @@ clap = "2.33.0" dirs = "2.0.2" dlopen = "0.1.8" dprint-plugin-typescript = "0.7.0" +errno = "0.1.8" futures = { version = "0.3.1", features = [ "compat", "io-compat" ] } glob = "0.3.0" http = "0.2.0" diff --git a/cli/js/deno.ts b/cli/js/deno.ts index c563d5112cb19a..bcb0aeb7edd426 100644 --- a/cli/js/deno.ts +++ b/cli/js/deno.ts @@ -93,6 +93,8 @@ export { execPath, hostname, loadavg, + getPriority, + setPriority, osRelease } from "./os.ts"; export { diff --git a/cli/js/lib.deno.ns.d.ts b/cli/js/lib.deno.ns.d.ts index b2e67b2889e54d..90ae52c1da0fb4 100644 --- a/cli/js/lib.deno.ns.d.ts +++ b/cli/js/lib.deno.ns.d.ts @@ -71,6 +71,12 @@ declare namespace Deno { /** Exit the Deno process with optional exit code. */ export function exit(code?: number): never; + /** Get process priority */ + export function getPriority(pid?: number): number; + + /** Set process priority */ + export function setPriority(priority: number, pid?: number): void; + /** Returns a snapshot of the environment variables at invocation. Mutating a * property in the object will set that variable in the environment for the * process. The environment object will only accept `string`s as values. diff --git a/cli/js/os.ts b/cli/js/os.ts index 89632e34fc7611..a83105a195a81a 100644 --- a/cli/js/os.ts +++ b/cli/js/os.ts @@ -36,6 +36,16 @@ export function exit(code = 0): never { return util.unreachable(); } +/** Get process priority */ +export function getPriority(pid = 0): number { + return sendSync("op_get_priority", { pid }); +} + +/** Set process priority */ +export function setPriority(priority: number, pid = 0): void { + sendSync("op_set_priority", { pid, priority }); +} + function setEnv(key: string, value: string): void { sendSync("op_set_env", { key, value }); } diff --git a/cli/js/os_test.ts b/cli/js/os_test.ts index b0561840bb69fb..501d8be84974cf 100644 --- a/cli/js/os_test.ts +++ b/cli/js/os_test.ts @@ -116,6 +116,11 @@ test(function osPid(): void { assert(Deno.pid > 0); }); +test(function testGetPriority(): void { + const priority = Deno.getPriority(); + console.log("priority", priority); +}); + testPerm({ env: true }, function getDir(): void { type supportOS = "mac" | "win" | "linux"; diff --git a/cli/lib.rs b/cli/lib.rs index a4d4ec331deac5..b292eefdad9188 100644 --- a/cli/lib.rs +++ b/cli/lib.rs @@ -43,6 +43,7 @@ pub mod msg; pub mod op_error; pub mod ops; pub mod permissions; +pub mod priority; mod repl; pub mod resolve_addr; pub mod signal; diff --git a/cli/ops/os.rs b/cli/ops/os.rs index 428856707821e1..473163afdc0232 100644 --- a/cli/ops/os.rs +++ b/cli/ops/os.rs @@ -1,6 +1,7 @@ // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license. use super::dispatch_json::{Deserialize, JsonOp, Value}; use crate::op_error::OpError; +use crate::priority::{get_priority, set_priority}; use crate::state::State; use deno_core::*; use std::collections::HashMap; @@ -19,6 +20,8 @@ pub fn init(i: &mut Isolate, s: &State) { i.register_op("op_hostname", s.stateful_json_op(op_hostname)); i.register_op("op_loadavg", s.stateful_json_op(op_loadavg)); i.register_op("op_os_release", s.stateful_json_op(op_os_release)); + i.register_op("op_get_priority", s.stateful_json_op(op_get_priority)); + i.register_op("op_set_priority", s.stateful_json_op(op_set_priority)); } #[derive(Deserialize)] @@ -185,3 +188,34 @@ fn op_os_release( let release = sys_info::os_release().unwrap_or_else(|_| "".to_string()); Ok(JsonOp::Sync(json!(release))) } + +#[derive(Deserialize)] +struct GetPriorityArgs { + pid: u32, +} + +fn op_get_priority( + _state: &State, + args: Value, + _zero_copy: Option, +) -> Result { + let args: GetPriorityArgs = serde_json::from_value(args)?; + let priority = get_priority(args.pid)?; + Ok(JsonOp::Sync(json!(priority))) +} + +#[derive(Deserialize)] +struct SetPriorityArgs { + pid: u32, + priority: i32, +} + +fn op_set_priority( + _state: &State, + args: Value, + _zero_copy: Option, +) -> Result { + let args: SetPriorityArgs = serde_json::from_value(args)?; + set_priority(args.pid, args.priority)?; + Ok(JsonOp::Sync(json!({}))) +} diff --git a/cli/priority.rs b/cli/priority.rs new file mode 100644 index 00000000000000..15b0fdcf589d00 --- /dev/null +++ b/cli/priority.rs @@ -0,0 +1,159 @@ +// Copyright 2018-2020 the Deno authors. All rights reserved. MIT license. +use crate::op_error::OpError; + +#[cfg(unix)] +use errno::{errno, set_errno, Errno}; +#[cfg(unix)] +use libc::{id_t, PRIO_PROCESS}; +#[cfg(windows)] +use winapi::shared::minwindef::{DWORD, FALSE}; +#[cfg(windows)] +use winapi::shared::ntdef::NULL; +#[cfg(windows)] +use winapi::um::handleapi::CloseHandle; +#[cfg(windows)] +use winapi::um::processthreadsapi::{ + GetCurrentProcess, GetPriorityClass, OpenProcess, SetPriorityClass, +}; +#[cfg(windows)] +use winapi::um::winbase::{ + ABOVE_NORMAL_PRIORITY_CLASS, BELOW_NORMAL_PRIORITY_CLASS, + HIGH_PRIORITY_CLASS, IDLE_PRIORITY_CLASS, NORMAL_PRIORITY_CLASS, + REALTIME_PRIORITY_CLASS, +}; +#[cfg(windows)] +use winapi::um::winnt::PROCESS_QUERY_LIMITED_INFORMATION; +#[cfg(target_os = "macos")] +#[allow(non_camel_case_types)] +type priority_t = i32; +#[cfg(target_os = "linux")] +#[allow(non_camel_case_types)] +type priority_t = u32; + +pub const PRIORITY_LOW: i32 = 19; +pub const PRIORITY_BELOW_NORMAL: i32 = 10; +pub const PRIORITY_NORMAL: i32 = 0; +pub const PRIORITY_ABOVE_NORMAL: i32 = -7; +pub const PRIORITY_HIGH: i32 = -14; +pub const PRIORITY_HIGHEST: i32 = -20; + +#[cfg(unix)] +pub fn get_priority(pid: u32) -> Result { + unsafe { + set_errno(Errno(0)); + match ( + libc::getpriority(PRIO_PROCESS as priority_t, pid as id_t), + errno(), + ) { + (-1, Errno(0)) => Ok(PRIORITY_HIGH), + (-1, _) => Err(OpError::from(std::io::Error::last_os_error())), + (priority, _) => Ok(priority), + } + } +} + +#[cfg(unix)] +pub fn set_priority(pid: u32, priority: i32) -> Result<(), OpError> { + unsafe { + match libc::setpriority(PRIO_PROCESS as priority_t, pid as id_t, priority) { + -1 => Err(OpError::from(std::io::Error::last_os_error())), + _ => Ok(()), + } + } +} + +#[cfg(windows)] +pub fn get_priority(pid: u32) -> Result { + unsafe { + let handle = if pid == 0 { + GetCurrentProcess() + } else { + OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION, FALSE, pid as DWORD) + }; + if handle == NULL { + Err(OpError::from(std::io::Error::last_os_error())) + } else { + let result = match GetPriorityClass(handle) { + 0 => Err(OpError::from(std::io::Error::last_os_error())), + REALTIME_PRIORITY_CLASS => Ok(PRIORITY_HIGHEST), + HIGH_PRIORITY_CLASS => Ok(PRIORITY_HIGH), + ABOVE_NORMAL_PRIORITY_CLASS => Ok(PRIORITY_ABOVE_NORMAL), + NORMAL_PRIORITY_CLASS => Ok(PRIORITY_NORMAL), + BELOW_NORMAL_PRIORITY_CLASS => Ok(PRIORITY_BELOW_NORMAL), + IDLE_PRIORITY_CLASS => Ok(PRIORITY_LOW), + _ => Ok(PRIORITY_LOW), + }; + CloseHandle(handle); + result + } + } +} + +#[cfg(windows)] +pub fn set_priority(pid: u32, priority: i32) -> Result<(), OpError> { + unsafe { + let handle = if pid == 0 { + GetCurrentProcess() + } else { + OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION, FALSE, pid as DWORD) + }; + if handle == NULL { + Err(OpError::from(std::io::Error::last_os_error())) + } else { + let prio_class = match priority { + p if p <= PRIORITY_HIGHEST => REALTIME_PRIORITY_CLASS, + p if PRIORITY_HIGHEST < p && p <= PRIORITY_HIGH => HIGH_PRIORITY_CLASS, + p if PRIORITY_HIGH < p && p <= PRIORITY_ABOVE_NORMAL => { + ABOVE_NORMAL_PRIORITY_CLASS + } + p if PRIORITY_ABOVE_NORMAL < p && p <= PRIORITY_NORMAL => { + NORMAL_PRIORITY_CLASS + } + p if PRIORITY_NORMAL < p && p <= PRIORITY_BELOW_NORMAL => { + BELOW_NORMAL_PRIORITY_CLASS + } + _ => IDLE_PRIORITY_CLASS, + }; + let result = match SetPriorityClass(handle, prio_class) { + FALSE => Err(OpError::from(std::io::Error::last_os_error())), + _ => Ok(()), + }; + CloseHandle(handle); + result + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_get_current_process_priority() { + get_priority(0).expect("Should get priority"); + } + + #[cfg(unix)] + #[test] + fn test_set_current_process_high_priority_should_fail() { + assert!(set_priority(0, PRIORITY_HIGH).is_err()); + } + + /// this test makes multiple tests at once + /// because we need to set them in order and rust + /// does not garanty test order execution + #[test] + fn test_set_current_process_priority_from_normal_to_low() { + set_priority(0, PRIORITY_NORMAL).expect("Should set priority"); + let priority = get_priority(0).expect("Should get priority"); + assert_eq!(priority, PRIORITY_NORMAL); + + set_priority(0, PRIORITY_BELOW_NORMAL).expect("Should set priority"); + let priority = get_priority(0).expect("Should get priority"); + assert_eq!(priority, PRIORITY_BELOW_NORMAL); + + set_priority(0, PRIORITY_LOW).expect("Should set priority"); + let priority = get_priority(0).expect("Should get priority"); + assert_eq!(priority, PRIORITY_LOW); + } +} diff --git a/std/node/os.ts b/std/node/os.ts index 4bc70d1ffa4a01..dacfe822a65fe2 100644 --- a/std/node/os.ts +++ b/std/node/os.ts @@ -100,6 +100,13 @@ totalmem[Symbol.toPrimitive] = (): number => totalmem(); type[Symbol.toPrimitive] = (): string => type(); uptime[Symbol.toPrimitive] = (): number => uptime(); +const PRIORITY_LOW = 19; +const PRIORITY_BELOW_NORMAL = 10; +const PRIORITY_NORMAL = 0; +const PRIORITY_ABOVE_NORMAL = -7; +const PRIORITY_HIGH = -14; +const PRIORITY_HIGHEST = -20; + /** Returns the operating system CPU architecture for which the Deno binary was compiled */ export function arch(): string { return Deno.build.arch; @@ -128,10 +135,10 @@ export function freemem(): number { notImplemented(SEE_GITHUB_ISSUE); } -/** Not yet implemented */ +/** Get process priority */ export function getPriority(pid = 0): number { validateIntegerRange(pid, "pid"); - notImplemented(SEE_GITHUB_ISSUE); + return Deno.getPriority(pid); } /** Returns the string path of the current user's home directory. */ @@ -166,7 +173,7 @@ export function release(): string { return Deno.osRelease(); } -/** Not yet implemented */ +/** Set process priority */ export function setPriority(pid: number, priority?: number): void { /* The node API has the 'pid' as the first parameter and as optional. This makes for a problematic implementation in Typescript. */ @@ -175,9 +182,8 @@ export function setPriority(pid: number, priority?: number): void { pid = 0; } validateIntegerRange(pid, "pid"); - validateIntegerRange(priority, "priority", -20, 19); - - notImplemented(SEE_GITHUB_ISSUE); + validateIntegerRange(priority, "priority", PRIORITY_HIGHEST, PRIORITY_LOW); + Deno.setPriority(priority, pid); } /** Returns the operating system's default directory for temporary files as a string. */ @@ -219,6 +225,12 @@ export const constants = { signals: Deno.Signal, priority: { // see https://nodejs.org/docs/latest-v12.x/api/os.html#os_priority_constants + PRIORITY_LOW, + PRIORITY_BELOW_NORMAL, + PRIORITY_NORMAL, + PRIORITY_ABOVE_NORMAL, + PRIORITY_HIGH, + PRIORITY_HIGHEST } }; diff --git a/std/node/os_test.ts b/std/node/os_test.ts index a73a2d4e99d2e4..1681f5324e6ab9 100644 --- a/std/node/os_test.ts +++ b/std/node/os_test.ts @@ -44,6 +44,58 @@ test({ } }); +test({ + name: "getPriority(): should get current process priority if no params", + fn() { + os.getPriority(); + } +}); + +if (os.platform() === "win32") + test({ + name: "setPriority(): should set current process priority to high", + fn() { + os.setPriority(os.constants.priority.PRIORITY_HIGH); + assertEquals(os.getPriority(), os.constants.priority.PRIORITY_HIGH); + } + }); + +if (os.platform() === "win32") + test({ + name: "setPriority(): should set current process priority to above normal", + fn() { + os.setPriority(os.constants.priority.PRIORITY_ABOVE_NORMAL); + assertEquals( + os.getPriority(), + os.constants.priority.PRIORITY_ABOVE_NORMAL + ); + } + }); + +test({ + name: "setPriority(): should set current process priority to normal", + fn() { + os.setPriority(os.constants.priority.PRIORITY_NORMAL); + assertEquals(os.getPriority(), os.constants.priority.PRIORITY_NORMAL); + } +}); + +test({ + name: "setPriority(): should set current process priority to below normal", + fn() { + os.setPriority(os.constants.priority.PRIORITY_BELOW_NORMAL); + assertEquals(os.getPriority(), os.constants.priority.PRIORITY_BELOW_NORMAL); + } +}); + +test({ + name: "setPriority(): should set current process priority to low", + fn() { + os.setPriority(os.constants.priority.PRIORITY_LOW); + assertEquals(os.getPriority(), os.constants.priority.PRIORITY_LOW); + } +}); + test({ name: "getPriority(): PID must be a 32 bit integer", fn() { @@ -216,13 +268,6 @@ test({ Error, "Not implemented" ); - assertThrows( - () => { - os.getPriority(); - }, - Error, - "Not implemented" - ); assertThrows( () => { os.networkInterfaces(); @@ -230,13 +275,6 @@ test({ Error, "Not implemented" ); - assertThrows( - () => { - os.setPriority(0); - }, - Error, - "Not implemented" - ); assertThrows( () => { os.totalmem(); From 3a04261808d147ccaa4967523042dff999aae98d Mon Sep 17 00:00:00 2001 From: ecyrbe Date: Mon, 2 Mar 2020 00:25:54 +0100 Subject: [PATCH 2/4] fix: review --- cli/js/lib.deno.ns.d.ts | 32 ++++++++++++++++++++++--- cli/js/os.ts | 29 ++++++++++++++++++++--- cli/js/os_test.ts | 52 ++++++++++++++++++++++++++++++++++++++--- cli/priority.rs | 2 +- std/node/os.ts | 43 +++++++++++++++++++++------------- std/node/os_test.ts | 4 +++- 6 files changed, 135 insertions(+), 27 deletions(-) diff --git a/cli/js/lib.deno.ns.d.ts b/cli/js/lib.deno.ns.d.ts index 90ae52c1da0fb4..383e74f24d76b1 100644 --- a/cli/js/lib.deno.ns.d.ts +++ b/cli/js/lib.deno.ns.d.ts @@ -71,11 +71,37 @@ declare namespace Deno { /** Exit the Deno process with optional exit code. */ export function exit(code?: number): never; - /** Get process priority */ + /** **UNSTABLE**: Might not use all-caps. */ + export enum OsPriority { + LOW = 19, + BELOW_NORMAL = 10, + NORMAL = 0, + ABOVE_NORMAL = -7, + HIGH = -14, + HIGHEST = -20 + } + + /** + * Returns the scheduling priority for the process specified by pid. + * If pid is not provided or is 0, the priority of the current process is returned. + */ export function getPriority(pid?: number): number; - /** Set process priority */ - export function setPriority(priority: number, pid?: number): void; + /** + * Attempts to set the scheduling priority for the process specified by pid. + * If pid is not provided or is 0, the process ID of the current process is used. + * The priority input must be an integer between -20 (high priority) and 19 (low priority). + * Due to differences between Unix priority levels and Windows priority classes, + * priority is mapped to one of six priority constants in os.constants.priority. + * When retrieving a process priority level, this range mapping may cause the return value to be slightly different on Windows. + * To avoid confusion, set priority to one of the priority constants. + * On Windows, setting priority to PRIORITY_HIGHEST requires elevated user privileges. + * Otherwise the set priority will be silently reduced to PRIORITY_HIGH. + */ + export function setPriority( + priority: number | OsPriority, + pid?: number + ): void; /** Returns a snapshot of the environment variables at invocation. Mutating a * property in the object will set that variable in the environment for the diff --git a/cli/js/os.ts b/cli/js/os.ts index a83105a195a81a..8e2c4e4a910b09 100644 --- a/cli/js/os.ts +++ b/cli/js/os.ts @@ -36,13 +36,36 @@ export function exit(code = 0): never { return util.unreachable(); } -/** Get process priority */ +/** **UNSTABLE**: Might not use all-caps. */ +export enum OsPriority { + LOW = 19, + BELOW_NORMAL = 10, + NORMAL = 0, + ABOVE_NORMAL = -7, + HIGH = -14, + HIGHEST = -20 +} + +/** + * Returns the scheduling priority for the process specified by pid. + * If pid is not provided or is 0, the priority of the current process is returned. + */ export function getPriority(pid = 0): number { return sendSync("op_get_priority", { pid }); } -/** Set process priority */ -export function setPriority(priority: number, pid = 0): void { +/** + * Attempts to set the scheduling priority for the process specified by pid. + * If pid is not provided or is 0, the process ID of the current process is used. + * The priority input must be an integer between -20 (high priority) and 19 (low priority). + * Due to differences between Unix priority levels and Windows priority classes, + * priority is mapped to one of six priority constants in os.constants.priority. + * When retrieving a process priority level, this range mapping may cause the return value to be slightly different on Windows. + * To avoid confusion, set priority to one of the priority constants. + * On Windows, setting priority to PRIORITY_HIGHEST requires elevated user privileges. + * Otherwise the set priority will be silently reduced to PRIORITY_HIGH. + */ +export function setPriority(priority: number | OsPriority, pid = 0): void { sendSync("op_set_priority", { pid, priority }); } diff --git a/cli/js/os_test.ts b/cli/js/os_test.ts index 501d8be84974cf..313cfb51514e29 100644 --- a/cli/js/os_test.ts +++ b/cli/js/os_test.ts @@ -116,9 +116,55 @@ test(function osPid(): void { assert(Deno.pid > 0); }); -test(function testGetPriority(): void { - const priority = Deno.getPriority(); - console.log("priority", priority); +test({ + name: "getPriority(): should get current process priority if no params", + fn() { + const priority = Deno.getPriority(); + assert(Deno.OsPriority.HIGHEST <= priority); + assert(priority <= Deno.OsPriority.LOW); + } +}); + +if (Deno.build.os === "win") + test({ + name: "setPriority(): should set current process priority to high", + fn() { + Deno.setPriority(Deno.OsPriority.HIGH); + assertEquals(Deno.getPriority(), Deno.OsPriority.HIGH); + } + }); + +if (Deno.build.os === "win") + test({ + name: "setPriority(): should set current process priority to above normal", + fn() { + Deno.setPriority(Deno.OsPriority.ABOVE_NORMAL); + assertEquals(Deno.getPriority(), Deno.OsPriority.ABOVE_NORMAL); + } + }); + +test({ + name: "setPriority(): should set current process priority to normal", + fn() { + Deno.setPriority(Deno.OsPriority.NORMAL); + assertEquals(Deno.getPriority(), Deno.OsPriority.NORMAL); + } +}); + +test({ + name: "setPriority(): should set current process priority to below normal", + fn() { + Deno.setPriority(Deno.OsPriority.BELOW_NORMAL); + assertEquals(Deno.getPriority(), Deno.OsPriority.BELOW_NORMAL); + } +}); + +test({ + name: "setPriority(): should set current process priority to low", + fn() { + Deno.setPriority(Deno.OsPriority.LOW); + assertEquals(Deno.getPriority(), Deno.OsPriority.LOW); + } }); testPerm({ env: true }, function getDir(): void { diff --git a/cli/priority.rs b/cli/priority.rs index 15b0fdcf589d00..0d89b10147f10e 100644 --- a/cli/priority.rs +++ b/cli/priority.rs @@ -141,7 +141,7 @@ mod tests { /// this test makes multiple tests at once /// because we need to set them in order and rust - /// does not garanty test order execution + /// does not garantee test order execution #[test] fn test_set_current_process_priority_from_normal_to_low() { set_priority(0, PRIORITY_NORMAL).expect("Should set priority"); diff --git a/std/node/os.ts b/std/node/os.ts index dacfe822a65fe2..e17d963165f5a3 100644 --- a/std/node/os.ts +++ b/std/node/os.ts @@ -100,13 +100,6 @@ totalmem[Symbol.toPrimitive] = (): number => totalmem(); type[Symbol.toPrimitive] = (): string => type(); uptime[Symbol.toPrimitive] = (): number => uptime(); -const PRIORITY_LOW = 19; -const PRIORITY_BELOW_NORMAL = 10; -const PRIORITY_NORMAL = 0; -const PRIORITY_ABOVE_NORMAL = -7; -const PRIORITY_HIGH = -14; -const PRIORITY_HIGHEST = -20; - /** Returns the operating system CPU architecture for which the Deno binary was compiled */ export function arch(): string { return Deno.build.arch; @@ -135,7 +128,10 @@ export function freemem(): number { notImplemented(SEE_GITHUB_ISSUE); } -/** Get process priority */ +/** + * Returns the scheduling priority for the process specified by pid. + * If pid is not provided or is 0, the priority of the current process is returned. + */ export function getPriority(pid = 0): number { validateIntegerRange(pid, "pid"); return Deno.getPriority(pid); @@ -173,7 +169,17 @@ export function release(): string { return Deno.osRelease(); } -/** Set process priority */ +/** + * Attempts to set the scheduling priority for the process specified by pid. + * If pid is not provided or is 0, the process ID of the current process is used. + * The priority input must be an integer between -20 (high priority) and 19 (low priority). + * Due to differences between Unix priority levels and Windows priority classes, + * priority is mapped to one of six priority constants in os.constants.priority. + * When retrieving a process priority level, this range mapping may cause the return value to be slightly different on Windows. + * To avoid confusion, set priority to one of the priority constants. + * On Windows, setting priority to PRIORITY_HIGHEST requires elevated user privileges. + * Otherwise the set priority will be silently reduced to PRIORITY_HIGH. + */ export function setPriority(pid: number, priority?: number): void { /* The node API has the 'pid' as the first parameter and as optional. This makes for a problematic implementation in Typescript. */ @@ -182,7 +188,12 @@ export function setPriority(pid: number, priority?: number): void { pid = 0; } validateIntegerRange(pid, "pid"); - validateIntegerRange(priority, "priority", PRIORITY_HIGHEST, PRIORITY_LOW); + validateIntegerRange( + priority, + "priority", + Deno.OsPriority.HIGHEST, + Deno.OsPriority.LOW + ); Deno.setPriority(priority, pid); } @@ -225,12 +236,12 @@ export const constants = { signals: Deno.Signal, priority: { // see https://nodejs.org/docs/latest-v12.x/api/os.html#os_priority_constants - PRIORITY_LOW, - PRIORITY_BELOW_NORMAL, - PRIORITY_NORMAL, - PRIORITY_ABOVE_NORMAL, - PRIORITY_HIGH, - PRIORITY_HIGHEST + PRIORITY_LOW: Deno.OsPriority.LOW, + PRIORITY_BELOW_NORMAL: Deno.OsPriority.BELOW_NORMAL, + PRIORITY_NORMAL: Deno.OsPriority.NORMAL, + PRIORITY_ABOVE_NORMAL: Deno.OsPriority.ABOVE_NORMAL, + PRIORITY_HIGH: Deno.OsPriority.HIGH, + PRIORITY_HIGHEST: Deno.OsPriority.HIGHEST } }; diff --git a/std/node/os_test.ts b/std/node/os_test.ts index 1681f5324e6ab9..ea663ce96a82c8 100644 --- a/std/node/os_test.ts +++ b/std/node/os_test.ts @@ -47,7 +47,9 @@ test({ test({ name: "getPriority(): should get current process priority if no params", fn() { - os.getPriority(); + const priority = os.getPriority(); + assert(os.constants.priority.PRIORITY_HIGHEST <= priority); + assert(priority <= os.constants.priority.PRIORITY_LOW); } }); From cff74d8daeb4d91bb15e6172f4445985cbab7a6e Mon Sep 17 00:00:00 2001 From: ecyrbe Date: Mon, 2 Mar 2020 00:38:52 +0100 Subject: [PATCH 3/4] fix: review 2 --- cli/js/deno.ts | 1 + cli/js/lib.deno.ns.d.ts | 6 ++--- cli/js/os.ts | 6 ++--- cli/js/os_test.ts | 56 ++++++++++++++--------------------------- cli/priority.rs | 2 +- 5 files changed, 27 insertions(+), 44 deletions(-) diff --git a/cli/js/deno.ts b/cli/js/deno.ts index bcb0aeb7edd426..ddad910958d0fd 100644 --- a/cli/js/deno.ts +++ b/cli/js/deno.ts @@ -93,6 +93,7 @@ export { execPath, hostname, loadavg, + OsPriority, getPriority, setPriority, osRelease diff --git a/cli/js/lib.deno.ns.d.ts b/cli/js/lib.deno.ns.d.ts index 383e74f24d76b1..e67ea71aded91e 100644 --- a/cli/js/lib.deno.ns.d.ts +++ b/cli/js/lib.deno.ns.d.ts @@ -92,11 +92,11 @@ declare namespace Deno { * If pid is not provided or is 0, the process ID of the current process is used. * The priority input must be an integer between -20 (high priority) and 19 (low priority). * Due to differences between Unix priority levels and Windows priority classes, - * priority is mapped to one of six priority constants in os.constants.priority. + * priority is mapped to one of six priority constants in Deno.OsPriority enum. * When retrieving a process priority level, this range mapping may cause the return value to be slightly different on Windows. * To avoid confusion, set priority to one of the priority constants. - * On Windows, setting priority to PRIORITY_HIGHEST requires elevated user privileges. - * Otherwise the set priority will be silently reduced to PRIORITY_HIGH. + * On Windows, setting priority to Deno.OsPriority.HIGHEST requires elevated user privileges. + * Otherwise the set priority will be silently reduced to Deno.OsPriority.HIGH. */ export function setPriority( priority: number | OsPriority, diff --git a/cli/js/os.ts b/cli/js/os.ts index 8e2c4e4a910b09..d81e11660b4698 100644 --- a/cli/js/os.ts +++ b/cli/js/os.ts @@ -59,11 +59,11 @@ export function getPriority(pid = 0): number { * If pid is not provided or is 0, the process ID of the current process is used. * The priority input must be an integer between -20 (high priority) and 19 (low priority). * Due to differences between Unix priority levels and Windows priority classes, - * priority is mapped to one of six priority constants in os.constants.priority. + * priority is mapped to one of six priority constants in Deno.OsPriority enum. * When retrieving a process priority level, this range mapping may cause the return value to be slightly different on Windows. * To avoid confusion, set priority to one of the priority constants. - * On Windows, setting priority to PRIORITY_HIGHEST requires elevated user privileges. - * Otherwise the set priority will be silently reduced to PRIORITY_HIGH. + * On Windows, setting priority to Deno.OsPriority.HIGHEST requires elevated user privileges. + * Otherwise the set priority will be silently reduced to Deno.OsPriority.HIGH. */ export function setPriority(priority: number | OsPriority, pid = 0): void { sendSync("op_set_priority", { pid, priority }); diff --git a/cli/js/os_test.ts b/cli/js/os_test.ts index 313cfb51514e29..2ad0a7d74dce0a 100644 --- a/cli/js/os_test.ts +++ b/cli/js/os_test.ts @@ -116,55 +116,37 @@ test(function osPid(): void { assert(Deno.pid > 0); }); -test({ - name: "getPriority(): should get current process priority if no params", - fn() { - const priority = Deno.getPriority(); - assert(Deno.OsPriority.HIGHEST <= priority); - assert(priority <= Deno.OsPriority.LOW); - } +test(function testGetPriorityShouldBeBetweenBounds() { + const priority = Deno.getPriority(); + assert(Deno.OsPriority.HIGHEST <= priority); + assert(priority <= Deno.OsPriority.LOW); }); if (Deno.build.os === "win") - test({ - name: "setPriority(): should set current process priority to high", - fn() { - Deno.setPriority(Deno.OsPriority.HIGH); - assertEquals(Deno.getPriority(), Deno.OsPriority.HIGH); - } + test(function testSetPriorityShouldBeSetToHigh() { + Deno.setPriority(Deno.OsPriority.HIGH); + assertEquals(Deno.getPriority(), Deno.OsPriority.HIGH); }); if (Deno.build.os === "win") - test({ - name: "setPriority(): should set current process priority to above normal", - fn() { - Deno.setPriority(Deno.OsPriority.ABOVE_NORMAL); - assertEquals(Deno.getPriority(), Deno.OsPriority.ABOVE_NORMAL); - } + test(function testSetPriorityShouldBeSetToAboveNormal() { + Deno.setPriority(Deno.OsPriority.ABOVE_NORMAL); + assertEquals(Deno.getPriority(), Deno.OsPriority.ABOVE_NORMAL); }); -test({ - name: "setPriority(): should set current process priority to normal", - fn() { - Deno.setPriority(Deno.OsPriority.NORMAL); - assertEquals(Deno.getPriority(), Deno.OsPriority.NORMAL); - } +test(function testSetPriorityShouldBeSetToNormal() { + Deno.setPriority(Deno.OsPriority.NORMAL); + assertEquals(Deno.getPriority(), Deno.OsPriority.NORMAL); }); -test({ - name: "setPriority(): should set current process priority to below normal", - fn() { - Deno.setPriority(Deno.OsPriority.BELOW_NORMAL); - assertEquals(Deno.getPriority(), Deno.OsPriority.BELOW_NORMAL); - } +test(function testSetPriorityShouldBeSetToBelowNormal() { + Deno.setPriority(Deno.OsPriority.BELOW_NORMAL); + assertEquals(Deno.getPriority(), Deno.OsPriority.BELOW_NORMAL); }); -test({ - name: "setPriority(): should set current process priority to low", - fn() { - Deno.setPriority(Deno.OsPriority.LOW); - assertEquals(Deno.getPriority(), Deno.OsPriority.LOW); - } +test(function testSetPriorityShouldBeSetToLow() { + Deno.setPriority(Deno.OsPriority.LOW); + assertEquals(Deno.getPriority(), Deno.OsPriority.LOW); }); testPerm({ env: true }, function getDir(): void { diff --git a/cli/priority.rs b/cli/priority.rs index 0d89b10147f10e..0ab64ff1efb07e 100644 --- a/cli/priority.rs +++ b/cli/priority.rs @@ -141,7 +141,7 @@ mod tests { /// this test makes multiple tests at once /// because we need to set them in order and rust - /// does not garantee test order execution + /// does not guarantee test order execution #[test] fn test_set_current_process_priority_from_normal_to_low() { set_priority(0, PRIORITY_NORMAL).expect("Should set priority"); From 2653c6f66abc2c0422479f30db9068481a951fa3 Mon Sep 17 00:00:00 2001 From: ecyrbe Date: Tue, 3 Mar 2020 19:35:11 +0100 Subject: [PATCH 4/4] docs: review 3 --- cli/js/lib.deno.ns.d.ts | 4 ++++ cli/js/os.ts | 4 ++++ cli/priority.rs | 8 ++++++++ std/node/os.ts | 4 ++++ 4 files changed, 20 insertions(+) diff --git a/cli/js/lib.deno.ns.d.ts b/cli/js/lib.deno.ns.d.ts index e67ea71aded91e..aa0fb0aed503c2 100644 --- a/cli/js/lib.deno.ns.d.ts +++ b/cli/js/lib.deno.ns.d.ts @@ -82,12 +82,16 @@ declare namespace Deno { } /** + * **UNSTABLE:** new api + * * Returns the scheduling priority for the process specified by pid. * If pid is not provided or is 0, the priority of the current process is returned. */ export function getPriority(pid?: number): number; /** + * **UNSTABLE:** new api + * * Attempts to set the scheduling priority for the process specified by pid. * If pid is not provided or is 0, the process ID of the current process is used. * The priority input must be an integer between -20 (high priority) and 19 (low priority). diff --git a/cli/js/os.ts b/cli/js/os.ts index d81e11660b4698..538570412f227e 100644 --- a/cli/js/os.ts +++ b/cli/js/os.ts @@ -47,6 +47,8 @@ export enum OsPriority { } /** + * **UNSTABLE:** new api + * * Returns the scheduling priority for the process specified by pid. * If pid is not provided or is 0, the priority of the current process is returned. */ @@ -55,6 +57,8 @@ export function getPriority(pid = 0): number { } /** + * **UNSTABLE:** new api + * * Attempts to set the scheduling priority for the process specified by pid. * If pid is not provided or is 0, the process ID of the current process is used. * The priority input must be an integer between -20 (high priority) and 19 (low priority). diff --git a/cli/priority.rs b/cli/priority.rs index 0ab64ff1efb07e..ac8f409974dd05 100644 --- a/cli/priority.rs +++ b/cli/priority.rs @@ -1,4 +1,8 @@ // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license. + +//! This module provides cross-platform ability to get and set program scheduling priority. +//! It uses libc getpriority/setpriority on posix +//! It uses winapi GetPriorityClass/SetPriorityClass on windows use crate::op_error::OpError; #[cfg(unix)] @@ -37,6 +41,7 @@ pub const PRIORITY_ABOVE_NORMAL: i32 = -7; pub const PRIORITY_HIGH: i32 = -14; pub const PRIORITY_HIGHEST: i32 = -20; +/* **UNSTABLE:** new api */ #[cfg(unix)] pub fn get_priority(pid: u32) -> Result { unsafe { @@ -52,6 +57,7 @@ pub fn get_priority(pid: u32) -> Result { } } +/* **UNSTABLE:** new api */ #[cfg(unix)] pub fn set_priority(pid: u32, priority: i32) -> Result<(), OpError> { unsafe { @@ -62,6 +68,7 @@ pub fn set_priority(pid: u32, priority: i32) -> Result<(), OpError> { } } +/* **UNSTABLE:** new api */ #[cfg(windows)] pub fn get_priority(pid: u32) -> Result { unsafe { @@ -89,6 +96,7 @@ pub fn get_priority(pid: u32) -> Result { } } +/* **UNSTABLE:** new api */ #[cfg(windows)] pub fn set_priority(pid: u32, priority: i32) -> Result<(), OpError> { unsafe { diff --git a/std/node/os.ts b/std/node/os.ts index e17d963165f5a3..e625d83d1a7f03 100644 --- a/std/node/os.ts +++ b/std/node/os.ts @@ -129,6 +129,8 @@ export function freemem(): number { } /** + * **UNSTABLE:** new api + * * Returns the scheduling priority for the process specified by pid. * If pid is not provided or is 0, the priority of the current process is returned. */ @@ -170,6 +172,8 @@ export function release(): string { } /** + * **UNSTABLE:** new api + * * Attempts to set the scheduling priority for the process specified by pid. * If pid is not provided or is 0, the process ID of the current process is used. * The priority input must be an integer between -20 (high priority) and 19 (low priority).