Finalization group iterator vs Array

[littledan]

Why is the cleanup callback called with a custom iterator, as opposed to a simple Array of the relevant elements? An Array would seem simpler.

[littledan]

@tschneidereit explained that this is so that you can break out of the loop early if your time to perform finalization actions ends.

[kmiller68]

Why not just have the callback directly called with the holdings value as "separate" calls (the engine can iterate under the hood)? That seems like a simpler API from the user's perspective.

[kmiller68]

Thinking about this more, I think the callback taking the holdings directly is the only sane way to design this API. There are so many issues with the iterator escaping and reentrancy that could be avoided without the iterator.

[littledan]

Reopening to discuss @kmiller68 's comments.

Personally, I was initially leaning in the way that @kmiller68 is getting at. However, @tschneidereit convinced me otherwise: that it's important to be able to express backpressure (e.g., related to the time budget for doing this processing within a frame), and then let the engine handle the backpressure in an implementation-defined way, so that the GC has a role in defining the relative rates of these things. I think the reentrancy issues are actually really simple, and resolved well in #159 .

[wingo]

Speaking only to the backpressure issue, and not about any other aspects of the iterator interface: I am not sure that there is any effective backpressure concern that the iterator interface solves.

What would an engine do if you don't drain the finalization iterator? It would keep the memory for the holdings, and obviously any associated resources that would be released via cleanup. This would lead to larger heap sizes as the GC feels the need for more headroom, or to more frequent collection, and it also creates a kind of debt in terms of pending finalization work. All of these provide backpressure on mutator throughput. Indeed if I understand correctly, the only backpressure to allocation rate in the mutator is time spent in the collector. These concerns are the same with a holdings set that's given to JS in the form of an array, or whether it's kept private to an iterator.

There is the question of latency of the finalization callback. Here there are two sources: collating the set of collectable holdings, and actually running cleanup on the items. I think that the collation overhead would be similar if the callback were passed an array instead of an iterator. Also, collation can happen concurrently and in parallel. As far as the "user-space" cleanup latency, the finalizer callback can always suspend itself and run more work in the future. I am not sure that there is any way in which you can apply backpressure and affect finalizer latency.

In summary, AFAIU, backpressure is about throughput, not latency. All of the proposed interfaces can provide backpressure on throughput by storing a pending set of finalizable holdings in memory, and all of them can manage latency to the same degree by deferring work to later. At some point of course, if the finalization throughput is too high, the mutator's throughput will have to drop.

[littledan]

@wingo I don't quite follow your explanation. The latency concern here is that doing the cleanup work may take time, that may blow your frame budget, in an environment like the web that renders on the main thread. It may be better to put off the work. When you do put it off, the engine would be the one best positioned to make the call of when to take it up again--making a tradeoff based on a combination of memory pressure and understanding of how much idle time is likely to be available. Yes, putting off of this work may increase memory usage: that's why this is a tricky tradeoff. The current iterator-based API gives engines the flexibility to explore this tradeoff.

[hotsphink]

Why is the engine best positioned? It doesn't know how long user-space finalization is going to take. It'll pretty much have to process an element, check the elapsed time, and decide whether to continue or not. That could be done in user-space with either the iterator or the Array, or with the callback-at-a-time approach if it had a "suspend" return value. The difference is that user-space doesn't have information on either expected idle time or memory pressure.

IIUC, @wingo is arguing that not fully processing all finalizers (whether decided by the engine or user-space code) will provide the back-pressure signal.

memory-pressure: if memory is low, do more finalization. This makes sense only if finalization is going to drop references to a lot of memory. User-space code is more likely to predict this than the engine.

A bit of a tangent, but user-space might want to reorder finalizers. Any of the three allow for this, though the Array is the most straightforward. (Callback-at-a-time is worst, in that the engine isn't guaranteed to trigger all finalizers even if the userspace code is just appending to a queue.)

expected idle time: the engine may have a good guess at this; user-space code probably won't. Then again, the benefit is pretty small. The engine can't predict how long finalizers will take to run, while the user-space code might. If this is owned by the engine, then callback-at-a-time provides a good mechanism for controlling how much runs (by only running one if there's enough expected time remaining, and suspending once it gets too close to the deadline.) User-space code can only control the total duration of finalization work that it processes at a time. But for the web platform, it could use idle callbacks to do pretty much everything the engine could. (And it could break down finalization work into more pieces as well, though this is possible with any approach.)

Assuming I'm understanding all of this properly, I lean towards an Array approach because I expect the gain from user-space chunking control to be larger than the gain from hypothetical engine scheduling. Most users can just do all the finalization at once. The ones who need more will likely want to take application-specific knowledge into account. In any case, I'm not clear on what iterators buy us.

[mhofman]

Maybe I'm missing something but isn't the iterator a superset of the array?

If a program wants to do it's own chunking, take into consideration application knowledge or process finalization is a different order, it can always do:

const arr = [...iter];

[hotsphink]

Maybe I'm missing something but isn't the iterator a superset of the array?

Yes. I'm concerned it adds implementation and specification complexity for no gain over a simpler Array.

[Jamesernator]

As an alternative alternative idea, why not just have FinalizationGroup expose an async iterator? Then people can just do things like:

const group = new FinalizationGroup(async (holdings) => {
  for await (const holding of holdings) {
    cleanupSomehow(holdings);
  }
});

This would solve the problem of re-entrancy as well as there'd only be a single iterator.

[mhofman]

Interesting idea, but it does make cleanupSome problematic. My understanding is that one of the use case for cleanupSome is to support purely synchronous code.

[littledan]

I think we should keep the API as is (a synchronous Iterator), for reasons discussed above in this thread.

[devsnek]

I'm also thinking it should probably be an array. The engine is free to provide as few elements as it wants if it wants to limit how much cleanup is done, and the callback is free to process as many or as few items as it wants, possibly storing them for later based on its own scheduling concerns. It also gets rid of this weird encapsulation failure where the engine has to care about whether or not the callback handled the items (#34).

[zenparsing]

@devsnek has it.

[littledan]

The explanation in #60 (comment) is missing the possibility of benefits from leaving these scheduling decisions to the engine. Sometimes, the engine may have more knowledge about not just when it seems to be idle, and also what sort of memory pressure is being experienced currently (when running the finalizer can free up memory pointed to by the held value). So, giving the system power to determine the timing of finalizers can be beneficial to the end user in terms of memory usage and responsiveness.

[devsnek]

@littledan isn't that the "engine omits some items from the array" case?

[littledan]

@devsnek I don't understand the relationship between the array/iterator/callback question and omitting items from the array. Omissions would be permitted no matter which alternative we choose. I was thinking of the motivation for the iterator as being more related to giving the engine freedom to when to re-schedule the callbacks (which has nothing to do with omission). (However, based on some more recent feedback from implementers, it's not clear exactly how engines can/should take advantage of this freedom; maybe it's not a strong argument.)

[devsnek]

@littledan my point was that with an array the engine doesn't know how much of it was "consumed" so it can't reschedule anything. I don't think it makes sense that you'd rely on the engine implicitly scheduling things, and browsers already have requestIdleCallback.

[zenparsing]

More generally, the iterator-with-reschedule design allows for a limited form of bidirectional communication between the application and the garbage collector, allowing the application to "throw things back" to the engine. It's not clear to me that this complexity is useful, especially in the limited form where the application can only "throw back" the things at the tail of the sequence. A one-way data flow with separation of concerns would seem preferable, if we can get away with it. Is something like requestIdleCallback sufficient from the application's point of view?

[littledan]

Seems like this discussion rests on whether engines have a way of dealing with this which is more detailed/advanced than requestIdleCallback which is worth it in practice, or whether we expect one to be developed in the future. Now that we have implementations due to Stage 3, we can look into this question in some more detail. cc @syg @kmiller68 @hotsphink @phoddie

[devsnek]

I also don't understand how we can simultaneously say "don't depend on the callback ever being called" and "depend on the callback being called not just a few times but as many times as you need it to be called". Like as a user I would be afraid to not either handle all the items at once or collect them in an array and do my own scheduling cuz who knows when the finalizer will run again.

[littledan]

Yes, we'd have to document the shared expectation that the callback will probably eventually be re-triggered, unless the program shuts down. (You shouldn't depend in a strong way on the finalizer ever being called in the first place, though.)

[syg]

To sum up, we're confounding two very different questions:

1. Should the user be able to communicate a "didn't fully consume" signal back to the engine, and what should that signal do?
2. How should holdings be iterated?

The hard question is 1.

Firstly, I don't see a way for the spec to require a cleanup to be rescheduled if the user doesn't fully consume an iterator. I'd like to affirm the consensus that the spec does not require cleanups to be rescheduled if the user doesn't fully consume all dead cells in a single invocation of the cleanup callback, however the "doesn't fully consume" part is signaled to the engine.

Secondly, I also don't think we all share the expectation that the cleanup callback will be eventually re-triggered. It's a reasonable implementation strategy to only re-trigger the cleanup if a new finalization event is seen by the FinalizationRegistry. I.e. an object registered to the registry dying after the initial callback that partially consumed the iterator.

If we agree to that, that means the "didn't fully consume" signal is just a hint. And the question becomes: is it a useful hint? We have some evidence from the implementers that it is not a useful hint from @littledan's above comment.

It's a weird hint anyway, right? If the intent is to give full control of incrementality to the engine, that signal shouldn't come from the user. The engine shouldn't hand out an iterator for the full dead cell list if it thinks it'll take too long. This hint assumes that the user knows best how long each finalization slice should take, but the engine knows best when to schedule the next slice. That's weird, right?

Given all of that then, I would prefer there to be no way to communicate "didn't fully consume" back to the engine. I.e. have communication be unidirectional per #60 (comment).

There are ways to make the communication unidirectional no matter how holdings are iterated. An iterator can copy and clear the FinalizationRegistry's cleared cell list in the beginning, and it's more obvious with an array or a per-item callback.

If we want to let engines have maximum control of incremental slices, then an iterator or per-item callback are better than an array, since it's much easier for the engine to adhere to a budget if it's able to cancel the loop. With an array the engine would need to guess the number of items ahead of time that'd fit in the budget.

From the spec's POV, the engine doing its own incremental thing is indistinguishable from an oracle that magically only collects the right set of objects per FinalizationRegistry to fit a time budget. So it's fine for the spec to clear the dead cell list per cleanup call.

[devsnek]

Iterator that copies the elements is a good idea, I'd be okay with that as well.

[mhofman]

There are ways to make the communication unidirectional no matter how holdings are iterated. An iterator can copy and clear the FinalizationRegistry's cleared cell list in the beginning, and it's more obvious with an array or a per-item callback.

Just to make sure I got this right, not consuming the iterator in the callback would then mean we can never get those holdings anymore?

What would be the behavior if the callback calls unregister for a cell that is in the copied cell list?

I'm not really comfortable with a per-item callback. It feels spammy and the program no longer has a way to batch process the holdings.

[syg]

Just to make sure I got this right, not consuming the iterator in the callback would then mean we can never get those holdings anymore?

That's what I'm proposing, yes. In the same way if it was an array you wouldn't get them if you didn't stash'em.

What would be the behavior if the callback calls unregister for a cell that is in the copied cell list?

I dunno. If the iterator clears the list before invoking the callback, it'd return false. If the iterator clears the list after invoking the callback, it'd return true. Both seem reasonable?

I'm not really comfortable with a per-item callback. It feels spammy and the program no longer has a way to batch process the holdings.

Fair enough. I don't feel strongly at all.

[mhofman]

If that's the idea, then maybe an array is better, because it communicates that the set of holdings is fixed (unregister will not prevent the related holding from being iterated on), and you won't be given a second chance to process the holdings.

[syg]

If that's the idea, then maybe an array is better, because it communicates that the set of holdings is fixed (unregister will not prevent the related holding from being iterated on), and you won't be given a second chance to process the holdings.

I never understood that communication. The iterator is already invalidated when the cleanup function returns: step 8 here. You can't stash the iterator somewhere and use it later.

I dislike arrays for the reason I gave above: it limits the engine's ability to better manage an incremental slice time budget.

Edit: of course the "better manage" point is a heuristic. A user cleanup callback can still mess it up by e.g. iterating up front and copying to an array, but the point is that with an array there's no way for the user to write something that's amenable to being terminated in the middle.

[mhofman]

IMO an iterator gives an impression of the program controlling the consumption.

Currently when not consuming the holdings, you could get them later (new callback or manual call to cleanupSome). Now they'd just be gone.

Also with an iterator, I'd kinda expect holdings for unregistered cells to not be yielded.

[zenparsing]

I dislike arrays for the reason I gave above: it limits the engine's ability to better manage an incremental slice time budget.

That's interesting; I didn't consider the engine deciding "hey, too much time has passed inside of the callback, so let's reserve some things for later". In such a case, would JS users have to know that they ought to use for-of rather than conversion to array and filtering/mapping/etc. (or using corresponding iterator prototype methods)?

[syg]

@mhofman

IMO an iterator gives an impression of the program controlling the consumption.

Fair enough. Pull-based iteration does give that impression, vs a push-based iteration like per-item callbacks. Is batching your main concern against the per-item callback? That's an arguably more advanced use case, and it's still possible to express with per-item callbacks, just more awkwardly.

Also with an iterator, I'd kinda expect holdings for unregistered cells to not be yielded.

That can be possible if you clear the list after the cleanup and not before.

@zenparsing

In such a case, would JS users have to know that they ought to use for-of rather than conversion to array and filtering/mapping/etc. (or using corresponding iterator prototype methods)?

for-of wouldn't help unless the array was a special array that vended a special iterator. Which... is possible, but seems too complex to be worth it.

[syg]

Here is the recap and my position.

The core design questions are around backpressure:

1. Who should express it?
   Maybe the engine. It has the best idea what the time budget is. Maybe the developer. They know what fits best for the app. If the developer can express backpressure, then we also need to ask:

2. What should engines do about user-signaled backpressure?
   The original motivation alluded to here was to have the engine do something smart. After doing the work, I think all of V8, JSC, and SM implementers don't think there's an obviously smart thing to do. Does the engine reschedule? When? Should it be guaranteed to reschedule? Should it try to guarantee forward progress? Further, none of this can be mandated by the spec to be depended upon anyhow.

Additionally, there are three ways to iterate holdings, some better suited than others for expressing backpressure and user-land scheduling (chunking of finalizers, etc).

A. Iterators (current).

• Easy for the programmer to express backpressure. Affordance for backpressure by not consuming part of the iterator.
• Kind of easy for programmer to chunk / schedule.
• Easy for the engine to express backpressure by stopping iteration early.
• Small allocation of one object.
• Has to be invalidated.

B. Array.

• Hard for the programmer to express backpressure. No affordance for backpressure.
• Easy for programmer to chunk / schedule.
• Hard for engine to express backpressure since it has to guess how long it takes to process the items in the array that's provided.
• Big allocation of an array up front.
• Don't have to be invalidated.

C. Per-item callback function.

• Not easy or hard for the programmer to express backpressure. Could have affordance for it via a special return code, or no affordance if its return value is ignored.
• Kind of hard for programmer to chunk / schedule.
• Easy for the engine to express backpressure by stopping invocation of the callback.
• No allocation.

After stewing on it for a few days, my opinion is: we shouldn't have user-signaled backpressure. It'll be a source of divergence and there is no obvious benefit for implementations, only complications.

Given that, I propose we change FinalizationRegistry to take a per-item callback whose return value is ignored. With the return value ignored, there is no affordance for user-signaled backpressure. Unlike an array, it offers maximum flexibility for the engine itself to express backpressure.

Edit: This is @kmiller68's original proposal which I am now fully on board with.

@tschneidereit @kmiller68 @hotsphink What do you think?

[kmiller68]

I'm all for the per-item call back! It's the same API I originally argued for and still think is the best overall. You've done a much better job laying out the pros and cons than I ever did though.

[mhofman]

Out of curiosity, what is the idea for supporting program chunking with per-item callback?

Also, why not do away with the group level finalization callback then, and pass a per registration callback instead of a heldValue? Or return a promise that resolves when the target has been collected?

[mhofman]

Btw, the node addons API basically only allow for a per-item callback, and in my shim to support the current semantics of batching, I've basically had to rely on setImmediate. I'm not sure how you'd do it with pure ES.

[syg]

Out of curiosity, what is the idea for supporting program chunking with per-item callback?

I don't think there's a pretty way to do it. I was thinking of exactly either setImmediate as you pointed out, or making a resolved promise, maybe?

Also, why not do away with the group level finalization callback then, and pass a per registration callback instead of a heldValue? Or return a promise that resolves when the target has been collected?

Because that decision is orthogonal to backpressure, and was motivated by the hypothesis that finalizers tend to be per "type" of object in practice, not per object. As for the promise API, I haven't thought it through that much but it seems scary. It'll be a natural affordance for patterns we explicitly don't want, like awaiting finalization of an object?

[littledan]

I'm pretty convinced by #60 (comment) , so I wrote up a PR for this at #187 .

[mhofman]

What should the behavior of cleanupSome be with per-item callback?

[mhofman]

What should happen if the callback throws? Should it continue with the next empty cell or interrupt the iteration?

Interrupting would technically give the program a way to provide some back pressure ;)

[littledan]

Here's the behavior in the PR:

• cleanupSome calls the callback multiple times, synchronously.
• if an exception is thrown, iteration stops. From cleanupSome, this is propagated to the caller. From the background calls, on the web, this will be catchable by window.onerror