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

Add proposal for skipping data segment initialization #65

Merged
merged 2 commits into from
Aug 29, 2017

Conversation

binji
Copy link
Member

@binji binji commented Aug 29, 2017

This adds a new proposal doc, with two potential solutions as discussed in #62.

initializer expression for its offset, and its raw data.

Since WebAssembly currently does not allow for multiple memories, the memory
index must be zero. We can repurpose this field as a flags field and use it to
Copy link
Member

Choose a reason for hiding this comment

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

Repurpose as in make it impossible to specify multiple memories? Or something else?

Copy link
Contributor

Choose a reason for hiding this comment

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

If it means not allowing multiple memories "forever", that doesn't seem wise. IMHO solution 1 looks to be a better approach.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry, I should make this more clear. The intention is that it becomes a flags field, but one of the bits can be used to specify that there exists a non-zero memory index, which would be a separate field. I'll add that to the proposal.

@@ -0,0 +1,135 @@
# Data segment initialization flag proposal

This page describes a proposal for providing a flag to skip data segment
Copy link
Member

Choose a reason for hiding this comment

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

Although it's not technically an issue yet b/c tables can't be shared, it would be good to keep elem and data segments symmetric and make the same change to both.

});

// worker.js
let moduleBytes = ...;
Copy link
Member

@lukewagner lukewagner Aug 29, 2017

Choose a reason for hiding this comment

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

Mostly a nit, but if the example becomes long-lived, it'd be nice if main.js would postMessage the module to the worker which the worker would receive via onmessage. This also drives home the point of "one module"

* Apply changes to element segments too
* Pass module to worker in example
* Clarify that resuing the memory/table index as flag field doesn't
  prevent multiple memories/tables
@binji
Copy link
Member Author

binji commented Aug 29, 2017

Addressed comments, PTAL

@lukewagner
Copy link
Member

Beautiful, thanks! lgtm as a proposal

@jfbastien
Copy link
Member

Yeah as a proposal this is good. I like 2 better.

@binji binji merged commit 2a16bc2 into master Aug 29, 2017
@binji binji deleted the no-data-seg-init branch August 29, 2017 20:12
(memory (export "memory") 1)

;; Some value used as a counter.
(data (i32.const 1) "\0")
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there should be i32.const 0? IIUC, addOne expects counter to be at 0.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops, thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

@binji fixed #66 )

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.

5 participants