Skip to content

Commit

Permalink
chore: migrate to new deno_core and metrics (denoland#21057)
Browse files Browse the repository at this point in the history
- Uses the new OpMetrics system for sync and async calls
- Partial revert of denoland#21048 as we moved Array.fromAsync upstream to
deno_core
  • Loading branch information
mmastrac committed Nov 5, 2023
1 parent 4530cd5 commit 485fade
Show file tree
Hide file tree
Showing 19 changed files with 102 additions and 177 deletions.
12 changes: 6 additions & 6 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ repository = "https://github.com/denoland/deno"

[workspace.dependencies]
deno_ast = { version = "0.31.2", features = ["transpiling"] }
deno_core = { version = "0.224.0" }
deno_core = { version = "0.225.0" }

deno_runtime = { version = "0.130.0", path = "./runtime" }
napi_sym = { version = "0.52.0", path = "./cli/napi/sym" }
Expand Down
1 change: 1 addition & 0 deletions cli/args/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,7 @@ pub struct Flags {
pub config_flag: ConfigFlag,
pub node_modules_dir: Option<bool>,
pub vendor: Option<bool>,
pub enable_op_summary_metrics: bool,
pub enable_testing_features: bool,
pub ext: Option<String>,
pub ignore: Vec<PathBuf>,
Expand Down
8 changes: 8 additions & 0 deletions cli/args/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1137,6 +1137,14 @@ impl CliOptions {
}
}

pub fn enable_op_summary_metrics(&self) -> bool {
self.flags.enable_op_summary_metrics
|| matches!(
self.flags.subcommand,
DenoSubcommand::Test(_) | DenoSubcommand::Repl(_)
)
}

pub fn enable_testing_features(&self) -> bool {
self.flags.enable_testing_features
}
Expand Down
1 change: 1 addition & 0 deletions cli/factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -670,6 +670,7 @@ impl CliFactory {
argv: self.options.argv().clone(),
log_level: self.options.log_level().unwrap_or(log::Level::Info).into(),
coverage_dir: self.options.coverage_dir(),
enable_op_summary_metrics: self.options.enable_op_summary_metrics(),
enable_testing_features: self.options.enable_testing_features(),
has_node_modules_dir: self.options.has_node_modules_dir(),
hmr: self.options.has_hmr(),
Expand Down
3 changes: 1 addition & 2 deletions cli/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -367,8 +367,7 @@ pub fn main() {
// Using same default as VSCode:
// https://github.com/microsoft/vscode/blob/48d4ba271686e8072fc6674137415bc80d936bc7/extensions/typescript-language-features/src/configuration/configuration.ts#L213-L214
DenoSubcommand::Lsp => vec!["--max-old-space-size=3072".to_string()],
// TODO(bartlomieju): upstream this to `deno_core` crate
_ => vec!["--harmony-array-from-async".to_string()],
_ => vec![],
};
init_v8_flags(&default_v8_flags, &flags.v8_flags, get_v8_flags_from_env());
deno_core::JsRuntime::init_platform(None);
Expand Down
94 changes: 41 additions & 53 deletions cli/ops/testing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,17 @@ use deno_core::error::AnyError;
use deno_core::op2;
use deno_core::v8;
use deno_core::ModuleSpecifier;
use deno_core::OpMetrics;
use deno_core::OpMetricsSummary;
use deno_core::OpMetricsSummaryTracker;
use deno_core::OpState;
use deno_runtime::deno_fetch::reqwest;
use deno_runtime::permissions::create_child_permissions;
use deno_runtime::permissions::ChildPermissionsArg;
use deno_runtime::permissions::PermissionsContainer;
use serde::Serialize;
use std::cell::Ref;
use std::collections::hash_map::Entry;
use std::collections::HashMap;
use std::rc::Rc;
use std::sync::atomic::AtomicUsize;
use std::sync::atomic::Ordering;
use uuid::Uuid;
Expand Down Expand Up @@ -243,38 +244,28 @@ fn op_test_event_step_result_failed(
struct TestOpSanitizers(HashMap<u32, TestOpSanitizerState>);

enum TestOpSanitizerState {
Collecting { metrics: Vec<OpMetrics> },
Collecting { metrics: Vec<OpMetricsSummary> },
Finished { report: Vec<TestOpSanitizerReport> },
}

fn try_collect_metrics(
state: &OpState,
metrics: &OpMetricsSummaryTracker,
force: bool,
op_id_host_recv_msg: usize,
op_id_host_recv_ctrl: usize,
) -> Result<Ref<Vec<OpMetrics>>, bool> {
let metrics = state.tracker.per_op();
for op_metric in &*metrics {
let has_pending_ops = op_metric.ops_dispatched_async
+ op_metric.ops_dispatched_async_unref
> op_metric.ops_completed_async + op_metric.ops_completed_async_unref;
if has_pending_ops && !force {
let host_recv_msg = metrics
.get(op_id_host_recv_msg)
.map(|op_metric| {
op_metric.ops_dispatched_async + op_metric.ops_dispatched_async_unref
> op_metric.ops_completed_async
+ op_metric.ops_completed_async_unref
})
.unwrap_or(false);
let host_recv_ctrl = metrics
.get(op_id_host_recv_ctrl)
.map(|op_metric| {
op_metric.ops_dispatched_async + op_metric.ops_dispatched_async_unref
> op_metric.ops_completed_async
+ op_metric.ops_completed_async_unref
})
.unwrap_or(false);
) -> Result<std::cell::Ref<Vec<OpMetricsSummary>>, bool> {
let metrics = metrics.per_op();
let host_recv_msg = metrics
.get(op_id_host_recv_msg)
.map(OpMetricsSummary::has_outstanding_ops)
.unwrap_or(false);
let host_recv_ctrl = metrics
.get(op_id_host_recv_ctrl)
.map(OpMetricsSummary::has_outstanding_ops)
.unwrap_or(false);

for op_metric in metrics.iter() {
if op_metric.has_outstanding_ops() && !force {
return Err(host_recv_msg || host_recv_ctrl);
}
}
Expand All @@ -294,23 +285,23 @@ fn op_test_op_sanitizer_collect(
#[smi] op_id_host_recv_msg: usize,
#[smi] op_id_host_recv_ctrl: usize,
) -> Result<u8, AnyError> {
let metrics = {
let metrics = match try_collect_metrics(
state,
force,
op_id_host_recv_msg,
op_id_host_recv_ctrl,
) {
Ok(metrics) => metrics,
Err(false) => {
return Ok(1);
}
Err(true) => {
return Ok(2);
}
};
metrics.clone()
};
let metrics = state.borrow::<Rc<OpMetricsSummaryTracker>>();
let metrics = match try_collect_metrics(
metrics,
force,
op_id_host_recv_msg,
op_id_host_recv_ctrl,
) {
Ok(metrics) => metrics,
Err(false) => {
return Ok(1);
}
Err(true) => {
return Ok(2);
}
}
.clone();

let op_sanitizers = state.borrow_mut::<TestOpSanitizers>();
match op_sanitizers.0.entry(id) {
Entry::Vacant(entry) => {
Expand Down Expand Up @@ -348,11 +339,12 @@ fn op_test_op_sanitizer_finish(
) -> Result<u8, AnyError> {
// Drop `fetch` connection pool at the end of a test
state.try_take::<reqwest::Client>();
let metrics = state.borrow::<Rc<OpMetricsSummaryTracker>>();

// Generate a report of pending ops
let report = {
let after_metrics = match try_collect_metrics(
state,
metrics,
force,
op_id_host_recv_msg,
op_id_host_recv_ctrl,
Expand Down Expand Up @@ -380,14 +372,10 @@ fn op_test_op_sanitizer_finish(
for (id, (before, after)) in
before_metrics.iter().zip(after_metrics.iter()).enumerate()
{
let async_pending_before = before.ops_dispatched_async
+ before.ops_dispatched_async_unref
- before.ops_completed_async
- before.ops_completed_async_unref;
let async_pending_after = after.ops_dispatched_async
+ after.ops_dispatched_async_unref
- after.ops_completed_async
- after.ops_completed_async_unref;
let async_pending_before =
before.ops_dispatched_async - before.ops_completed_async;
let async_pending_after =
after.ops_dispatched_async - after.ops_completed_async;
let diff = async_pending_after as i64 - async_pending_before as i64;
if diff != 0 {
report.push(TestOpSanitizerReport { id, diff });
Expand Down
1 change: 1 addition & 0 deletions cli/standalone/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,7 @@ pub async fn run(
argv: metadata.argv,
log_level: WorkerLogLevel::Info,
coverage_dir: None,
enable_op_summary_metrics: false,
enable_testing_features: false,
has_node_modules_dir,
hmr: false,
Expand Down
1 change: 0 additions & 1 deletion cli/tests/integration/js_unit_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ util::unit_test_factory!(
link_test,
make_temp_test,
message_channel_test,
metrics_test,
mkdir_test,
navigator_test,
net_test,
Expand Down
5 changes: 0 additions & 5 deletions cli/tests/integration/run_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3425,11 +3425,6 @@ itest!(unstable_ffi_19 {
exit_code: 70,
});

itest!(future_check2 {
args: "run --check run/future_check.ts",
output: "run/future_check2.out",
});

itest!(event_listener_error {
args: "run --quiet run/event_listener_error.ts",
output: "run/event_listener_error.ts.out",
Expand Down
1 change: 0 additions & 1 deletion cli/tests/testdata/run/future_check.ts

This file was deleted.

1 change: 0 additions & 1 deletion cli/tests/testdata/run/future_check2.out

This file was deleted.

93 changes: 0 additions & 93 deletions cli/tests/unit/metrics_test.ts

This file was deleted.

13 changes: 0 additions & 13 deletions cli/tests/unit/timers_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -412,19 +412,6 @@ Deno.test(function clearTimeoutAndClearIntervalNotBeEquals() {
assertNotEquals(clearTimeout, clearInterval);
});

Deno.test(async function timerMaxCpuBug() {
// There was a bug where clearing a timeout would cause Deno to use 100% CPU.
clearTimeout(setTimeout(() => {}, 1000));
// We can check this by counting how many ops have triggered in the interim.
// Certainly less than 10 ops should have been dispatched in next 100 ms.
const { ops: pre } = Deno.metrics();
await delay(100);
const { ops: post } = Deno.metrics();
const before = pre.op_sleep.opsDispatched;
const after = post.op_sleep.opsDispatched;
assert(after - before < 10);
});

Deno.test(async function timerOrdering() {
const array: number[] = [];
const donePromise = deferred();
Expand Down
2 changes: 1 addition & 1 deletion cli/tsc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -948,7 +948,7 @@ mod tests {
.context("Unable to get CWD")
.unwrap(),
);
let mut op_state = OpState::new(1, None);
let mut op_state = OpState::new(None);
op_state.put(state);
op_state
}
Expand Down
Loading

0 comments on commit 485fade

Please sign in to comment.