Skip to content
This repository has been archived by the owner on Jan 25, 2022. It is now read-only.

Reviews: Second round #105

Closed
lars-t-hansen opened this issue Jul 11, 2016 · 14 comments
Closed

Reviews: Second round #105

lars-t-hansen opened this issue Jul 11, 2016 · 14 comments
Milestone

Comments

@lars-t-hansen
Copy link
Collaborator

@littledan @bterlson @pizlonator @waldemarhorwat @domenic

I have updated the draft spec and I am attending the meeting in Redmond in two weeks to ask to have it moved to Stage 3. As per the TC39 process, this is the reviewers' notice of the start of the designated two-week review period. (I'll send you all email too.)

For the spec to move to Stage 3, the reviewers as well as the Editor must sign off on it. Any reviewers not traveling to the meeting should probably add a note to this ticket to indicate their position on the matter.

Relative to the earlier specs, the changes here are:

  • The memory model, rewritten again. I like it better (and I hope you will too) but once again it needs serious scrutiny.
  • The section on agents has been pared down to fit in with the existing PR 522 on the TC39 spec.
  • The section on browser tie-ins has been cut significantly and only discusses how this spec fits into current web technology, not how web technology ought to evolve.
  • The formalization of Atomics.wait / Atomics.wake has been clarified.
  • Minor bugfixes and changes in wording many places.

I'm grateful for every and all comments, and I'll be especially grateful if serious objections reach me before the Redmond meeting so that I will have a chance to address them.

@lars-t-hansen lars-t-hansen added this to the Stage 3 milestone Jul 11, 2016
@domenic
Copy link
Member

domenic commented Jul 12, 2016

I did a quick pass-through. My main comments are that this spec has a lot of editorial work to do to align with existing ES spec text, and that makes it a bit hard to review since it's easy to get distracted by all the trivial errors. I tried to file as many as I can.

@lars-t-hansen
Copy link
Collaborator Author

An update has been posted addressing all the open editorial concerns.

Among the larger changes:

  • As the old-style ReturnIfAbrupt steps were removed, many algorithms have renumbered steps
  • The Shared Data Block ID concept was removed, thus removing that section and making referents refer to Shared Data Blocks instead

@pizlonator
Copy link

Nice work! Thank you for your hard work on this!

I'm happy with this document in its current form.

-Filip

On Jul 11, 2016, at 11:19 AM, Lars T Hansen [email protected] wrote:

@littledan https://github.com/littledan @bterlson https://github.com/bterlson @pizlonator https://github.com/pizlonator @waldemarhorwat https://github.com/waldemarhorwat @domenic https://github.com/domenic
I have updated the draft spec https://tc39.github.io/ecmascript_sharedmem/shmem.html and I am attending the meeting in Redmond in two weeks to ask to have it moved to Stage 3. As per the TC39 process, this is the reviewers' notice of the start of the designated two-week review period. (I'll send you all email too.)

For the spec to move to Stage 3, the reviewers as well as the Editor must sign off on it. Any reviewers not traveling to the meeting should probably add a note to this ticket to indicate their position on the matter.

Relative to the earlier specs, the changes here are:

The memory model, rewritten again. I like it better (and I hope you will too) but once again it needs serious scrutiny.
The section on agents has been pared down to fit in with the existing PR 522 on the TC39 spec.
The section on browser tie-ins has been cut significantly and only discusses how this spec fits into current web technology, not how web technology ought to evolve.
The formalization of Atomics.wait / Atomics.wake has been clarified.
Minor bugfixes and changes in wording many places.
I'm grateful for every and all comments, and I'll be especially grateful if serious objections reach me before the Redmond meeting so that I will have a chance to address them.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub #105, or mute the thread https://github.com/notifications/unsubscribe/ALnOO69bdfsp--ccKUcbwfiQ2k9YyE2-ks5qUokYgaJpZM4JJozo.

@lars-t-hansen
Copy link
Collaborator Author

@pizlonator, thank you!

Everyone, the formatted draft has been updated with an editorial fix for Issue #120.

@domenic
Copy link
Member

domenic commented Jul 20, 2016

I want to give my LGTM. There are some other minor editorial issues about consistifying with 262 but I think those will best be handled by @bterlson during a merge. (Unless he wants to handle them here.)

@lars-t-hansen
Copy link
Collaborator Author

@domenic, thank you!

@bterlson
Copy link
Member

bterlson commented Jul 20, 2016

Also LGTM. Further tweaks will probably be required for integration into 262 but this seems like a great start that will be easy to make a PR from. Also I filed #121 with some editorial fixes.

@littledan
Copy link
Member

LGTM for pedantic spec things, but I have not evaluated the memory model in detail.

@lars-t-hansen
Copy link
Collaborator Author

@bterlson @littledan, thank you!

@lars-t-hansen
Copy link
Collaborator Author

Everyone, the draft has been updated:

@lars-t-hansen
Copy link
Collaborator Author

The draft and DISCUSSION.md have been updated with sundry clarifications in response to feedback from @jfbastien on the memory model. No semantic changes.

@jfbastien
Copy link
Contributor

As discussed in #88 we've now secured funding to develop a formal memory model. I filed #133 for details and to track progress. I think that this second round of reviews, and the decision of whether or not to move to Stage 3, should take this research effort into account: most flaws in the memory model should be found through mathematizing it.

@lars-t-hansen
Copy link
Collaborator Author

@waldemarhorwat found some bugs in the proposed memory model. Most importantly, the synchronization order is not well-defined because "viable" atomics are not well-defined in the current draft, see #134; and without a well-defined synchronization order we don't have a workable model. I believe this is fixable but it's not a simple edit, it will require rethinking the concept once again. Anyway, in light of those developments, the proposal is at least arguably not ready for stage 3: It would be incorrect to say that the spec text is "complete", and I would not expect Waldemar to sign off on the current draft.

@lars-t-hansen
Copy link
Collaborator Author

At the meeting in Redmond the committee agreed to an API freeze for this proposal (which I take to mean: no globals or properties removed; no names changed; no non-optional parameters to be introduced; no breaking semantic changes), which together with the four LGTMs here takes the proposal to "Stage 2.95" or so. We still need credible prose for the memory model, but all other matters are now settled for Stage 3.

The plan of record for the memory model prose is to have a new draft ready for review by Waldemar by EOD Monday, 5 September (Labor Day in the US), with an eye toward rapidly iterating toward something acceptable by the September meeting in California.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants