explainers-by-googlers / prompt-api

A proposal for a web API for prompting browser-provided language models
Creative Commons Attribution 4.0 International
267 stars 20 forks source link

Reason behind `session.destroy()` #19

Closed jakearchibald closed 4 months ago

jakearchibald commented 4 months ago

Why does this need a manual destroy method rather than relying on GC? It seems relatively uncommon in web APIs.

domenic commented 4 months ago

Fair question. We may have just been exposing the internals without thinking through it enough.

The reason it would make sense would be if these sorts of sessions represent large or limited amounts of system resources that you want to be able to deterministically reclaim, apart from just memory. E.g., similar to fileHandle.close()

I can't see a clear reason why this would be the case though, so I suspect we can remove this.

domenic commented 4 months ago

Right, so, starting up a text session will consume multiple gigabytes of memory and/or graphics memory. It seems plausible that developers might want to deterministically release that memory. Especially because tracking down every reference to your AITextSession, and making sure they go out of scope or get set to null, is not trivial in most apps.

What do you think?

jakearchibald commented 4 months ago

E.g., similar to fileHandle.close()

I thought that existed because of locking behaviour - the open/closed state is observable elsewhere and you want to be able to control the timing of it.

I don't have strong feelings here, but why add it here and not blob.destroy(), img.destroy(), document.destroy() etc etc.

MuraraAllan commented 4 months ago

I think the .destroy comes actually in hand. And should be kept there.

It is not that heavy to instantiate new sessions, it is heavy to first load the model. But with the multi-shot system prompt, might be interesting to leave the session open, to clone or etc. Plus, what is Garbage in an inference session?

Assumi v8 for my comment :

Like when to know it should be garbaged? What to garbage? Nowadays the turbofan can't even identify exactly what is happenning under the hood with the API. Turbolizer seems to just understand something is being requested. And GC of turbofan is much more oriented to what is still rendered or not on the DOM.

That is why it seems to be confusion and even relation to the loadFiles and blob files, which is the DOM area.

Here we are in the window area, the GC must be treated related to JavascriptObjects and not related to Documents / DOM.

Just for clarification purposes, the GC behaves way different when understanding a blob ( related to a input ) and a blob ( related to window.x ). So we might be going out of context when comparing blob / img.destroy with window.X methods

domenic commented 4 months ago

I don't have strong feelings here, but why add it here and not blob.destroy(), img.destroy(), document.destroy() etc etc.

I think largely because these aren't multi-gigabyte resources, in the majority of cases. (Documents, maybe, but destroying those while leaving JavaScript running is hard to imagine...)

Yvem commented 2 months ago

I agree that destroy() feels a bit alien in a Garbage Collected language such as JS.

Also it's unclear whether calling "destroy" is needed or optional.

I understand the need for now.

But could we check if it's true that the GC can't accurately free such resources? (as implied by @MuraraAllan) If true, I'd pass that to the v8 team to understand what can be done about it. Maybe this could be the opportunity to devise a new pattern to hint the interpreter / v8?

domenic commented 2 months ago

GCs can destroy the resources. The point of destroy() is if you want to release them before GC. This is explained in the readme.