| 00:28 | <shu> | as it turns out, it is super hard to use weakrefs correctly, whoda thought |
| 00:45 | <devsnek> | whatcha' weakref'n |
| 00:47 | <drousso_> | really? |
| 00:48 | <drousso_> | i'd love to know more |
| 00:48 | <drousso_> | i have plans/hopes to use them quite extensively for certain things |
| 00:48 | <shu> | i was writing an example for a talk which wraps event listeners in weakrefs so you don't have to manually unregister event listeners |
| 00:48 | <shu> | since that's a super common way to leak memory |
| 00:49 | <shu> | my example had a class like so: |
| 00:49 | <devsnek> | i don't think i've ever heard of memory being leaked via event listeners |
| 00:49 | <devsnek> | except code that accidentally attaches them in a loop |
| 00:52 | <shu> | https://gist.github.com/syg/13d44f2b1a36095a90f722a1b84e6da2 |
| 00:53 | <shu> | if you forget to unregister an event listener when you close a panel, for instance, it tends to leak everything the listener held onto |
| 00:54 | <shu> | it is my impression that's a common gotcha in frontend programming |
| 00:54 | <shu> | oops, that gist has a bug |
| 00:55 | <shu> | reload please |
| 00:55 | <shu> | drousso_: can you spot the bug? `this.listener` isn't collectable in V8's GC |
| 00:55 | <shu> | and i reckon in JSC's GC as well |
| 00:56 | <drousso_> | shu that's actually one of the primary use cases i'd want 😅 |
| 00:56 | <drousso_> | Web Inspector has it's own event listener system which keeps things alive |
| 00:56 | <shu> | drousso_: yep! |
| 00:56 | <drousso_> | using a `WeakRef` to hold the `thisObject` would likely eliminate that issue |
| 00:56 | <shu> | there're plenty of google JS code that has some notion of "observer" that gets misused |
| 00:56 | <shu> | and leaks craptons |
| 00:57 | <devsnek> | the listener function isn't collectable? |
| 00:57 | <shu> | nope |
| 00:57 | <drousso_> | shu is the issue that `this` is captured by the arrow function? |
| 00:57 | <shu> | it's for a really shitty subtle reason |
| 00:57 | <shu> | drousso_: bingo, specifically, this chain: |
| 00:57 | <shu> | - this.listener captures `this` |
| 00:57 | <drousso_> | ya |
| 00:57 | <devsnek> | but `this` is the only thing that holds it |
| 00:57 | <devsnek> | isn't that a normal cycle |
| 00:57 | <shu> | - the wrapper arrow function is held onto strongly by the holdings of the FR |
| 00:58 | <drousso_> | shu i agree that that's a pretty massive footgun |
| 00:58 | <shu> | the wrapper arrow holds alive the environment of the function |
| 00:58 | <devsnek> | oh the other arrow function |
| 00:58 | <shu> | the environment has references to two things: the `this`, and `weakRef` |
| 00:58 | <devsnek> | y'all should not capture `this` if nothing inside references it |
| 00:58 | <drousso_> | shu if you're curious, i prototyped a patch for Web Inspector's event listener system to use `WeakRef` <https://webkit.org/b/196956>; |
| 00:59 | <shu> | which means the FR can reach the `this` in the environment, and thus `this.listener`, making `this.listener` held alive by the FR |
| 00:59 | <drousso_> | the way i did it was just to manually change it so that we never used an arrow function :P |
| 00:59 | <shu> | devsnek: V8 tried a patch to split the environment so that there are different environments for different inner closures |
| 00:59 | <Bakkot> | off |
| 00:59 | <drousso_> | the majority of Web Inspector's event listener usage is bound to a prototype function anyways (i.e. non-local non-anonymous functions) so it's not that bad IMO |
| 00:59 | <Bakkot> | *oof |
| 00:59 | <shu> | e.g., in that example, the first arrow only captures `this`, but the second arrow only captures `weakRef`, so they shouldn't both capture the *same* environment that has both things alive |
| 00:59 | <shu> | that patch ended up causing memory regressions and couldn't land |
| 01:00 | <shu> | so there is no easy answer atm |
| 01:00 | <devsnek> | aw |
| 01:00 | <Bakkot> | I do not think we should be telling webdevs about weakrefs |
| 01:00 | <shu> | haha |
| 01:00 | <Bakkot> | weakrefs are for chains with wasm, and for no other purpose |
| 01:00 | <Bakkot> | s/chains/cycles/ |
| 01:00 | <shu> | drousso_: yeah, bind() is okay for V8 |
| 01:00 | <drousso_> | oh we don't even use `bind()` |
| 01:00 | <drousso_> | sorry i should've clarified |
| 01:00 | <devsnek> | Bakkot: web devs write wasm now |
| 01:00 | <shu> | Bakkot: fwiw i do think the event listener case *is* compelling |
| 01:00 | <Bakkot> | devsnek no they don't |
| 01:00 | <shu> | you almost never want to hold onto event listeners strongly |
| 01:00 | <Bakkot> | devsnek their tools do |
| 01:01 | <devsnek> | nah they manually write the heap implementation |
| 01:01 | <drousso_> | shu because we have our own Event Listener system, we allow the `thisObject` to passed in along with the `listener`, which are both held by the `eventTarget` |
| 01:01 | <Bakkot> | devsnek well, they should not do that either |
| 01:01 | <shu> | drousso_: ah ha |
| 01:01 | <Bakkot> | webdevs use weakrefs too I'm sure |
| 01:01 | <Bakkot> | but, like, don't |
| 01:01 | <devsnek> | i use weakrefs sometimes |
| 01:01 | <drousso_> | shu this way we can avoid creating new functions when using a prototype/class function as the `listener` |
| 01:01 | <shu> | i mean the "you must be this tall to ride" is like 10ft |
| 01:01 | <devsnek> | i should check for that in my usage of them |
| 01:02 | <shu> | because it requires you to understand, like this example shows, to understand not only the GC but how closures are represented |
| 01:02 | <shu> | which seems unreasonable to ask of for most web devs |
| 01:02 | <Bakkot> | shu I clean up my listeners manually when I'm done with them, which seems... obviously superior? |
| 01:02 | <Bakkot> | I guess this is the manual vs automatic memory management debate |
| 01:02 | <shu> | Bakkot: correct |
| 01:02 | <Bakkot> | except that the automatic one is way harder to use because it is not automatic at all |
| 01:02 | <devsnek> | why do js devs not leaksan |
| 01:02 | <drousso_> | Bakkot the "problem" with that is knowing when you're done with them |
| 01:03 | <drousso_> | it's totally doable |
| 01:03 | <shu> | Bakkot: at even google levels of effort, whatever you take that to mean, there is evidence that manual management does not scale to complex projects, like GSuite |
| 01:03 | <drousso_> | just can be hard to keep track of as the size of something increases |
| 01:03 | <shu> | leaks abound |
| 01:03 | <shu> | there is a Disposable pattern that people recommend using |
| 01:03 | <drousso_> | ^ same here |
| 01:03 | <shu> | but it is buggy |
| 01:04 | <Bakkot> | shu I am not convinced leaks will go down if we tell people about weakrefs, though |
| 01:04 | <shu> | that's a separate discussion |
| 01:04 | <drousso_> | i think it's more library/framework authors that'll benefit |
| 01:04 | <shu> | i definitely agree most people will use them wrong |
| 01:04 | <shu> | but yeah i agree with drousso_ |
| 01:04 | <Bakkot> | right, so, we shouldn't tell webdevs about weakrefs :P |
| 01:04 | <shu> | i want the framework people at the big web property cos to hear about it |
| 01:05 | <drousso_> | lol |
| 01:05 | <drousso_> | ah poop i gtg |
| 01:05 | <devsnek> | when someone asks about weakrefs "who's asking 👀" |
| 01:05 | <drousso_> | have a good night yall :) |
| 01:05 | <drousso_> | (or equivalent wherever you are) |
| 01:05 | <shu> | g'night |
| 01:05 | <Bakkot> | we should get yusuke or someone to write a library that does the right thing and tell people about that instea |
| 01:05 | <Bakkot> | d |
| 01:06 | <Bakkot> | maybe I'll ask Gilad if he wants to do that after his current project |
| 01:07 | <Bakkot> | the answer will be no, though |
| 01:07 | <shu> | gilad? |
| 01:07 | <Bakkot> | https://en.wikipedia.org/wiki/Gilad_Bracha |
| 01:08 | <Bakkot> | used to sit two desks over from me, back when being proximate to other people was not illegal |
| 01:08 | <devsnek> | oh he made java |
| 01:08 | <shu> | ah *the* gilad |
| 01:08 | <shu> | he's at f5? |
| 01:09 | <Bakkot> | yeah, joined Shape a few months before the acquisition |
| 01:09 | <shu> | oh wow, good catch |
| 01:09 | <shu> | sure ask away, i'm also sure it'll be a no |
| 01:09 | <rkirsling> | lol he made a Newspeak language with all sorts of associated 1984 references? |
| 01:10 | <rkirsling> | amusing |
| 14:37 | <littledan> | shu: Everyone's been talking about this observer pattern as a primary use case; it'd be great to have some code for this in the WeakRefs README so people can use it as a "cookbook" |
| 14:37 | <littledan> | once you work it out correctly... |
| 15:08 | <shu> | littledan: a cookbook sounds good... |