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

A more minimal version #8

Closed
Jack-Works opened this issue Mar 17, 2022 · 2 comments · Fixed by #12
Closed

A more minimal version #8

Jack-Works opened this issue Mar 17, 2022 · 2 comments · Fixed by #12

Comments

@Jack-Works
Copy link
Member

TLDR:

We don't add signals into the language.

Details:

Add an internal slot:

[[RevocationSignal]]: Record { revoked: boolean, callbacks: list of Abstract Closure }

These callbacks should be synchronously invoked.

  1. Host object AbortSignal is having this slot.
  2. new Proxy:
    1. Let signal be options?.signal
    2. If signal does not have a [[RevocationSignal]] internal slot, throw a TypeError.
    3. Let revocable be %Proxy.revocable%(target, handler)
    4. Let revokeFunction be revocable.revoke
    5. Let closure be a new Abstract Closure that captures revokeFunction with the following steps:
      1. Call revokeFunction()
    6. Push closure to signal.[[RevocationSignal]].callbacks
    7. Return revocable.proxy
@ajvincent
Copy link
Collaborator

ajvincent commented Mar 17, 2022

I have questions. Also, the recording of today's SES meeting isn't up yet, so I might mis-remember this. Please be understanding that I lack some understanding / context myself as I write this.

  • I assume you're referring to sections 10.5.14 and 28.2 of ECMA-262, which are the ProxyCreate() and Proxy sections of the specification. Which parts does the above fit into?
  • Why is calling Proxy.revocable() internally desirable here?
    • Particularly since an array of revoker functions (each of which holds a strong reference to the proxy in the internal slot, which is a separate memory-leak issue I've discussed elsewhere) is about as expensive as the current proxy model
    • Is there a better data structure that we can use (an array of weak pointers to proxies, or alternatively a slot in each Proxy, referencing the controller) that can provide the access we need?
  • Did we actually resolve issue Should new Proxy get a third argument as well? #6 during the meeting, and I just missed it?
  • Defining signal as options?.signal implies signal could be null or undefined, which would result in the TypeError on the next step. Was this intended?
  • If we did these steps, would it not be better to extract the steps for creating the abstract closure in 28.2.2.1 as a separate procedure?
  • What about the objection that adding AbortSignal from the Document Object Model risks importing large parts of DOM into ECMAScript? AbortSignal inherits from EventTarget, has timeout options and has an onabort() event handler attribute, for starters.

https://tc39.es/ecma262/#sec-proxycreate 10.5.14
https://tc39.es/ecma262/#sec-proxy-objects 28.2

@Jack-Works
Copy link
Member Author

API shape is not important here. My given spec is very informal, just used to express the idea of, we can use the AbortSignal in the language even we don't add it into the language.

The core idea here is to define a "protocol" to take the host AbortSignal for our usage. It is a bit like [[IsHTMLDDA]], no object defined in the language has this field, but the host can define one.

Once AbortSignal has the [[RevocationSignal]] field we need, it can be used as the signal we want in this proposal.

Particularly since an array of revoker functions (each of which holds a strong reference to the proxy in the internal slot, which is a separate memory-leak issue I've discussed elsewhere) is about as expensive as the current proxy model

If the optimized version (a flag in the memory) does not have an observable difference, the engine can optimize it.

An example of this is Map.prototype.has: https://tc39.es/ecma262/multipage/keyed-collections.html#sec-map.prototype.has

It use a O(N) algrithm to check if an element is present in the map, but since it does not have an observable difference (other than the performance), most engine will use a hash map instead. The spec is just make it easier to read.

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 a pull request may close this issue.

2 participants