domenic / promises-unwrapping

The ES6 promises spec, as per September 2013 TC39 meeting
1.23k stars 94 forks source link

Lazy create Derived list at end of PropagateToDerived #57

Closed junosuarez closed 10 years ago

junosuarez commented 10 years ago

The last step in PropagateToDerived says Set the value of p's [[Derived]] internal data property to a new empty List.

While implementing this in Obj-C it felt more natural (and efficient) to lazily create the collection object in a getter, especially when creating Promises which may never have downstream consumers. Rather than specifying that the new list be created in PropagateToDerived, perhaps specify that the existing list should be deleted or emptied.

domenic commented 10 years ago

Does this have any observable impact?

junosuarez commented 10 years ago

No - if another derived promise were added to an already resolved promise, an implementation would then have to ensure that the [[Derived]] List exists and then contains as its first element the Derived promise.

In JavaScript, it's the difference between ending with derived = [] and derived = undefined. Let derived be a property where:

{
  get: function () {
    return _derived || (_derived = [])
  },
  set: function (val) {
    return _derived = val
  }
}

It frees up the reference to the list data structure, allowing it to be garbage collected or deallocated until it is actually needed again in the future. In my view, the spec should allow but not require this behavior as a transparent implementation detail.

domenic commented 10 years ago

If it has no observable impact, then it is outside the scope of the spec. Implementations are allowed to do whatever they want as long as their behavior is observably the same as the spec---that is how optimizations work, for example :).

junosuarez commented 10 years ago

Fair enough - but since it's not observable, why is a particular implementation indicated by the spec? Why not use more generic language?

domenic commented 10 years ago

BTW the reason I set it to a new empty List instead of clearing it somehow is because the former seemed doable in ES spec language, whereas the latter did not. (E.g. the [[MapData]] list "clears" itself by looping through all its elements and setting them to "empty".)

In the end I think this line isn't even necessary, as [[Derived]] will never be touched again after this. Perhaps I should remove this line? The only reason I kept it was because otherwise a naive implementation might not clean it up, and this would leave references to the derived tuples in memory (which would be a memory leak).

@allenwb, your thoughts welcomed on whether this kind of unnnecessary cleanup belongs in the spec or not.

domenic commented 10 years ago

Closed in favor of #59.