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

Update proposal to include {mem,table}.drop #85

Merged
merged 4 commits into from
Jan 10, 2018
Merged

Update proposal to include {mem,table}.drop #85

merged 4 commits into from
Jan 10, 2018

Conversation

binji
Copy link
Member

@binji binji commented Dec 18, 2017

No description provided.

@binji
Copy link
Member Author

binji commented Dec 18, 2017

See #62 (comment).

@binji binji requested a review from lars-t-hansen December 18, 2017 21:46
as an operand to `mem.init` or `table.init`.

Segments (either active or inactive) can also be discarded by using the
following new instructions:

Choose a reason for hiding this comment

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

Gosh, I'm not sure if it's a good idea to be able to drop active memories. Suppose you can, and do. Then you instantiate the module again. What happens then? A trap? (That would be the interpretation of the prose further down.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I wasn't sure about this one. I was thinking it would be a nice optimization to be able to discard to active segments (say, if you know that there will never be another instance), but perhaps we should be more conservative.

Copy link
Member

Choose a reason for hiding this comment

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

I'd err on the side of conservative.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, changed.


Note that it is allowed to use `init_memory` on the same data segment more than
Note that it is allowed to use `mem.init` on the same data segment more than
once, or with an active data segment.

Choose a reason for hiding this comment

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

I think it's a mistake to allow mem.init to be used with an active data segment. The current contract is that the active data segment may be dropped by the implementation once the Module object becomes GC-able. This makes mem.init sensitive to GC timing, because it'll have to trap if the data segment has been removed.

IMO all the programmatic mem opcodes should only be usable with the inactive data segments.

(I also have decided I dislike the terminology "active" and "inactive", but I've been at a loss to propose something better. Still am, really. But "instantiation-time" vs "run-time" would capture the distinction better.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I think you're right. I'll try to find some time to update before the end of the year... (at home with the family 😄).

@binji
Copy link
Member Author

binji commented Dec 25, 2017

I've renamed inactive to passive and also made it a validation error to use *.init or *.drop with active segments.

@binji
Copy link
Member Author

binji commented Jan 10, 2018

Landing for now, we can iterate in further PRs.

@binji binji merged commit 7ab0075 into master Jan 10, 2018
@binji binji deleted the cond-seg-drop branch January 10, 2018 19:30
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.

3 participants