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

What is the new API? #55

Closed
littledan opened this issue Dec 5, 2018 · 14 comments
Closed

What is the new API? #55

littledan opened this issue Dec 5, 2018 · 14 comments

Comments

@littledan
Copy link
Member

littledan commented Dec 5, 2018

I see some notes about a new API for WeakRefs at #54 , but it's somewhat underspecified. Is someone planning on writing up the new API in detail? cc @tschneidereit

@tschneidereit
Copy link
Member

I plan on writing this up, yes. I have a polyfill mostly done, but might not get around to finishing it and writing an explainer until the end of the year: I'm at a company all hands this week, and on PTO the next two.

I'll at least give a high-level overview here. In short, we decided to decouple weak references from finalization. They are related but distinct capabilities, and there are use cases for either of them in isolation in addition to ones that need both.

API

The API we arrived at in the breakout session is very simple:

class WeakRef {
  constructor(target: Object);
  deref(): Object;
}

class FinalizationGroup {
  constructor(cleanupCallback: (items: Iterator) => void);
  register(target: Object, holdings: any, unregisterToken?: any): void;
  unregister(unregisterToken: any): boolean;
  cleanupSome(cleanupCallback: (items: Iterator) => void);
}

Basic usage examples:

WeakRefs:

let target = {};
let ref = new WeakRef(target);
ref.deref() === target;

Finalization:

let target1 = {};
let holdings1 = 1;
let target2 = {};
let holdings2 = {};

function cleanup(items) {
  for (let holding of items) {
    // Perform cleanup action based on the associated holdings.
  }
}

let token = {};
let group = new FinalizationGroup(cleanup);
group.register(target1, holdings1, token);
group.register(target2, holdings2, token);

// Unregisters both target1 and target2:
group.unregister(token) === true;

@erights
Copy link
Contributor

erights commented Dec 5, 2018

Not quite. We agreed that WeakRef can only be a class-like constructor function if it deviates from the normal class pattern: the .constructor property of WeakRef.prototype can only appear in appendix B, i.e., an initial state in which it is omitted[1] is still conformant. A correct program may not rely on WeakRef.prototype.constructor === WeakRef. If we cannot agree on this we must use a WeakRef factory function instead.

Both WeakRef and FinalizationGroup must only appear on the System object, or some other acceptable means of quarantining magic powers (such as @ljharb 's reflective proposal, which we need to write down.)

[1] Rather than omitting it, a better option may be to include it with value undefined. But this would make it something other than simply a creature of appendix B. We'd need some other way to talk about normative optional.

@tschneidereit
Copy link
Member

I didn't include these parts of our conclusions because, if we can make them work in the way we discussed, they should lead to the API as described above for the vast majority of use cases.

I should've at least mentioned that there are important aspects to our conclusions that aren't immediately reflected in the API, but are relevant in some important scenarios—and have meaningful impact on the spec.

@erights
Copy link
Contributor

erights commented Dec 5, 2018

Hi @tschneidereit I think we're converging nicely. But to be pedantic (this is standards work after all ;) ), your phrasing:

they should lead to the API as described above for the vast majority of use cases

compromises the notion of program correctness too far. Even if most hosts implement WeakRef.prototype.constructor === WeakRef, a correct host-independent JavaScript program cannot rely on it. Testing frameworks for host-independent JavaScript should take this into account. This makes the "appendix B or not" question more than just a political fiction. It also means that the parts of Appendix B that are specific to sloppy mode, or are harmless and universally implemented (I posted a list somewhere) should be moved into the main text so that correct programs can count on them, and testing frameworks can reduce their case explosion.

OTOH, if we go with @ljharb 's alternative to System, then your phrase above would apply to the System issue: Most users would not need to notice the category distinction. I'm not sure if this is good or bad, but it would be true. If we go instead with @codehag 's notion of quarantined built-in resource modules, the degree to which "normal" users are aware of the distinction would be somewhere in the middle.

@zenparsing
Copy link
Member

I very much like the direction this API is taking.

The register/unregister part reminds me "subscribe" type APIs which typically return a capability that can be used to unsubscribe (e.g. setTimeout, Observable, etc).

let group = new FinalizationGroup(cleanup);
let list = [];

list.push(group.register(target1, holdings1));
list.push(group.register(target2, holdings2));

// Unregisters both target1 and target2:
list.forEach(token => group.unregister(token));

// Or maybe register just returns a function
list.forEach(unreg => unreg());

@tschneidereit
Copy link
Member

The register/unregister part reminds me "subscribe" type APIs which typically return a capability that can be used to unsubscribe (e.g. setTimeout, Observable, etc).

There are three reasons we didn't go with this design:

  1. It means we have to decide what the type of the token must be. There are use cases that'd want to have an unfalsifiable token—i.e. and object—, and others where you want to have a primitive. An example for the latter would be WebAssembly, where you might want to be able to store the token on the Wasm heap, so it has to be numeric.
  2. It always creates the capability, whether you need it or not. This can be optimized away in many cases, but not always, and is not ideal.
  3. It doesn't allow you to reuse the same token. It's possible that this isn't as important, because groups might already be the mechanism for, well, grouping things, but it's at least nice to have the ability.

@allenwb
Copy link
Member

allenwb commented Dec 5, 2018

@erights

Pardon me if I'm missing something by popping into the middle of this discussion, but I don't see why Annex B has to be involved in order to excluded the constructor property from WeakRef.prototype. We already dealt with a situation like this in ES6 when we decide that we did not want the unique prototype that each generator function has for their instances to include the constructor capability (ie, property) that would allow navigation from generator instances back to the generator function that could be used to create additional instances of that generator. See 25.2.4.3 and the Figure 2 diagram in the original ES6 spec (I just discovered that the corresponding diagram in the current edition was broken at some point when it was redrafted).

I don't see why a similar restriction could not be normatively specified in a similar manner for WeakRef.prototype.

I don't think it is relevant that you are explaining WeakRef as if it was defined using a class definition. The spec. does use class definitions internally to describe built-in object structures and is not constrained to exactly mimimic always the structures created by them. If you want to clarify this in an explainer (or even as part of an implementation) you can just do something like this as the WeakRef definition:

class WeakRef {
   constructor (target) { ...}
   deref() {...)
}
delete WeakRef.prototype.constructor;

@erights
Copy link
Contributor

erights commented Dec 5, 2018

Hi @allenwb thanks! That would be a much better solution. The other was a reluctant compromise.

@zenparsing
Copy link
Member

An example for the latter would be WebAssembly, where you might want to be able to store the token on the Wasm heap, so it has to be numeric.

With the more "subscribe"-like design, I'm guessing the workaround for this use case would be to have a map on the JS side mapping numbers into "unregisterables"? (I'm still a little fuzzy about the interaction model.)

guybedford pushed a commit to guybedford/v8 that referenced this issue Dec 20, 2018
New API is here: tc39/proposal-weakrefs#55

The WeakCell parts stay in the old API, resulting in temporary code duplication
in some parts. Those parts will go away once the WeakCell-related parts are
migrated to the new API (but the spec needs some work first).

BUG=v8:8179

Change-Id: I81ca824a14d830e3c5fa515d5ad7e5f78c10e19d
Reviewed-on: https://chromium-review.googlesource.com/c/1378171
Commit-Queue: Marja Hölttä <marja@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Sathya Gunasekaran <gsathya@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58264}
@Jamesernator
Copy link

Jamesernator commented Jan 5, 2019

I've currently been playing around with NAPI and decided to try writing a polyfill but I wasn't quite clear on what .cleanupSome is supposed to do. Is it simply meant to call the cleanupCallback with a cleanup-iterator if there are remaining holdings that are unprocessed? Or does it also call the callback that was passed into the constructor of the FinalizationGroup if not all get consumed?

For people interested, currently the polyfill doesn't support .unregister or the key thing because of the discussion in #56, also if anyone understands Napi::External finalizers that'd help to remove the polling from the FinalizationGroup.

To include the polyfill just do require("@jamesernator/weakref/polyfill.js") or if you don't want globals const { WeakRef, FinalizationGroup } = require('@jamesernator/weakref').

UPDATE: Managed to adapt @addaleax's excellent work with finalizers from this library so the polyfill now collects via the garbage collector.

littledan added a commit to littledan/proposal-weakrefs that referenced this issue Feb 5, 2019
Based on tc39#55 (comment)

```js
let token = {};
let group = new FinalizationGroup(cleanup);
group.register(target1, holdings1, token);
group.register(target2, holdings2, token);

// Unregisters both target1 and target2:
group.unregister(token) === true;
```

Previously, only target1 would be unregistered.
@littledan
Copy link
Member Author

When are the finalization callbacks supposed to be implicitly called? My understanding of past discussions was, "after the job queue empties", which I tried to write up in this patch. Is this what was intended with this API revision, or are there further thoughts? Another option would be to permit finalizers to be called interspersed with Promise jobs. cc @domenic

@tschneidereit
Copy link
Member

@littledan this API revision doesn't have any bearing on when the cleanup callback is automatically invoked. We haven't settled on the timing here for now, but @erights and I discussed this to some degree at the November meeting.

I think the overriding concern here should be to reduce risk of timing changes based on engine implementation details. That makes me lean towards not interspersing with Promise jobs.

@erights
Copy link
Contributor

erights commented Feb 6, 2019

Hi @tschneidereit , afterwards @dtribble and I went over the rationale for that. @dtribble changed my mind back. Approx: That we should specify this only in terms of jobs, not some higher granularity unit (or equivalently, not a lower priority queue).

@dtribble , when you have time, could you please restate your arguments here? Thanks.

@littledan
Copy link
Member Author

The explainer and specification text draft now reflect the new API. Let's continue discussing the granularity/timing of finalization in #57/#61.

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