From 734cf781c6e606a8a836863a391c94cf4fad22d7 Mon Sep 17 00:00:00 2001 From: Vincent LE GOFF Date: Mon, 8 Apr 2019 22:22:40 +0200 Subject: [PATCH] Allow high precision performance.now() (#1977) --- cli/flags.rs | 24 ++++++++++++++++++++++++ cli/isolate_state.rs | 3 +++ cli/msg.fbs | 4 +++- cli/ops.rs | 32 +++++++++++++++++++++++++------- cli/permissions.rs | 12 ++++++++++++ js/performance.ts | 12 ++++-------- js/performance_test.ts | 4 ++-- js/permissions.ts | 5 +++-- js/permissions_test.ts | 3 ++- js/test_util.ts | 31 +++++++++++++++++++++++++------ tests/025_high_precision.test | 2 ++ tests/025_high_precision.ts | 3 +++ tests/025_high_precision.ts.out | 2 ++ tools/unit_tests.py | 16 +++++++++------- 14 files changed, 119 insertions(+), 34 deletions(-) create mode 100644 tests/025_high_precision.test create mode 100644 tests/025_high_precision.ts create mode 100644 tests/025_high_precision.ts.out diff --git a/cli/flags.rs b/cli/flags.rs index e52e55b5e6..9f43b0b807 100644 --- a/cli/flags.rs +++ b/cli/flags.rs @@ -19,6 +19,7 @@ pub struct DenoFlags { pub allow_net: bool, pub allow_env: bool, pub allow_run: bool, + pub allow_high_precision: bool, pub no_prompts: bool, pub types: bool, pub prefetch: bool, @@ -54,6 +55,9 @@ impl<'a> From> for DenoFlags { if matches.is_present("allow-run") { flags.allow_run = true; } + if matches.is_present("allow-high-precision") { + flags.allow_high_precision = true; + } if matches.is_present("allow-all") { flags.allow_read = true; flags.allow_env = true; @@ -61,6 +65,7 @@ impl<'a> From> for DenoFlags { flags.allow_run = true; flags.allow_read = true; flags.allow_write = true; + flags.allow_high_precision = true; } if matches.is_present("no-prompt") { flags.no_prompts = true; @@ -124,6 +129,10 @@ pub fn set_flags( Arg::with_name("allow-run") .long("allow-run") .help("Allow running subprocesses"), + ).arg( + Arg::with_name("allow-high-precision") + .long("allow-high-precision") + .help("Allow high precision time measurement"), ).arg( Arg::with_name("allow-all") .short("A") @@ -338,6 +347,7 @@ fn test_set_flags_7() { allow_run: true, allow_read: true, allow_write: true, + allow_high_precision: true, ..DenoFlags::default() } ) @@ -356,3 +366,17 @@ fn test_set_flags_8() { } ) } + +#[test] +fn test_set_flags_9() { + let (flags, rest) = + set_flags(svec!["deno", "--allow-high-precision", "script.ts"]).unwrap(); + assert_eq!(rest, svec!["deno", "script.ts"]); + assert_eq!( + flags, + DenoFlags { + allow_high_precision: true, + ..DenoFlags::default() + } + ) +} diff --git a/cli/isolate_state.rs b/cli/isolate_state.rs index 500cdd7d7c..313f4f6cef 100644 --- a/cli/isolate_state.rs +++ b/cli/isolate_state.rs @@ -17,6 +17,7 @@ use std::env; use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::Arc; use std::sync::Mutex; +use std::time::Instant; pub type WorkerSender = async_mpsc::Sender; pub type WorkerReceiver = async_mpsc::Receiver; @@ -51,6 +52,7 @@ pub struct IsolateState { pub global_timer: Mutex, pub workers: Mutex, pub is_worker: bool, + pub start_time: Instant, } impl IsolateState { @@ -73,6 +75,7 @@ impl IsolateState { global_timer: Mutex::new(GlobalTimer::new()), workers: Mutex::new(UserWorkerTable::new()), is_worker, + start_time: Instant::now(), } } diff --git a/cli/msg.fbs b/cli/msg.fbs index c515b9add0..4eca8dcf85 100644 --- a/cli/msg.fbs +++ b/cli/msg.fbs @@ -281,6 +281,7 @@ table PermissionsRes { write: bool; net: bool; env: bool; + high_precision: bool; } // Note this represents The WHOLE header of an http message, not just the key @@ -527,7 +528,8 @@ table RunStatusRes { table Now {} table NowRes { - time: uint64; + seconds: uint64; + subsec_nanos: uint32; } table IsTTY {} diff --git a/cli/ops.rs b/cli/ops.rs index a0169d6abc..d5656a3b62 100644 --- a/cli/ops.rs +++ b/cli/ops.rs @@ -41,7 +41,7 @@ use std::path::Path; use std::path::PathBuf; use std::process::Command; use std::sync::Arc; -use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; +use std::time::{Duration, Instant, UNIX_EPOCH}; use tokio; use tokio::net::TcpListener; use tokio::net::TcpStream; @@ -212,19 +212,35 @@ pub fn op_selector_std(inner_type: msg::Any) -> Option { } } +// Returns a milliseconds and nanoseconds subsec +// since the start time of the deno runtime. +// If the High precision flag is not set, the +// nanoseconds are rounded on 2ms. fn op_now( - _sc: &IsolateStateContainer, + sc: &IsolateStateContainer, base: &msg::Base<'_>, data: deno_buf, ) -> Box { assert_eq!(data.len(), 0); - let start = SystemTime::now(); - let since_the_epoch = start.duration_since(UNIX_EPOCH).unwrap(); - let time = since_the_epoch.as_secs() * 1000 - + u64::from(since_the_epoch.subsec_millis()); + let seconds = sc.state().start_time.elapsed().as_secs(); + let mut subsec_nanos = sc.state().start_time.elapsed().subsec_nanos(); + let reduced_time_precision = 2000000; // 2ms in nanoseconds + + // If the permission is not enabled + // Round the nano result on 2 milliseconds + // see: https://developer.mozilla.org/en-US/docs/Web/API/DOMHighResTimeStamp#Reduced_time_precision + if !sc.state().permissions.allows_high_precision() { + subsec_nanos -= subsec_nanos % reduced_time_precision + } let builder = &mut FlatBufferBuilder::new(); - let inner = msg::NowRes::create(builder, &msg::NowResArgs { time }); + let inner = msg::NowRes::create( + builder, + &msg::NowResArgs { + seconds, + subsec_nanos, + }, + ); ok_future(serialize_response( base.cmd_id(), builder, @@ -555,6 +571,7 @@ fn op_permissions( write: sc.state().permissions.allows_write(), net: sc.state().permissions.allows_net(), env: sc.state().permissions.allows_env(), + high_precision: sc.state().permissions.allows_high_precision(), }, ); ok_future(serialize_response( @@ -582,6 +599,7 @@ fn op_revoke_permission( "write" => sc.state().permissions.revoke_write(), "net" => sc.state().permissions.revoke_net(), "env" => sc.state().permissions.revoke_env(), + "highPrecision" => sc.state().permissions.revoke_high_precision(), _ => Ok(()), }; if let Err(e) = result { diff --git a/cli/permissions.rs b/cli/permissions.rs index 2240d94c1a..6247cd0d11 100644 --- a/cli/permissions.rs +++ b/cli/permissions.rs @@ -131,6 +131,7 @@ pub struct DenoPermissions { pub allow_net: PermissionAccessor, pub allow_env: PermissionAccessor, pub allow_run: PermissionAccessor, + pub allow_high_precision: PermissionAccessor, pub no_prompts: AtomicBool, } @@ -142,6 +143,9 @@ impl DenoPermissions { allow_env: PermissionAccessor::from(flags.allow_env), allow_net: PermissionAccessor::from(flags.allow_net), allow_run: PermissionAccessor::from(flags.allow_run), + allow_high_precision: PermissionAccessor::from( + flags.allow_high_precision, + ), no_prompts: AtomicBool::new(flags.no_prompts), } } @@ -263,6 +267,10 @@ impl DenoPermissions { self.allow_env.is_allow() } + pub fn allows_high_precision(&self) -> bool { + return self.allow_high_precision.is_allow(); + } + pub fn revoke_run(&self) -> DenoResult<()> { self.allow_run.revoke(); Ok(()) @@ -287,6 +295,10 @@ impl DenoPermissions { self.allow_env.revoke(); Ok(()) } + pub fn revoke_high_precision(&self) -> DenoResult<()> { + self.allow_high_precision.revoke(); + return Ok(()); + } } /// Quad-state value for representing user input on permission prompt diff --git a/js/performance.ts b/js/performance.ts index 1af75809bb..51e213f0f9 100644 --- a/js/performance.ts +++ b/js/performance.ts @@ -5,13 +5,9 @@ import * as flatbuffers from "./flatbuffers"; import { assert } from "./util"; export class Performance { - timeOrigin = 0; - - constructor() { - this.timeOrigin = new Date().getTime(); - } - - /** Returns a current time from Deno's start + /** Returns a current time from Deno's start. + * In milliseconds. Flag --allow-high-precision give + * a precise measure. * * const t = performance.now(); * console.log(`${t} ms since start!`); @@ -23,6 +19,6 @@ export class Performance { assert(msg.Any.NowRes === baseRes.innerType()); const res = new msg.NowRes(); assert(baseRes.inner(res) != null); - return res.time().toFloat64() - this.timeOrigin; + return res.seconds().toFloat64() * 1e3 + res.subsecNanos() / 1e6; } } diff --git a/js/performance_test.ts b/js/performance_test.ts index 154dadbbe8..8f9dbc9901 100644 --- a/js/performance_test.ts +++ b/js/performance_test.ts @@ -1,7 +1,7 @@ // Copyright 2018-2019 the Deno authors. All rights reserved. MIT license. -import { test, assert } from "./test_util.ts"; +import { testPerm, assert } from "./test_util.ts"; -test(function now() { +testPerm({ highPrecision: false }, function now() { const start = performance.now(); setTimeout(() => { const end = performance.now(); diff --git a/js/permissions.ts b/js/permissions.ts index 46c809cf6e..d0885ba6aa 100644 --- a/js/permissions.ts +++ b/js/permissions.ts @@ -11,7 +11,7 @@ export interface Permissions { net: boolean; env: boolean; run: boolean; - + highPrecision: boolean; // NOTE: Keep in sync with src/permissions.rs } @@ -29,7 +29,8 @@ function createPermissions(inner: msg.PermissionsRes): Permissions { write: inner.write(), net: inner.net(), env: inner.env(), - run: inner.run() + run: inner.run(), + highPrecision: inner.highPrecision() }; } diff --git a/js/permissions_test.ts b/js/permissions_test.ts index e44be79bbe..c49696c014 100644 --- a/js/permissions_test.ts +++ b/js/permissions_test.ts @@ -6,7 +6,8 @@ const knownPermissions: Deno.Permission[] = [ "read", "write", "net", - "env" + "env", + "highPrecision" ]; for (let grant of knownPermissions) { diff --git a/js/test_util.ts b/js/test_util.ts index e683d32b10..15bbb521b3 100644 --- a/js/test_util.ts +++ b/js/test_util.ts @@ -26,6 +26,7 @@ interface DenoPermissions { net?: boolean; env?: boolean; run?: boolean; + highPrecision?: boolean; } function permToString(perms: DenoPermissions): string { @@ -34,11 +35,12 @@ function permToString(perms: DenoPermissions): string { const n = perms.net ? 1 : 0; const e = perms.env ? 1 : 0; const u = perms.run ? 1 : 0; - return `permR${r}W${w}N${n}E${e}U${u}`; + const h = perms.highPrecision ? 1 : 0; + return `permR${r}W${w}N${n}E${e}U${u}H${h}`; } function permFromString(s: string): DenoPermissions { - const re = /^permR([01])W([01])N([01])E([01])U([01])$/; + const re = /^permR([01])W([01])N([01])E([01])U([01])H([01])$/; const found = s.match(re); if (!found) { throw Error("Not a permission string"); @@ -48,7 +50,8 @@ function permFromString(s: string): DenoPermissions { write: Boolean(Number(found[2])), net: Boolean(Number(found[3])), env: Boolean(Number(found[4])), - run: Boolean(Number(found[5])) + run: Boolean(Number(found[5])), + highPrecision: Boolean(Number(found[6])) }; } @@ -62,7 +65,14 @@ export function testPerm( export function test(fn: testing.TestFunction): void { testPerm( - { read: false, write: false, net: false, env: false, run: false }, + { + read: false, + write: false, + net: false, + env: false, + run: false, + highPrecision: false + }, fn ); } @@ -73,8 +83,17 @@ test(function permSerialization() { for (const env of [true, false]) { for (const run of [true, false]) { for (const read of [true, false]) { - const perms: DenoPermissions = { write, net, env, run, read }; - assertEquals(perms, permFromString(permToString(perms))); + for (const highPrecision of [true, false]) { + const perms: DenoPermissions = { + write, + net, + env, + run, + read, + highPrecision + }; + assertEquals(perms, permFromString(permToString(perms))); + } } } } diff --git a/tests/025_high_precision.test b/tests/025_high_precision.test new file mode 100644 index 0000000000..2bc460c9bc --- /dev/null +++ b/tests/025_high_precision.test @@ -0,0 +1,2 @@ +args: --allow-high-precision --reload tests/025_high_precision.ts +output: tests/025_high_precision.ts.out diff --git a/tests/025_high_precision.ts b/tests/025_high_precision.ts new file mode 100644 index 0000000000..85b3c839bf --- /dev/null +++ b/tests/025_high_precision.ts @@ -0,0 +1,3 @@ +console.log(performance.now() % 2 !== 0); +Deno.revokePermission("highPrecision"); +console.log(performance.now() % 2 === 0); diff --git a/tests/025_high_precision.ts.out b/tests/025_high_precision.ts.out new file mode 100644 index 0000000000..bb101b641b --- /dev/null +++ b/tests/025_high_precision.ts.out @@ -0,0 +1,2 @@ +true +true diff --git a/tools/unit_tests.py b/tools/unit_tests.py index c774497598..21fea32318 100755 --- a/tools/unit_tests.py +++ b/tools/unit_tests.py @@ -45,14 +45,16 @@ def run_unit_test(deno_exe, permStr, flags=None): # tests by the special string. permW0N0 means allow-write but not allow-net. # See js/test_util.ts for more details. def unit_tests(deno_exe): - run_unit_test(deno_exe, "permR0W0N0E0U0", ["--reload"]) - run_unit_test(deno_exe, "permR1W0N0E0U0", ["--allow-read"]) - run_unit_test(deno_exe, "permR0W1N0E0U0", ["--allow-write"]) - run_unit_test(deno_exe, "permR1W1N0E0U0", + run_unit_test(deno_exe, "permR0W0N0E0U0H0", ["--reload"]) + run_unit_test(deno_exe, "permR1W0N0E0U0H0", ["--allow-read"]) + run_unit_test(deno_exe, "permR0W1N0E0U0H0", ["--allow-write"]) + run_unit_test(deno_exe, "permR1W1N0E0U0H0", ["--allow-read", "--allow-write"]) - run_unit_test(deno_exe, "permR0W0N0E1U0", ["--allow-env"]) - run_unit_test(deno_exe, "permR0W0N0E0U1", ["--allow-run"]) - run_unit_test(deno_exe, "permR0W1N0E0U1", ["--allow-run", "--allow-write"]) + run_unit_test(deno_exe, "permR0W0N0E1U0H0", ["--allow-env"]) + run_unit_test(deno_exe, "permR0W0N0E0U0H1", ["--allow-high-precision"]) + run_unit_test(deno_exe, "permR0W0N0E0U1H0", ["--allow-run"]) + run_unit_test(deno_exe, "permR0W1N0E0U1H0", + ["--allow-run", "--allow-write"]) # TODO We might accidentally miss some. We should be smarter about which we # run. Maybe we can use the "filtered out" number to check this.