Skip to content

Commit

Permalink
fix op dispatch/completed metrics
Browse files Browse the repository at this point in the history
  • Loading branch information
ry committed Aug 22, 2019
1 parent 8675c27 commit 28f7778
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 27 deletions.
9 changes: 1 addition & 8 deletions cli/ops/dispatch_flatbuffers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,13 +62,8 @@ pub fn dispatch(

let op_result = op_func(state, &base, zero_copy);

let state = state.clone();

match op_result {
Ok(Op::Sync(buf)) => {
state.metrics_op_completed(buf.len());
Op::Sync(buf)
}
Ok(Op::Sync(buf)) => Op::Sync(buf),
Ok(Op::Async(fut)) => {
let result_fut = Box::new(
fut
Expand Down Expand Up @@ -104,7 +99,6 @@ pub fn dispatch(
},
)
};
state.metrics_op_completed(buf.len());
Ok(buf)
})
.map_err(|err| panic!("unexpected error {:?}", err)),
Expand All @@ -126,7 +120,6 @@ pub fn dispatch(
..Default::default()
},
);
state.metrics_op_completed(response_buf.len());
Op::Sync(response_buf)
}
}
Expand Down
11 changes: 3 additions & 8 deletions cli/ops/dispatch_minimal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,19 +74,15 @@ fn test_parse_min_record() {

pub fn dispatch(
d: Dispatcher,
state: &ThreadSafeState,
_state: &ThreadSafeState,
control: &[u8],
zero_copy: Option<PinnedBuf>,
) -> CoreOp {
let mut record = parse_min_record(control).unwrap();
let is_sync = record.promise_id == 0;

// TODO(ry) Currently there aren't any sync minimal ops. This is just a sanity
// check. Remove later.
assert!(!is_sync);

let state = state.clone();

let rid = record.arg;
let min_op = d(rid, zero_copy);

Expand All @@ -102,10 +98,9 @@ pub fn dispatch(
record.result = -1;
}
}
let buf: Buf = record.into();
state.metrics_op_completed(buf.len());
Ok(buf)
Ok(record.into())
}));

if is_sync {
Op::Sync(fut.wait().unwrap())
} else {
Expand Down
17 changes: 16 additions & 1 deletion cli/ops/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,5 +79,20 @@ pub fn dispatch(
};

state.metrics_op_dispatched(bytes_sent_control, bytes_sent_zero_copy);
op

match op {
Op::Sync(buf) => {
state.metrics_op_completed(buf.len());
Op::Sync(buf)
}
Op::Async(fut) => {
use crate::futures::Future;
let state = state.clone();
let result_fut = Box::new(fut.map(move |buf: Buf| {
state.clone().metrics_op_completed(buf.len());
buf
}));
Op::Async(result_fut)
}
}
}
7 changes: 0 additions & 7 deletions js/dispatch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,6 @@ export function handleAsyncMsgFromRust(opId: number, ui8: Uint8Array): void {
case OP_UTIME:
json.handleAsyncMsgFromRust(opId, ui8);
break;
case OP_EXIT:
case OP_IS_TTY:
case OP_ENV:
case OP_EXEC_PATH:
case OP_SET_ENV:
case OP_HOME_DIR:
throw Error("expected sync");
default:
throw Error("bad opId");
}
Expand Down
6 changes: 3 additions & 3 deletions js/metrics_test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// Copyright 2018-2019 the Deno authors. All rights reserved. MIT license.
import { test, testPerm, assert } from "./test_util.ts";
import { test, testPerm, assert, assertEquals } from "./test_util.ts";

test(async function metrics(): Promise<void> {
const m1 = Deno.metrics();
Expand Down Expand Up @@ -29,7 +29,7 @@ testPerm({ write: true }, function metricsUpdatedIfNoResponseSync(): void {
Deno.writeFileSync(filename, data, { perm: 0o666 });

const metrics = Deno.metrics();
assert(metrics.opsDispatched === metrics.opsCompleted);
assertEquals(metrics.opsDispatched, metrics.opsCompleted);
});

testPerm(
Expand All @@ -41,6 +41,6 @@ testPerm(
await Deno.writeFile(filename, data, { perm: 0o666 });

const metrics = Deno.metrics();
assert(metrics.opsDispatched === metrics.opsCompleted);
assertEquals(metrics.opsDispatched, metrics.opsCompleted);
}
);

0 comments on commit 28f7778

Please sign in to comment.