Skip to content
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

Deno.core.dispatch uses op name instead of id #6395

Merged
merged 4 commits into from
Jun 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions cli/js/globals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,11 @@ declare global {
control: Uint8Array,
...zeroCopy: ArrayBufferView[]
): Uint8Array | null;
dispatchByName(
opName: string,
control: Uint8Array,
...zeroCopy: ArrayBufferView[]
): Uint8Array | null;
setAsyncHandler(opId: number, cb: (msg: Uint8Array) => void): void;
sharedQueue: {
head(): number;
Expand Down
13 changes: 4 additions & 9 deletions cli/js/ops/dispatch_json.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// Copyright 2018-2020 the Deno authors. All rights reserved. MIT license.
import * as util from "../util.ts";
import { core } from "../core.ts";
import { OPS_CACHE } from "../runtime.ts";
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removes OPS_CACHE indirection which was once problematic place when trying to snapshot ES modules

import { ErrorKind, getErrorClass } from "../errors.ts";

// eslint-disable-next-line @typescript-eslint/no-explicit-any
Expand Down Expand Up @@ -61,12 +60,10 @@ export function sendSync(
args: object = {},
...zeroCopy: Uint8Array[]
): Ok {
const opId = OPS_CACHE[opName];
util.log("sendSync", opName, opId);
util.log("sendSync", opName);
const argsUi8 = encode(args);
const resUi8 = core.dispatch(opId, argsUi8, ...zeroCopy);
const resUi8 = core.dispatchByName(opName, argsUi8, ...zeroCopy);
util.assert(resUi8 != null);

const res = decode(resUi8);
util.assert(res.promiseId == null);
return unwrapResponse(res);
Expand All @@ -77,14 +74,12 @@ export async function sendAsync(
args: object = {},
...zeroCopy: Uint8Array[]
): Promise<Ok> {
const opId = OPS_CACHE[opName];
util.log("sendAsync", opName, opId);
util.log("sendAsync", opName);
const promiseId = nextPromiseId();
args = Object.assign(args, { promiseId });
const promise = util.createResolvable<Ok>();

const argsUi8 = encode(args);
const buf = core.dispatch(opId, argsUi8, ...zeroCopy);
const buf = core.dispatchByName(opName, argsUi8, ...zeroCopy);
if (buf) {
// Sync result.
const res = decode(buf);
Expand Down
8 changes: 4 additions & 4 deletions cli/js/ops/dispatch_minimal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ export function asyncMsgFromRust(ui8: Uint8Array): void {
}

export async function sendAsyncMinimal(
opId: number,
opName: string,
arg: number,
zeroCopy: Uint8Array
): Promise<number> {
Expand All @@ -92,7 +92,7 @@ export async function sendAsyncMinimal(
scratch32[1] = arg;
scratch32[2] = 0; // result
const promise = util.createResolvable<RecordMinimal>();
const buf = core.dispatch(opId, scratchBytes, zeroCopy);
const buf = core.dispatchByName(opName, scratchBytes, zeroCopy);
if (buf) {
const record = recordFromBufMinimal(buf);
// Sync result.
Expand All @@ -107,13 +107,13 @@ export async function sendAsyncMinimal(
}

export function sendSyncMinimal(
opId: number,
opName: string,
arg: number,
zeroCopy: Uint8Array
): number {
scratch32[0] = 0; // promiseId 0 indicates sync
scratch32[1] = arg;
const res = core.dispatch(opId, scratchBytes, zeroCopy)!;
const res = core.dispatchByName(opName, scratchBytes, zeroCopy)!;
const resRecord = recordFromBufMinimal(res);
return unwrapResponse(resRecord);
}
27 changes: 4 additions & 23 deletions cli/js/ops/io.ts
Original file line number Diff line number Diff line change
@@ -1,22 +1,12 @@
// Copyright 2018-2020 the Deno authors. All rights reserved. MIT license.

import { sendAsyncMinimal, sendSyncMinimal } from "./dispatch_minimal.ts";
// TODO(bartlomieju): remove this import and maybe lazy-initialize
// OPS_CACHE that belongs only to this module
import { OPS_CACHE } from "../runtime.ts";
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto


// This is done because read/write are extremely performance sensitive.
let OP_READ = -1;
let OP_WRITE = -1;

export function readSync(rid: number, buffer: Uint8Array): number | null {
if (buffer.length == 0) {
return 0;
}
if (OP_READ < 0) {
OP_READ = OPS_CACHE["op_read"];
}
const nread = sendSyncMinimal(OP_READ, rid, buffer);
const nread = sendSyncMinimal("op_read", rid, buffer);
if (nread < 0) {
throw new Error("read error");
} else if (nread == 0) {
Expand All @@ -33,10 +23,7 @@ export async function read(
if (buffer.length == 0) {
return 0;
}
if (OP_READ < 0) {
OP_READ = OPS_CACHE["op_read"];
}
const nread = await sendAsyncMinimal(OP_READ, rid, buffer);
const nread = await sendAsyncMinimal("op_read", rid, buffer);
if (nread < 0) {
throw new Error("read error");
} else if (nread == 0) {
Expand All @@ -47,10 +34,7 @@ export async function read(
}

export function writeSync(rid: number, data: Uint8Array): number {
if (OP_WRITE < 0) {
OP_WRITE = OPS_CACHE["op_write"];
}
const result = sendSyncMinimal(OP_WRITE, rid, data);
const result = sendSyncMinimal("op_write", rid, data);
if (result < 0) {
throw new Error("write error");
} else {
Expand All @@ -59,10 +43,7 @@ export function writeSync(rid: number, data: Uint8Array): number {
}

export async function write(rid: number, data: Uint8Array): Promise<number> {
if (OP_WRITE < 0) {
OP_WRITE = OPS_CACHE["op_write"];
}
const result = await sendAsyncMinimal(OP_WRITE, rid, data);
const result = await sendAsyncMinimal("op_write", rid, data);
if (result < 0) {
throw new Error("write error");
} else {
Expand Down
6 changes: 2 additions & 4 deletions cli/js/runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ import { setPrepareStackTrace } from "./error_stack.ts";
import { Start, opStart } from "./ops/runtime.ts";
import { handleTimerMacrotask } from "./web/timers.ts";

export let OPS_CACHE: { [name: string]: number };

function getAsyncHandler(opName: string): (msg: Uint8Array) => void {
switch (opName) {
case "op_write":
Expand All @@ -24,8 +22,8 @@ function getAsyncHandler(opName: string): (msg: Uint8Array) => void {
// TODO(bartlomieju): temporary solution, must be fixed when moving
// dispatches to separate crates
export function initOps(): void {
OPS_CACHE = core.ops();
for (const [name, opId] of Object.entries(OPS_CACHE)) {
const opsMap = core.ops();
for (const [name, opId] of Object.entries(opsMap)) {
core.setAsyncHandler(opId, getAsyncHandler(name));
}
Comment on lines +26 to 28
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Async handler registration is the last API bit that's a bit strange IMO and could be structured better

core.setMacrotaskCallback(handleTimerMacrotask);
Expand Down
9 changes: 8 additions & 1 deletion core/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ SharedQueue Binary Layout
let asyncHandlers;

let initialized = false;
let opsCache = {};

function maybeInit() {
if (!initialized) {
Expand All @@ -61,7 +62,8 @@ SharedQueue Binary Layout
// op id 0 is a special value to retrieve the map of registered ops.
const opsMapBytes = send(0, new Uint8Array([]));
const opsMapJson = String.fromCharCode.apply(null, opsMapBytes);
return JSON.parse(opsMapJson);
opsCache = JSON.parse(opsMapJson);
return { ...opsCache };
}

function assert(cond) {
Expand Down Expand Up @@ -181,9 +183,14 @@ SharedQueue Binary Layout
}
}

function dispatch(opName, control, ...zeroCopy) {
return send(opsCache[opName], control, ...zeroCopy);
}

Object.assign(window.Deno.core, {
setAsyncHandler,
dispatch: send,
dispatchByName: dispatch,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

name to be determined

ops,
// sharedQueue is private but exposed for testing.
sharedQueue: {
Expand Down