Skip to content

Commit

Permalink
perf: remove knowledge of promise IDs from deno (denoland#21132)
Browse files Browse the repository at this point in the history
We can move all promise ID knowledge to deno_core, allowing us to better
experiment with promise implementation in deno_core.

`{un,}refOpPromise(promise)` is equivalent to
`{un,}refOp(promise[promiseIdSymbol])`
  • Loading branch information
mmastrac committed Nov 9, 2023
1 parent c4029f6 commit 9010b8d
Show file tree
Hide file tree
Showing 11 changed files with 70 additions and 124 deletions.
55 changes: 6 additions & 49 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.228.0" }
deno_core = { version = "0.229.0" }

deno_runtime = { version = "0.130.0", path = "./runtime" }
napi_sym = { version = "0.52.0", path = "./cli/napi/sym" }
Expand Down
6 changes: 3 additions & 3 deletions cli/tests/unit/ref_unref_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@

import { assertNotEquals, execCode } from "./test_util.ts";

Deno.test("[unrefOp] unref'ing invalid ops does not have effects", async () => {
Deno.test("[unrefOpPromise] unref'ing invalid ops does not have effects", async () => {
const [statusCode, _] = await execCode(`
Deno[Deno.internal].core.unrefOp(-1);
Deno[Deno.internal].core.unrefOpPromise(new Promise(r => null));
setTimeout(() => { throw new Error() }, 10)
`);
// Invalid unrefOp call doesn't affect exit condition of event loop
// Invalid unrefOpPromise call doesn't affect exit condition of event loop
assertNotEquals(statusCode, 0);
});
7 changes: 2 additions & 5 deletions ext/ffi/00_ffi.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ const {
SafeMap,
SafeArrayIterator,
SafeWeakMap,
SymbolFor,
} = primordials;
import { pathFromURL } from "ext:deno_web/00_infra.js";

Expand All @@ -52,8 +51,6 @@ function getBufferSourceByteLength(source) {
}
return ArrayBufferPrototypeGetByteLength(source);
}
const promiseIdSymbol = SymbolFor("Deno.core.internalPromiseId");

const U32_BUFFER = new Uint32Array(2);
const U64_BUFFER = new BigUint64Array(TypedArrayPrototypeGetBuffer(U32_BUFFER));
const I64_BUFFER = new BigInt64Array(TypedArrayPrototypeGetBuffer(U32_BUFFER));
Expand Down Expand Up @@ -422,7 +419,7 @@ class UnsafeCallback {
if (this.#refcount++ === 0) {
if (this.#refpromise) {
// Re-refing
core.refOp(this.#refpromise[promiseIdSymbol]);
core.refOpPromise(this.#refpromise);
} else {
this.#refpromise = core.opAsync(
"op_ffi_unsafe_callback_ref",
Expand All @@ -437,7 +434,7 @@ class UnsafeCallback {
// Only decrement refcount if it is positive, and only
// unref the callback if refcount reaches zero.
if (this.#refcount > 0 && --this.#refcount === 0) {
core.unrefOp(this.#refpromise[promiseIdSymbol]);
core.unrefOpPromise(this.#refpromise);
}
return this.#refcount;
}
Expand Down
8 changes: 3 additions & 5 deletions ext/http/00_serve.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ const {
ObjectPrototypeIsPrototypeOf,
PromisePrototypeCatch,
Symbol,
SymbolFor,
TypeError,
Uint8Array,
Uint8ArrayPrototype,
Expand Down Expand Up @@ -642,7 +641,6 @@ function serveHttpOnConnection(connection, signal, handler, onError, onListen) {
function serveHttpOn(context, callback) {
let ref = true;
let currentPromise = null;
const promiseIdSymbol = SymbolFor("Deno.core.internalPromiseId");

const promiseErrorHandler = (error) => {
// Abnormal exit
Expand All @@ -666,7 +664,7 @@ function serveHttpOn(context, callback) {
}
currentPromise = op_http_wait(rid);
if (!ref) {
core.unrefOp(currentPromise[promiseIdSymbol]);
core.unrefOpPromise(currentPromise);
}
req = await currentPromise;
currentPromise = null;
Expand Down Expand Up @@ -708,13 +706,13 @@ function serveHttpOn(context, callback) {
ref() {
ref = true;
if (currentPromise) {
core.refOp(currentPromise[promiseIdSymbol]);
core.refOpPromise(currentPromise);
}
},
unref() {
ref = false;
if (currentPromise) {
core.unrefOp(currentPromise[promiseIdSymbol]);
core.unrefOpPromise(currentPromise);
}
},
[SymbolAsyncDispose]() {
Expand Down
50 changes: 24 additions & 26 deletions ext/net/01_net.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,23 +13,21 @@ import { SymbolDispose } from "ext:deno_web/00_infra.js";

const primordials = globalThis.__bootstrap.primordials;
const {
ArrayPrototypeFilter,
ArrayPrototypeForEach,
ArrayPrototypePush,
Error,
Number,
ObjectPrototypeIsPrototypeOf,
PromiseResolve,
SafeSet,
SetPrototypeAdd,
SetPrototypeDelete,
SetPrototypeForEach,
SymbolAsyncIterator,
Symbol,
SymbolFor,
TypeError,
TypedArrayPrototypeSubarray,
Uint8Array,
} = primordials;

const promiseIdSymbol = SymbolFor("Deno.core.internalPromiseId");

async function write(rid, data) {
return await core.write(rid, data);
}
Expand Down Expand Up @@ -70,7 +68,7 @@ class Conn {
#remoteAddr = null;
#localAddr = null;
#unref = false;
#pendingReadPromiseIds = [];
#pendingReadPromises = new SafeSet();

#readable;
#writable;
Expand Down Expand Up @@ -102,19 +100,15 @@ class Conn {
return 0;
}
const promise = core.read(this.rid, buffer);
const promiseId = promise[promiseIdSymbol];
if (this.#unref) core.unrefOp(promiseId);
ArrayPrototypePush(this.#pendingReadPromiseIds, promiseId);
if (this.#unref) core.unrefOpPromise(promise);
SetPrototypeAdd(this.#pendingReadPromises, promise);
let nread;
try {
nread = await promise;
} catch (e) {
throw e;
} finally {
this.#pendingReadPromiseIds = ArrayPrototypeFilter(
this.#pendingReadPromiseIds,
(id) => id !== promiseId,
);
SetPrototypeDelete(this.#pendingReadPromises, promise);
}
return nread === 0 ? null : nread;
}
Expand Down Expand Up @@ -149,17 +143,21 @@ class Conn {
if (this.#readable) {
readableStreamForRidUnrefableRef(this.#readable);
}
ArrayPrototypeForEach(this.#pendingReadPromiseIds, (id) => core.refOp(id));

SetPrototypeForEach(
this.#pendingReadPromises,
(promise) => core.refOpPromise(promise),
);
}

unref() {
this.#unref = true;
if (this.#readable) {
readableStreamForRidUnrefableUnref(this.#readable);
}
ArrayPrototypeForEach(
this.#pendingReadPromiseIds,
(id) => core.unrefOp(id),
SetPrototypeForEach(
this.#pendingReadPromises,
(promise) => core.unrefOpPromise(promise),
);
}

Expand All @@ -184,7 +182,7 @@ class Listener {
#rid = 0;
#addr = null;
#unref = false;
#promiseId = null;
#promise = null;

constructor(rid, addr) {
this.#rid = rid;
Expand All @@ -211,10 +209,10 @@ class Listener {
default:
throw new Error(`Unsupported transport: ${this.addr.transport}`);
}
this.#promiseId = promise[promiseIdSymbol];
if (this.#unref) core.unrefOp(this.#promiseId);
this.#promise = promise;
if (this.#unref) core.unrefOpPromise(promise);
const { 0: rid, 1: localAddr, 2: remoteAddr } = await promise;
this.#promiseId = null;
this.#promise = null;
if (this.addr.transport == "tcp") {
localAddr.transport = "tcp";
remoteAddr.transport = "tcp";
Expand Down Expand Up @@ -265,15 +263,15 @@ class Listener {

ref() {
this.#unref = false;
if (typeof this.#promiseId === "number") {
core.refOp(this.#promiseId);
if (this.#promise !== null) {
core.refOpPromise(this.#promise);
}
}

unref() {
this.#unref = true;
if (typeof this.#promiseId === "number") {
core.unrefOp(this.#promiseId);
if (this.#promise !== null) {
core.unrefOpPromise(this.#promise);
}
}
}
Expand Down
15 changes: 7 additions & 8 deletions ext/node/polyfills/http2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ const kDenoResponse = Symbol("kDenoResponse");
const kDenoRid = Symbol("kDenoRid");
const kDenoClientRid = Symbol("kDenoClientRid");
const kDenoConnRid = Symbol("kDenoConnRid");
const kPollConnPromiseId = Symbol("kPollConnPromiseId");
const kPollConnPromise = Symbol("kPollConnPromise");

const STREAM_FLAGS_PENDING = 0x0;
const STREAM_FLAGS_READY = 0x1;
Expand Down Expand Up @@ -364,7 +364,7 @@ export class ClientHttp2Session extends Http2Session {
this[kPendingRequestCalls] = null;
this[kDenoClientRid] = undefined;
this[kDenoConnRid] = undefined;
this[kPollConnPromiseId] = undefined;
this[kPollConnPromise] = undefined;

socket.on("error", socketOnError);
socket.on("close", socketOnClose);
Expand Down Expand Up @@ -394,8 +394,7 @@ export class ClientHttp2Session extends Http2Session {
"op_http2_poll_client_connection",
this[kDenoConnRid],
);
this[kPollConnPromiseId] =
promise[Symbol.for("Deno.core.internalPromiseId")];
this[kPollConnPromise] = promise;
if (!this.#refed) {
this.unref();
}
Expand All @@ -410,15 +409,15 @@ export class ClientHttp2Session extends Http2Session {

ref() {
this.#refed = true;
if (this[kPollConnPromiseId]) {
core.refOp(this[kPollConnPromiseId]);
if (this[kPollConnPromise]) {
core.refOpPromise(this[kPollConnPromise]);
}
}

unref() {
this.#refed = false;
if (this[kPollConnPromiseId]) {
core.unrefOp(this[kPollConnPromiseId]);
if (this[kPollConnPromise]) {
core.unrefOpPromise(this[kPollConnPromise]);
}
}

Expand Down
Loading

0 comments on commit 9010b8d

Please sign in to comment.