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

Switch from heap/stack to just a heap #1069

Merged
merged 1 commit into from
Nov 30, 2018

Conversation

alexcrichton
Copy link
Contributor

This commit switches strategies for storing JsValue from a heap/stack
to just one heap. This mirrors the new strategy for JsValue storage
in #1002 and should make multiplexing those strategies at
wasm-bindgen-time much easier.

Instead of having one array which acts as a stack for borrowed values
and one array for a heap of borrowed values, only one JS array is used
for storage of JS values now. This makes getObject far simpler by
simply being an array access, but it means that cloning an object now
reserves a new slot instead of reference counting it. If the old
reference counting behavior is needed it's thought that Rc<JsValue>
can be used in Rust.

The new "heap" has an initial stack pointer which grows downwards, and a
heap which grows upwards. The heap is a singly-linked-list which is
allocated/deallocated from. The stack grows downwards to zero and
presumably starts generating errors once it underflows. An initial stack
size of 32 is chosen as that should encompass all use cases today, but
we can eventually probably add configuration for this!

Note that the heap is initialized to all null for the stack and then
the initial JS values (undefined, null, true, false) are pushed
onto the heap in reserved locations.

Copy link
Member

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

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

Nice!

We should have a comment somewhere describing the memory layout of this array and how the heap grows towards the wilderness and the stack grows down.

crates/cli-support/src/js/mod.rs Outdated Show resolved Hide resolved
@alexcrichton alexcrichton force-pushed the rejigger-stack branch 3 times, most recently from f5ff65e to c603673 Compare November 30, 2018 20:06
This commit switches strategies for storing `JsValue` from a heap/stack
to just one heap. This mirrors the new strategy for `JsValue` storage
in rustwasm#1002 and should make multiplexing those strategies at
`wasm-bindgen`-time much easier.

Instead of having one array which acts as a stack for borrowed values
and one array for a heap of borrowed values, only one JS array is used
for storage of JS values now. This makes `getObject` far simpler by
simply being an array access, but it means that cloning an object now
reserves a new slot instead of reference counting it. If the old
reference counting behavior is needed it's thought that `Rc<JsValue>`
can be used in Rust.

The new "heap" has an initial stack pointer which grows downwards, and a
heap which grows upwards. The heap is a singly-linked-list which is
allocated/deallocated from. The stack grows downwards to zero and
presumably starts generating errors once it underflows. An initial stack
size of 32 is chosen as that should encompass all use cases today, but
we can eventually probably add configuration for this!

Note that the heap is initialized to all `null` for the stack and then
the initial JS values (`undefined`, `null`, `true`, `false`) are pushed
onto the heap in reserved locations.
@alexcrichton
Copy link
Contributor Author

We should have a comment somewhere describing the memory layout of this array and how the heap grows towards the wilderness and the stack grows down.

Oops right! I've updated the internal contributing documentation to reflect this change

@alexcrichton alexcrichton merged commit 13d9e47 into rustwasm:master Nov 30, 2018
@alexcrichton alexcrichton deleted the rejigger-stack branch November 30, 2018 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants