generated from legendecas/tc39-proposal
-
Notifications
You must be signed in to change notification settings - Fork 16
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
Optimize run to avoid quadratic map cloning #15
Merged
Merged
Changes from 14 commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
25e2f85
Optimize AsyncContext to avoid quadratic cloning
jridgewell 1337cbf
Revamp tests
jridgewell f149be2
Fix bug with mutating mappings frozen during deep run
jridgewell 57cce61
Prettier
jridgewell 1c3b3aa
Do now freeze initial state, ensure we restore frozen state
jridgewell 4df3e56
Test PRs
jridgewell 29878f4
Cleanup
jridgewell bd5c6ae
Check types
jridgewell 9f96230
Fix branch name
jridgewell 02a10a4
Fix node 14 tests
jridgewell 1873d43
Isolate initial state to Mapping
jridgewell 8591569
Test run within wrap
jridgewell 168d8fb
Remove debuggers
jridgewell b31b1ff
Ensure we test both initial and run states
jridgewell 235e780
Apply suggestions from code review
jridgewell 60c3700
Update naming and comments
jridgewell File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
name: Node.js CI | ||
|
||
on: | ||
push: | ||
branches: [ master ] | ||
pull_request: | ||
branches: [ master ] | ||
|
||
jobs: | ||
test: | ||
name: "Test" | ||
runs-on: ubuntu-latest | ||
|
||
strategy: | ||
matrix: | ||
node-version: [14.x, 16.x, 18.x] | ||
|
||
steps: | ||
- uses: actions/checkout@v3 | ||
- name: Use Node.js ${{ matrix.node-version }} | ||
uses: actions/setup-node@v3 | ||
with: | ||
node-version: ${{ matrix.node-version }} | ||
cache: 'npm' | ||
- run: npm ci | ||
- run: npm test | ||
|
||
lint: | ||
name: "Lint" | ||
runs-on: ubuntu-latest | ||
|
||
strategy: | ||
matrix: | ||
node-version: [18.x] | ||
|
||
steps: | ||
- uses: actions/checkout@v3 | ||
- name: Use Node.js ${{ matrix.node-version }} | ||
uses: actions/setup-node@v3 | ||
with: | ||
node-version: ${{ matrix.node-version }} | ||
cache: 'npm' | ||
- run: npm ci | ||
- run: npm run lint |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
import type { Mapping } from "./mapping"; | ||
import type { AsyncContext } from "./index"; | ||
|
||
/** | ||
* FrozenFork holds a frozen Mapping that will be simply restored when the fork is | ||
* rejoined. | ||
* | ||
* This is used when we already know that the mapping is frozen, so that | ||
* rejoining will not attempt to mutate the Mapping (and allocate a new | ||
* mapping) as an OwnedFork would. | ||
*/ | ||
export class FrozenFork { | ||
#mapping: Mapping; | ||
|
||
constructor(mapping: Mapping) { | ||
this.#mapping = mapping; | ||
} | ||
|
||
/** | ||
* The Storage container will call join when it wants to restore its current | ||
* Mapping to the state at the start of the fork. | ||
* | ||
* For FrozenFork, that's as simple as returning the known-frozen Mapping, | ||
* because we know it can't have been modified. | ||
*/ | ||
join(_current: Mapping): Mapping { | ||
return this.#mapping; | ||
} | ||
} | ||
|
||
/** | ||
* OwnedFork holds an unfrozen Mapping that we will attempt to modify when | ||
* rejoining to attempt to restore it to its prior state. | ||
* | ||
* This is used when we know that the Mapping is unfrozen at start, because | ||
* it's possible that no one will snapshot this Mapping before we rejoin. In | ||
* that case, we can simply modify the Mapping (without cloning) to restore it | ||
* to its prior state. If someone does snapshot it, then modifying will clone | ||
* the current state and we restore the clone to the prior state. | ||
*/ | ||
export class OwnedFork<T> { | ||
#key: AsyncContext<T>; | ||
#has: boolean; | ||
#prev: T | undefined; | ||
|
||
constructor(mapping: Mapping, key: AsyncContext<T>) { | ||
this.#key = key; | ||
this.#has = mapping.has(key); | ||
this.#prev = mapping.get(key); | ||
} | ||
|
||
/** | ||
* The Storage container will call join when it wants to restore its current | ||
* Mapping to the state at the start of the fork. | ||
* | ||
* For OwnedFork, we mutate the known-unfrozen-at-start mapping (which may | ||
* reallocate if anyone has since taken a snapshot) in the hopes that we | ||
* won't need to reallocate. | ||
*/ | ||
join(current: Mapping): Mapping { | ||
if (this.#has) { | ||
return current.set(this.#key, this.#prev); | ||
} else { | ||
return current.delete(this.#key); | ||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,45 +1,38 @@ | ||
type AnyFunc = (...args: any) => any; | ||
type Storage = Map<AsyncContext<unknown>, unknown>; | ||
import { Storage } from "./storage"; | ||
|
||
let __storage__: Storage = new Map(); | ||
type AnyFunc<T> = (this: T, ...args: any) => any; | ||
|
||
export class AsyncContext<T> { | ||
static wrap<F extends AnyFunc>(fn: F): F { | ||
const current = __storage__; | ||
static wrap<F extends AnyFunc<any>>(fn: F): F { | ||
const snapshot = Storage.snapshot(); | ||
|
||
function wrap(...args: Parameters<F>): ReturnType<F> { | ||
return run(fn, current, this, args); | ||
}; | ||
function wrap(this: ThisType<F>, ...args: Parameters<F>): ReturnType<F> { | ||
const fork = Storage.restore(snapshot); | ||
try { | ||
return fn.apply(this, args); | ||
} finally { | ||
Storage.join(fork); | ||
} | ||
} | ||
|
||
return wrap as unknown as F; | ||
} | ||
|
||
run<F extends AnyFunc>( | ||
run<F extends AnyFunc<null>>( | ||
value: T, | ||
fn: F, | ||
...args: Parameters<F> | ||
): ReturnType<F> { | ||
const next = new Map(__storage__); | ||
next.set(this, value); | ||
return run(fn, next, null, args); | ||
const fork = Storage.fork(this); | ||
Storage.set(this, value); | ||
try { | ||
return fn.apply(null, args); | ||
} finally { | ||
Storage.join(fork); | ||
} | ||
} | ||
|
||
get(): T { | ||
return __storage__.get(this) as T; | ||
} | ||
} | ||
|
||
function run<F extends AnyFunc>( | ||
fn: F, | ||
next: Storage, | ||
binding: ThisType<F>, | ||
args: Parameters<F> | ||
): ReturnType<F> { | ||
const previous = __storage__; | ||
try { | ||
__storage__ = next; | ||
return fn.apply(binding, args); | ||
} finally { | ||
__storage__ = previous; | ||
get(): T | undefined { | ||
return Storage.get(this); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,71 @@ | ||
import type { AsyncContext } from "./index"; | ||
|
||
/** | ||
* Stores all AsyncContext data, and tracks whether any snapshots have been | ||
* taken of the current data. | ||
*/ | ||
export class Mapping { | ||
#data: Map<AsyncContext<unknown>, unknown> | null; | ||
|
||
/** | ||
* If a snapshot of this data is taken, then further modifications cannot be | ||
* made directly. Instead, set/delete will clone this Mapping and modify | ||
* _that_ instance. | ||
*/ | ||
#frozen: boolean; | ||
|
||
constructor(data: Map<AsyncContext<unknown>, unknown> | null) { | ||
this.#data = data; | ||
this.#frozen = data === null; | ||
} | ||
|
||
has<T>(key: AsyncContext<T>): boolean { | ||
return this.#data?.has(key) || false; | ||
} | ||
|
||
get<T>(key: AsyncContext<T>): T | undefined { | ||
return this.#data?.get(key) as T | undefined; | ||
} | ||
|
||
/** | ||
* Like the standard Map.p.set, except that we will allocate a new Mapping | ||
* instance if this instance is frozen. | ||
*/ | ||
set<T>(key: AsyncContext<T>, value: T): Mapping { | ||
const mapping = this.#fork(); | ||
mapping.#data!.set(key, value); | ||
return mapping; | ||
} | ||
|
||
/** | ||
* Like the standard Map.p.delete, except that we will allocate a new Mapping | ||
* instance if this instance is frozen. | ||
*/ | ||
delete<T>(key: AsyncContext<T>): Mapping { | ||
const mapping = this.#fork(); | ||
mapping.#data!.delete(key); | ||
return mapping; | ||
} | ||
|
||
/** | ||
* Prevents further modifications to this Mapping. | ||
*/ | ||
freeze(): void { | ||
this.#frozen = true; | ||
} | ||
|
||
isFrozen(): boolean { | ||
return this.#frozen; | ||
} | ||
|
||
/** | ||
* We only need to fork if the Mapping is frozen (someone has a snapshot of | ||
* the current data), else we can just modify our data directly. | ||
*/ | ||
#fork(): Mapping { | ||
if (this.#frozen) { | ||
return new Mapping(new Map(this.#data)); | ||
} | ||
return this; | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,79 @@ | ||
import { Mapping } from "./mapping"; | ||
import { FrozenFork, OwnedFork } from "./fork"; | ||
import type { AsyncContext } from "./index"; | ||
|
||
/** | ||
* Storage is the (internal to the language) storage container of all | ||
* AsyncContext data. | ||
* | ||
* None of the methods here are exposed to users, they're only exposed to the AsyncContext class. | ||
*/ | ||
export class Storage { | ||
static #current: Mapping = new Mapping(null); | ||
|
||
/** | ||
* Get retrieves the current value assigned to the AsyncContext. | ||
*/ | ||
static get<T>(key: AsyncContext<T>): T | undefined { | ||
return this.#current.get(key); | ||
} | ||
|
||
/** | ||
* Set assigns a new value to the AsyncContext. | ||
*/ | ||
static set<T>(key: AsyncContext<T>, value: T): void { | ||
// If the Mappings are frozen (someone has snapshot it), then modifying the | ||
// mappings will return a clone containing the modification. | ||
this.#current = this.#current.set(key, value); | ||
} | ||
|
||
/** | ||
* Fork is called before modifying the global storage state (either by | ||
* replacing the current mappings or assigning a new value to an individual | ||
* AsyncContext). | ||
* | ||
* The Fork instance returned will be able to restore the mappings to the | ||
* unmodified state. | ||
*/ | ||
static fork<T>(key: AsyncContext<T>): FrozenFork | OwnedFork<T> { | ||
const current = this.#current; | ||
if (current.isFrozen()) { | ||
return new FrozenFork(current); | ||
} | ||
return new OwnedFork(current, key); | ||
} | ||
|
||
/** | ||
* Join will restore the global storage state to state at the time of the | ||
* fork. | ||
*/ | ||
static join<T>(fork: FrozenFork | OwnedFork<T>): void { | ||
this.#current = fork.join(this.#current); | ||
} | ||
|
||
/** | ||
* Snapshot freezes the current storage state, and returns a new fork which | ||
* can restore the global storage state to the state at the time of the | ||
* snapshot. | ||
*/ | ||
static snapshot(): FrozenFork { | ||
this.#current?.freeze(); | ||
jridgewell marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return new FrozenFork(this.#current); | ||
} | ||
|
||
/** | ||
* Restore restores the global storage state to the state at the time of the | ||
* snapshot. | ||
*/ | ||
static restore(snapshot: FrozenFork): FrozenFork { | ||
const previous = this.#current; | ||
this.#current = snapshot.join(previous); | ||
|
||
// Technically, previous may not be frozen. But we know its state cannot | ||
// change, because the only way to modify it is to restore it to the | ||
// Storage container, and the only way to do that is to have snapshot it. | ||
// So it's either snapshot (and frozen), or it's not and thus cannot be | ||
// modified. | ||
return new FrozenFork(previous); | ||
} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this method can be merged with
Storage.restore
. They all restore the global storage state to state at the time of the fork.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not quite.
join
andrestore
(nowrestore
andswitch
respectively) have different assumptions about what can happen to the current global state.For
join
, we assume the the current mappings will be modified (or if it's frozen, reallocated then modified). Forrestore
, we want to switch back to the state of a wrap's snapshot and return aFrozenFork
so the wrapper can restore the prior state after running. It wouldn't be valid forjoin
to return a fork.