tc39 / proposal-signals

A proposal to add signals to JavaScript.
MIT License
2.95k stars 54 forks source link

Signal set seems unnecessary in the watcher? #166

Open dead-claudia opened 1 month ago

dead-claudia commented 1 month ago

You can technically wire up the whole graph through just watch and unwatch.

This avoids needing to task the watcher with bookkeeping, and thus making it little more than a flyweight around its updater constructor argument. It's common to only have one watcher, and this makes that common case not any harder to handle, while allowing frameworks the flexibility of managing destruction.

And since it would only be a flyweight, you could then take it to the logical extreme and just expose Signal.subtle.{un,}watch(signal, onSet) methods.

shaylew commented 1 month ago

This sounds like it results in each watch call traversing (to pass through any Computeds and find all the States) and creating (to connect the watcher to the states) an unbounded number of edges. Does that end up paying off?

dead-claudia commented 1 month ago

@shaylew They're going to do that full traversal anyways when they're added for the first time. And how often are you likely to try watching the same signal twice with the same watcher?

shaylew commented 1 month ago

The watched/unwatched transition happens the first (last) time they're added to (removed from) any Watcher -- so I think the question is more "how often are you going to watch the same signal with multiple watchers" no?

dead-claudia commented 1 month ago

The watched/unwatched transition happens the first (last) time they're added to (removed from) any Watcher -- so I think the question is more "how often are you going to watch the same signal with multiple watchers" no?

@shaylew I think you're missing what I'm saying.

Signals have to track what watchers watch them, just so they can notify them all while avoiding duplicate notifications. This can of course be leveraged to provide lifecycle hooks as well, but that set is needed independent of that.

This issue is saying that watchers don't also need to track what signals they're watching. Deduplication within individual sources comes for free on the signal side. Thus, the watcher doesn't need to do anything special here to maintain that.

dead-claudia commented 1 month ago

Related: https://github.com/tc39/proposal-signals/issues/178

btakita commented 1 month ago

If watch is overloaded to accept a WeakRef<Signal>, unwatch wouldn't be necessary in those cases. See https://github.com/tc39/proposal-signals/issues/156

dead-claudia commented 1 month ago

@btakita That's orthogonal to this. Also, it's not redundant - there's no guarantee GC will ever run: on some embedded runtimes and in certain security-critical server environments, WeakRefs hold strong data references and FinalizationRegistry callbacks are never invoked.

btakita commented 1 month ago

@dead-claudia Are you saying that GC doesn't run on some environments? Do these environments have infinite RAM or it a short lived process? Sorry about my ignorance on this subject. I have not encountered such requirements & have not found any documented cases of these requirements.

dead-claudia commented 1 month ago

@btakita It's not that GC doesn't run, but that they don't let GC be observable.

Edit: https://github.com/endojs/endo is one such runtime environment that does this, as it by design removes all sources of non-determinism for security reasons (one of which is to categorically prevent timing attacks).

btakita commented 1 month ago

@dead-claudia I searched for a runtime that has the characteristics that you describe & could not find anything. Could you point me to which runtimes have a WeakRef act as a strong ref?

I'm asking because someone inevitably quotes the vague warnings about using WeakRef found in MDN or the spec. But I have yet to see any actual examples or cases to which it would be a problom.

Let's say using WeakRef is a problem. How common is the problem? If it's a problem, unwatch can be used. Or

wr.deref = ()=>undefined

My suspicion is that the extreme majority of cases have WeakRef acting as a weak reference. And runtimes with Garbage Collection.

If the js runtime has to run in a constrained environment, is JS the optimal choice? Or would a language with safe & explicit memory management, like Rust, be a better choice?


Edit: IMO, complexity is the far bigger & realized boogie man. Than a runtime environment which I have yet to come across in over 20 years of working with JS. Complexity is common while a niche runtime is uncommon.

dead-claudia commented 1 month ago

@btakita I linked to one environment that should stub it out (it's a low-severity security bug if it doesn't).

But also, V8 frequently takes a while in my experience to collect garbage. It's never immediate unless it was just allocated in the nursery. It's usually around 10 to 100ms, but I've seen it take over a full second to collect an object before while benchmarking libraries - its collector gets very lazy in freeing memory under high allocation load.