-
Notifications
You must be signed in to change notification settings - Fork 7
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
Use of registered symbols would allow observation that a WeakMap
entry has been freed
#21
Comments
The proposal will not advance with that restriction; either all symbols or none can be WeakMap keys. I also don't see the issue here. A registered symbol can, in general, be "freed" when it's unobservable. However, by putting it in a WeakMap, it can never be freed until that WeakMap is - a registered symbol that is weakly held remains "live" as long as the thing holding it is live. Why is this a problem? |
In other words, if you |
The point of a |
This is a pretty strong stance. I would hope there was still a way to arrive at some kind of consensus here.
As @ljharb mentions, the key bit is not that I personally see no reason to allow forgeable values as weak keys/targets, besides allowing lazy programs to shoot themselves in the foot. |
@brad4d no, because a registry symbol can never observably be freed, ever, already. The spec fiction is there to ensure that. In other words, it would only be in an implementation that attempted to walk the tightrope between freeing them, and keeping that unobservable, that this new difficulty would emerge. |
Btw, there is another reason why registered symbols being disallowed as weak keys would make sense: implementation complexity. Since If allowing registered symbols as WeakMap keys, such an implementation would likely be forced to change their implementation and required to generate a unique identity for registered symbols, akin to forcing ropes. |
Thanks @mhofman for that succinct example. This is in fact the point WH was trying to make, I think. They truly put symbols created with Allowing non-registered Symbols is OK because they can only ever be created once, so they can be safely cleaned up when no non-WeakMap-key references to them remain. |
I don't understand why such an implementation would need to change everything, as opposed to making a special carve-out for the subset of registered symbols that are weakly held. |
Separately, do any such implementations exist that could speak to these difficulties? If not, then if the spec was designed to allow a kind of implementation, but in practice there's no concrete evidence that there's any benefit to doing so, why preserve it? |
By special carve-out, I suppose you mean branch on the type of symbol when creating a WeakRef or WeakMap entry? That feels like significant complexity added everywhere registered symbols could be used.
I'm not sure, but @erights mentioned that's how he would have expected engines to implement registered symbols. |
@mhofman if we went with your suggestion of differentiating symbols between registered/well-known and "other", they'd already have to do that - so that complexity seems to be acceptable. |
They would only have to do that when adding the entry to the weakmap / creating the weak ref, and throw a TypeError. Here you're talking about having a parallel Map / Ref implementation for registered symbols keys/targets. |
I think disallowing registered symbols is a small wart that allows for a much more valuable use case to work, and so we shouldn't block just because of that. Once implementers get actual experience with how complex symbol GC becomes, we can try reopening to allow registered symbols later on. |
@jridgewell that's an opinion i don't share. |
Additional methods that would need* to branch for registered symbols if they were allowed:
The branch being to switch the logic to the separate internal Strong{Set,Map} used for registered symbols. Not being a member of a JS engine implementation team, I will leave the measure of added complexity to others. * In contrast: If registered symbols are not allowed, the above methods would not necessarily need to branch specifically for registered symbols. The existing code path for handling symbols could be used as the result would be the same. Though implementations may still add code to specially handle registered symbols as an optimisation (early return). ** EDIT: an implementation could store the registered symbols in the usual weakMap's internal structure. But also store hold registered-symbol strongly to avoid any optimisation that would otherwise collect it. This means that the https://gist.github.com/acutmore/8f16a8d4cf4b7b54c5956bd6eab05cfd The following methods would have to check if a symbol is a registered symbols either way.
And I don't think |
Perhaps the developer experience (in the case of allowing registered symbols) could be improved with clear warnings? For example on MDN and maybe even at runtime with implementations printing a ( I'm not advocating a position, but trying to help find common ground ) |
Yes, Firefox (SpiderMonkey) collects symbols created by |
I'd love to hear a FF contributor's take on the difficulty of knowing that registered Symbols that are weakly held remain observable while the weak container is. |
I think the question was also, do any implementation currently implement registered symbols as a wrapped description value without giving them a unique identity (aka doesn't use any kind of map / hash table, and doesn't implement the spec fiction of the global registry). |
That was the first half :-) the second half is speaking to the difficulties. |
Taking a step back however, the implementation complexity, while important, shouldn't be the driving factor. I still remain confused by the motivation to allow forgeable values in weak collections. What kind of program would want to legitimately do this? How are registered symbols, or tuples/records not containing symbols, any different from forgeable primitives like string and number? If a program does have a use case for forgeable values, why can't they implement the logic themselves combining a |
One use case is that I want a maximally weak collection. I don't particularly care if any of the values are weakly or strongly held - i just want to weakly hold them when possible. The more things I can put in the WeakMap, the simpler my wrapper class can be, and the more things i can delegate to the implementation. You're totally right that I can implement this already by wrapping a Map and a WeakMap - however, this means borrowed prototype methods fail (or, if i'm clever, one set of borrowed methods fails and the other works, depending on the key type), and this also means that even non-registered non-well-known Symbols will be strongly held. In other words, I value the additional optimistic weakness that "all symbols can be weakly held" provides, and since GC is already nondeterministic, (short of specific implementation knowledge) nobody can rely on "anything that can be weakly held will be collected", so there's nothing lost by this approach. Concerns about memory leaks are inevitable because the language spec already does not force collection. The language only discusses observability - and observability is all one can rely on. Collectibility is an emergent property an implementation may choose to provide, but it is not one that language practitioners may safely ever rely on. |
Does this apply to string and numbers as well? If not, what makes symbols different? Also, what purpose would such a collection have in a program?
How so? Why can't you put them in the
As a userland abstraction, why would it be necessary? Aka, why can't you use the prototype methods of your new abstraction.
I think we should be realistic, and build APIs for real world programs and machine. By your reasoning, WeakMap and WeakRef have no place in the language in the first place. Sure, a program should not rely on garbage collection, but without garbage collection, programs would very likely run out of memory, because unlike what the spec pretends, machines do have a finite amount of memory. Memory leaks are a very real issue in programs, and they're usually pretty obscure. While I wouldn't consider adding a forgeable value to a weak collection an obscure source of leak, I'm not sure the average developer would be able to realize the problem. Preventing the obvious leak and forcing them to find a workaround if that's the behavior they actually need is what I'm after here. |
I checked a few other languages (below) to see if they offered such a collection in their standard library and they did not seem to.
|
By my reasoning, WeakMap, WeakSet, and WeakRef's place in the language is to be optimistically weak - meaning, not providing guarantees, just conveying intentions to the engine.
The average developer won't be using either registered symbols or any Weak thing, both of which are incredibly obscure.
To me, that's a potential argument against any symbols being allowed; it's not an argument against registered symbols being allowed. |
I think this proposal is unrelated to "optimistically weak" collections: I agree that it would be nice to have it in the language (I had to implement it by myself multiple times), but it's not just about registered symbols. An |
I think what you're looking for is a new type of collection, which would accept any primitive, including strings and numbers. I don't see why registered symbols should be held to a different standard.
But do other languages have a notion of symbols though? Aka non-object values which may be unique and unforgeable? (genuine question) |
I think we're overindexing on my particular use case. Regardless, I think creating a bifurcation in the category of "Symbols" is an unacceptably confusing thing to add to the language, and I remain unswayed by arguments pertaining to collectibility, which is something the language does not guarantee. |
The bifurcation needs to happen somewhere. A program which wants to be responsible and not leak memory needs to be able to know what it can put in a WeakMap without causing a leak. Requiring the program to use heuristics like calling I maintain that we need a predicate offered by the language to perform this test. That predicate needs to implement the same check that I'm advocating should prevent the addition to weak collection. |
The use cases for this proposal are for patterns where unique symbols are created and then stored in a WeakMap. Not for taking symbols from an unknown source. In what use cases do we think this concern about registered symbols would come up in practice? As someone who has been in the stressful position of debugging a slow memory leak that is happening on clients where it’s not possible to get heap snapshots - I am sympathetic to an API that could catch a leak sooner. That said I’ve not come across code that creates an ever growing set of registered symbols. Code I’ve seen uses |
Why not then design a less orthogonal API, tailored specifically to that use case? Introduce |
Hi @fstirlitz,
EDIT: realised this is not the same as the other suggestions which had the functions on a global factory not on the instance. |
@acutmore @rricard when covering the update form yesterday during the SES meeting today, something new popped with respect to this change. Basically, it is going to be very hard now to polyfill well-known symbols, considering that they are, in principle, very similar to registered symbols. I don't know if that was discussed before, but worth discussing it. |
I'm not sure why it would; none of the polyfills I know of use the symbol registry, since having |
I filed #27 to discuss this |
I've just had a discussion with WH to get a clear picture of his objection to allowing symbols created with
Symbol.for()
asWeakMap
keys.To summarize, This is the use-case that must be disallowed.
Symbol.for('name')
gets used as aWeakMap
key.Symbol.for('name')
exist other than theWeakMap
key and the (virtual)GlobalSymbolRegistry
entry, which don't count. So, both of these are effectively freed.Symbol.for('name')
is called again and the result used to attempt lookup in the sameWeakMap
, finding nothing.The key misunderstanding many of us have had revolves around the assumption that the
GlobalSymbolRegistry
entry defined in the spec must keep theSymbol.for()
keys alive forever, making them effectively the same as well-known symbols that are stored on global objects. This is incorrect.In our conversation WH explained it like this.
So, I believe this proposal & its spec text need to be updated to disallow use of registered symbols as
WeakMap
keys.The text was updated successfully, but these errors were encountered: