Re: [IndexedDB] setVersion blocked on uncollected garbage IDBDatabases

[Jonas Sicking]

On Wed, Feb 9, 2011 at 6:12 PM, Glenn Maynard  wrote:
> On Wed, Feb 9, 2011 at 8:39 PM, Jonas Sicking  wrote:
>>
>> On Wed, Feb 9, 2011 at 5:26 PM, Glenn Maynard  wrote:
>> > Another related issue: what happens if a long-running number-cruncher
>> > worker
>> > keeps a database open while it works, to read data or output results?
>> > There's no API for sending versionchange events for IDBDatabaseSync, and
>> > it
>> > doesn't fit in any obvious way since it's inherently asynchronous.  I
>> > suppose the developer-side workaround would be to open the same database
>> > asynchronously in the UI thread just to listen for versionchange, and to
>> > terminate the thread when it's received.  Is that generally what's
>> > intended?
>>
>> This is a good point. We should add the ability to dispatch
>> versionchange events on IDBDatabaseSync. There is no reason we
>> couldn't just make it a event target and add a .onversionchange
>> property to it.
>>
>> Just because the object has some (or many, or only) synchronous
>> methods, doesn't mean it can't also dispatch events.
>
> That works provided your worker periocally returns to the caller, to allow
> events to be dispatched; otherwise the event will never be received.  That's
> usually the case--even for number-crunching workers, most use cases look
> like "receive a request, spend a few seconds doing calculations and I/O and
> return the result".
>
> Not all do--some number-crunching cases might take minutes to run.

That's true, but that's not really a problem with the IDBDatabaseSync
API, but rather with worker design. For example a worker could open
use the asynchronous IDBDatabase for a bit and then start doing a lot
of number crunching and synchronous FileReader/XMLHttpRequest
operations. We can asynchronously fire a event at the IDBDatabase
instance in the worker all we want, but as you point out, until the
worker returns to the event loop it won't be able to receive it.

We could add some sort of IDBDatabaseSync.versionChangePending
property which would allow a long-running script to check for pending
version changes and if so close the database. However it takes a fair
amount of work to use. With an event based interface all you need to
do is add a event handler once and then not worry any more about it.
With a .versionChangePending property, you have to add code to check
the property to every single inner loop where you can spend
significant amounts of time. This will quickly get out of hand as we
start adding more APIs like this which people will have to tend to in
their inner loops.

An alternative solution would be to add a
IDBDatabaseSync.onversionchange property along with an event. Then
asynchronously dispatch the event as with the asynchronous interface.
However if the event goes undispatched for X seconds, forcefully close
the IDBDatabaseSync instance. Note this means that as long as you
return to the event loop, even if you don't handle the event, the
IDBDatabaseSync won't be closed.

I'm not a huge fan of that solution though.

> The
> typical example is expensive renderers, such as a raytracer that may take a
> couple minutes to render a scene.  These sorts of tasks wouldn't want to
> return to the caller--being able to write these algorithms linearly rather
> than in an incremental, setTimeout-based fashion is a basic use case of
> workers.  That said, these are uncommon enough that it's the "asynchronous
> watcher in a separate thread" workaround seems acceptable.

Could be. And since we have worker.terminate() it is possible to
quickly terminate a worker which is using a database.

/ Jonas

Re: [IndexedDB] setVersion blocked on uncollected garbage IDBDatabases

[Glenn Maynard]

On Wed, Feb 9, 2011 at 8:39 PM, Jonas Sicking  wrote:

> On Wed, Feb 9, 2011 at 5:26 PM, Glenn Maynard  wrote:
> > Another related issue: what happens if a long-running number-cruncher
> worker
> > keeps a database open while it works, to read data or output results?
> > There's no API for sending versionchange events for IDBDatabaseSync, and
> it
> > doesn't fit in any obvious way since it's inherently asynchronous.  I
> > suppose the developer-side workaround would be to open the same database
> > asynchronously in the UI thread just to listen for versionchange, and to
> > terminate the thread when it's received.  Is that generally what's
> intended?
>
> This is a good point. We should add the ability to dispatch
> versionchange events on IDBDatabaseSync. There is no reason we
> couldn't just make it a event target and add a .onversionchange
> property to it.
>
> Just because the object has some (or many, or only) synchronous
> methods, doesn't mean it can't also dispatch events.
>

That works provided your worker periocally returns to the caller, to allow
events to be dispatched; otherwise the event will never be received.  That's
usually the case--even for number-crunching workers, most use cases look
like "receive a request, spend a few seconds doing calculations and I/O and
return the result".

Not all do--some number-crunching cases might take minutes to run.  The
typical example is expensive renderers, such as a raytracer that may take a
couple minutes to render a scene.  These sorts of tasks wouldn't want to
return to the caller--being able to write these algorithms linearly rather
than in an incremental, setTimeout-based fashion is a basic use case of
workers.  That said, these are uncommon enough that it's the "asynchronous
watcher in a separate thread" workaround seems acceptable.

(That's also another case where the API discussed in the "synchronously
handling events" thread would become useful: you could send a message to the
thread while it's running, without needing to terminate it outright.)

-- 
Glenn Maynard

Re: [IndexedDB] setVersion blocked on uncollected garbage IDBDatabases

[Drew Wilson]

On Wed, Feb 9, 2011 at 5:26 PM, Glenn Maynard  wrote
>
>
> On a related note, please see this statement from section 5.5 of the
>> MessagePort spec:
>>
>> Authors are strongly encouraged to explicitly close 
>> MessagePort objects
>> to disentangle them, so that their resources can be recollected. Creating
>> many MessagePort  objects
>> and discarding them without closing them can lead to high memory usage.
>>
>
> For explicitly-created MessagePorts this is natural enough.  It's slightly
> less obvious for MessagePorts created implicitly by Worker/SharedWorker
> objects, though; I'd predict that becoming a common cause of leaks on pages
> that create and destroy a lot of threads.
>

It's not an issue, because when a Worker shuts down it implicitly closes all
of its MessagePorts, which would allow the renderer side to be GC'd - the
leak only happens when you release a reference to a MessagePort without
closing it. If you're creating lots of worker threads and leaving them
running, then you've got bigger memory issues than a few hundred bytes of
MessagePorts.

But, yeah, back to IDB now :)

Re: [IndexedDB] setVersion blocked on uncollected garbage IDBDatabases

[Jonas Sicking]

On Wed, Feb 9, 2011 at 5:26 PM, Glenn Maynard  wrote:
> Another related issue: what happens if a long-running number-cruncher worker
> keeps a database open while it works, to read data or output results?
> There's no API for sending versionchange events for IDBDatabaseSync, and it
> doesn't fit in any obvious way since it's inherently asynchronous.  I
> suppose the developer-side workaround would be to open the same database
> asynchronously in the UI thread just to listen for versionchange, and to
> terminate the thread when it's received.  Is that generally what's intended?

This is a good point. We should add the ability to dispatch
versionchange events on IDBDatabaseSync. There is no reason we
couldn't just make it a event target and add a .onversionchange
property to it.

Just because the object has some (or many, or only) synchronous
methods, doesn't mean it can't also dispatch events.

/ Jonas

Re: [IndexedDB] setVersion blocked on uncollected garbage IDBDatabases

[Glenn Maynard]

On Wed, Feb 9, 2011 at 5:05 PM, Drew Wilson  wrote:

> In some cases we leak them, yes (they live for the life of the parent
> context) if the developer does not close them. Typically this is only when
> you've cloned a MessagePort and sent the other end to a different process.
> Trying to figure out if a port is reachable when the entangled port lives in
> a different process (and whose reachability may itself depend on the
> reachability of a third port in yet another process ad infinitum) is a
> fairly intractable problem.


OK.  At least the heap snapshotting in Chrome is good enough to make
debugging this reasonably easy, if it results in nontrivial leaks.


>  This does not happen with the implicit ports associated with dedicated
> workers, because in WebKit those aren't actually MessagePorts (mainly
> because the webkit implementation of dedicated workers predates message
> ports). Chromium SharedWorkers use real MessagePorts though, so if you do
> this:
>
> var w = new SharedWorker("foo.html");
> w.port.onmessage = function() {...};
>
> that port will not be GC'd in chromium, even if the SharedWorker drops its
> end of the port on the floor, until either the SharedWorker calls close() on
> the port, the worker exits or your parent document is collected.
>

That's natural, of course--the event handler holds a reference to the
object.

On a related note, please see this statement from section 5.5 of the
> MessagePort spec:
>
> Authors are strongly encouraged to explicitly close 
> MessagePort objects
> to disentangle them, so that their resources can be recollected. Creating
> many MessagePort  objects
> and discarding them without closing them can lead to high memory usage.
>

For explicitly-created MessagePorts this is natural enough.  It's slightly
less obvious for MessagePorts created implicitly by Worker/SharedWorker
objects, though; I'd predict that becoming a common cause of leaks on pages
that create and destroy a lot of threads.


Back on IDB: If one page has an opened database (whether from not being
closed, a GC issue, or a simpler site issue), and a new tab is failing to
setVersion as a result, figuring out the cause may be painful.  Hopefully
browsers will at least be able to tell which other tab is holding the
database open, and retrieve a stack trace of where it was opened, even if
it's in a closed tab.  If that information is available, it would be useful
to include it in the "blocked" event, as an optional, informative message
that scripts can send to their server-side error logging if it blocks for
too long.

Another related issue: what happens if a long-running number-cruncher worker
keeps a database open while it works, to read data or output results?
There's no API for sending versionchange events for IDBDatabaseSync, and it
doesn't fit in any obvious way since it's inherently asynchronous.  I
suppose the developer-side workaround would be to open the same database
asynchronously in the UI thread just to listen for versionchange, and to
terminate the thread when it's received.  Is that generally what's intended?

-- 
Glenn Maynard

Re: [IndexedDB] setVersion blocked on uncollected garbage IDBDatabases

[Drew Wilson]

On Wed, Feb 9, 2011 at 2:03 PM, Jonas Sicking  wrote:

>
> For what it's worth, shared workers already expose GC behavior. You'll
> get a already-existing shared worker, or a new one will be created,
> depending on if GC has collected the worker or not.
>

Hmmm. That was certainly not the intent when I was discussing the WebKit
implementation with Hixie, but either there was a miscommunication or the
definition of when a document becomes "discarded" has changed since then. In
the WebKit implementation, SharedWorker lifecycle is not tied to GC, but is
instead tied to specific deterministic navigation/close actions on the
parent document.

Dependence of SharedWorkers on GC is bad for all the same reasons I outlined
above - I'll follow up in a separate thread whether that is indeed the
intent of the spec.


> Not saying that this is great, just pointing out that this is the case.
>
> / Jonas
>

Re: [IndexedDB] setVersion blocked on uncollected garbage IDBDatabases

[Drew Wilson]

In some cases we leak them, yes (they live for the life of the parent
context) if the developer does not close them. Typically this is only when
you've cloned a MessagePort and sent the other end to a different process.
Trying to figure out if a port is reachable when the entangled port lives in
a different process (and whose reachability may itself depend on the
reachability of a third port in yet another process ad infinitum) is a
fairly intractable problem.

This does not happen with the implicit ports associated with dedicated
workers, because in WebKit those aren't actually MessagePorts (mainly
because the webkit implementation of dedicated workers predates message
ports). Chromium SharedWorkers use real MessagePorts though, so if you do
this:

var w = new SharedWorker("foo.html");
w.port.onmessage = function() {...};

that port will not be GC'd in chromium, even if the SharedWorker drops its
end of the port on the floor, until either the SharedWorker calls close() on
the port, the worker exits or your parent document is collected.

On a related note, please see this statement from section 5.5 of the
MessagePort spec:

Authors are strongly encouraged to explicitly close
MessagePort objects
to disentangle them, so that their resources can be recollected. Creating
many MessagePort  objects and
discarding them without closing them can lead to high memory usage.

-atw

On Wed, Feb 9, 2011 at 12:56 PM, Glenn Maynard  wrote:

> On Wed, Feb 9, 2011 at 2:05 PM, Drew Wilson  wrote:
>
>> This discussion reminds me of a similar issue with MessagePorts. The
>> original MessagePort spec exposed GC behavior through the use of onclose
>> events/closed attributes on MessagePorts. It turns out that on Chromium,
>> there are situations where it's very difficult for us to GC MessagePorts (a
>> port's reachability depends on the reachability of the entangled port on an
>> entirely separate process), and so we just don't.
>
>
> Err, so you just leak MessagePorts?  Or just in those situations, whatever
> they are?  Does this happen even for the implicit MessagePort associated
> with each worker?
>
> --
> Glenn Maynard
>

Re: [IndexedDB] setVersion blocked on uncollected garbage IDBDatabases

[Jonas Sicking]

On Wed, Feb 9, 2011 at 11:05 AM, Drew Wilson  wrote:
> This discussion reminds me of a similar issue with MessagePorts. The
> original MessagePort spec exposed GC behavior through the use of onclose
> events/closed attributes on MessagePorts. It turns out that on Chromium,
> there are situations where it's very difficult for us to GC MessagePorts (a
> port's reachability depends on the reachability of the entangled port on an
> entirely separate process), and so we just don't.
> My concern is that there may be situations like this with IDB - if at some
> point it's possible for events to be fired on an IDB instance (if we support
> triggers), you'll have a situation where the reachability of an IDB instance
> may depend on the reachability of that same DB in other processes. The net
> effect is that on multi-process/multi-heap platforms, we may not be able to
> GC databases, while on other platforms (which have a unified heap) we will
> be able to GC them. This will end up being a source of cross-browser
> incompatibility, because code will work just fine on platforms that are able
> to deterministically GC databases, but then will break on other platforms
> that cannot.
> (As an aside, Jeremy mentions that there may already be situations where we
> cannot GC databases today - I don't know the spec well enough to comment,
> though, so perhaps he can elaborate).

I definitely agree that if there could be events fired at the
IDBDatabase instance, and there are event listeners attached to it,
then you shouldn't GC the instance. This is similar to how you
shouldn't GC  elements which are still loading images and have an
onload handler attached.

That seems no different from script holding a strong reference to the
instance though, so it's a situation the implementation has to deal
with anyway (by firing a "blocked" event at the setVersion request and
waiting for the user to close the tab).

> In any case, I don't think that IDB should be the first place in the entire
> web platform where we expose GC behavior to clients.

For what it's worth, shared workers already expose GC behavior. You'll
get a already-existing shared worker, or a new one will be created,
depending on if GC has collected the worker or not.

Not saying that this is great, just pointing out that this is the case.

/ Jonas

Re: [IndexedDB] setVersion blocked on uncollected garbage IDBDatabases

[Glenn Maynard]

On Wed, Feb 9, 2011 at 2:05 PM, Drew Wilson  wrote:

> This discussion reminds me of a similar issue with MessagePorts. The
> original MessagePort spec exposed GC behavior through the use of onclose
> events/closed attributes on MessagePorts. It turns out that on Chromium,
> there are situations where it's very difficult for us to GC MessagePorts (a
> port's reachability depends on the reachability of the entangled port on an
> entirely separate process), and so we just don't.


Err, so you just leak MessagePorts?  Or just in those situations, whatever
they are?  Does this happen even for the implicit MessagePort associated
with each worker?

-- 
Glenn Maynard

Re: [IndexedDB] setVersion blocked on uncollected garbage IDBDatabases

[Drew Wilson]

On Wed, Feb 9, 2011 at 11:20 AM, Jeremy Orlow  wrote:

> On Wed, Feb 9, 2011 at 11:05 AM, Drew Wilson  wrote:
>
>> This discussion reminds me of a similar issue with MessagePorts. The
>> original MessagePort spec exposed GC behavior through the use of onclose
>> events/closed attributes on MessagePorts. It turns out that on Chromium,
>> there are situations where it's very difficult for us to GC MessagePorts (a
>> port's reachability depends on the reachability of the entangled port on an
>> entirely separate process), and so we just don't.
>>
>> My concern is that there may be situations like this with IDB - if at some
>> point it's possible for events to be fired on an IDB instance (if we support
>> triggers), you'll have a situation where the reachability of an IDB instance
>> may depend on the reachability of that same DB in other processes. The net
>> effect is that on multi-process/multi-heap platforms, we may not be able to
>> GC databases, while on other platforms (which have a unified heap) we will
>> be able to GC them. This will end up being a source of cross-browser
>> incompatibility, because code will work just fine on platforms that are able
>> to deterministically GC databases, but then will break on other platforms
>> that cannot.
>>
>> (As an aside, Jeremy mentions that there may already be situations where
>> we cannot GC databases today - I don't know the spec well enough to comment,
>> though, so perhaps he can elaborate).
>>
>
> Yeah.  Talking to Drew made me realize that we (WebKit) already have a
> cycle so that we probably can't collect IDBDatabase objects with event
> listeners attached to it.  When there's a listener, we have to hold a
> reference to the JavaScript wrapper since it's what holds onto the
> JavaScript function we call.  But the wrapper holds a reference to our
> WebCore object.  We can break the cycle only when we know that we're not
> going to call any more events on it.  We know that when .close() is called.
>
> Working around this as is will be tricky, but isn't really a spec problem.
>  But it does mean that the developer will need to always call .close() or
> ask the user to close the tab in order to ever be able to run a setVersion
> transaction.  At least for the time being in any WebKit browser.
>

This mainly becomes an issue if some browsers are able to GC when others are
not - if that happens, then you end up with incompatible behavior because
setVersion() calls that work on one browser won't work on others. So if
setVersion() is going to expose GC behavior, then I think you have to codify
in the spec the Least Common Denominator GC behavior (for example,
spec-compliant implementations *must not* GC databases with event handlers
registered). This is a rabbit hole I don't think we want to go down.


>
>
>> In any case, I don't think that IDB should be the first place in the
>> entire web platform where we expose GC behavior to clients.
>>
>> -atw
>>
>> On Tue, Feb 8, 2011 at 4:43 PM, Jonas Sicking  wrote:
>>
>>> On Tue, Feb 8, 2011 at 3:31 PM, Glenn Maynard  wrote:
>>> > On Tue, Feb 8, 2011 at 4:01 PM, Jeremy Orlow 
>>> wrote:
>>> >>
>>> >> I talked it over with Darin (Fisher), and he largely agreed with you
>>> guys.
>>> >>  I'll file a bug saying that after unload, all IDBDatabases attached
>>> to that
>>> >> document should be closed.
>>> >
>>> > What happens if a database is open in a page in the back-forward cache?
>>> > That's incompatible with onunload having side-effects.
>>> >
>>> > I know the BF-cache is off-spec, but it's extremely useful and will
>>> > hopefully find its way into the standard some day, so it'd be nice to
>>> keep
>>> > it in mind.
>>> >
>>> > I suppose the browser would discard whole pages from the BF-cache on
>>> demand
>>> > if required by a setVersion call.
>>>
>>> That's exactly what we do in Firefox. Implementations have to be able
>>> to throw things out of the BF cache on command anyway (since you
>>> generally want to limit the number of pages living in BF cache, and so
>>> loading a new page often causes other pages to be thrown out), so it's
>>> just a matter of calling into the same code here.
>>>
>>> / Jonas
>>>
>>>
>>
>

Re: [IndexedDB] setVersion blocked on uncollected garbage IDBDatabases

[Jeremy Orlow]

On Wed, Feb 9, 2011 at 11:05 AM, Drew Wilson  wrote:

> This discussion reminds me of a similar issue with MessagePorts. The
> original MessagePort spec exposed GC behavior through the use of onclose
> events/closed attributes on MessagePorts. It turns out that on Chromium,
> there are situations where it's very difficult for us to GC MessagePorts (a
> port's reachability depends on the reachability of the entangled port on an
> entirely separate process), and so we just don't.
>
> My concern is that there may be situations like this with IDB - if at some
> point it's possible for events to be fired on an IDB instance (if we support
> triggers), you'll have a situation where the reachability of an IDB instance
> may depend on the reachability of that same DB in other processes. The net
> effect is that on multi-process/multi-heap platforms, we may not be able to
> GC databases, while on other platforms (which have a unified heap) we will
> be able to GC them. This will end up being a source of cross-browser
> incompatibility, because code will work just fine on platforms that are able
> to deterministically GC databases, but then will break on other platforms
> that cannot.
>
> (As an aside, Jeremy mentions that there may already be situations where we
> cannot GC databases today - I don't know the spec well enough to comment,
> though, so perhaps he can elaborate).
>

Yeah.  Talking to Drew made me realize that we (WebKit) already have a cycle
so that we probably can't collect IDBDatabase objects with event listeners
attached to it.  When there's a listener, we have to hold a reference to the
JavaScript wrapper since it's what holds onto the JavaScript function we
call.  But the wrapper holds a reference to our WebCore object.  We can
break the cycle only when we know that we're not going to call any more
events on it.  We know that when .close() is called.

Working around this as is will be tricky, but isn't really a spec problem.
 But it does mean that the developer will need to always call .close() or
ask the user to close the tab in order to ever be able to run a setVersion
transaction.  At least for the time being in any WebKit browser.


> In any case, I don't think that IDB should be the first place in the entire
> web platform where we expose GC behavior to clients.
>
> -atw
>
> On Tue, Feb 8, 2011 at 4:43 PM, Jonas Sicking  wrote:
>
>> On Tue, Feb 8, 2011 at 3:31 PM, Glenn Maynard  wrote:
>> > On Tue, Feb 8, 2011 at 4:01 PM, Jeremy Orlow 
>> wrote:
>> >>
>> >> I talked it over with Darin (Fisher), and he largely agreed with you
>> guys.
>> >>  I'll file a bug saying that after unload, all IDBDatabases attached to
>> that
>> >> document should be closed.
>> >
>> > What happens if a database is open in a page in the back-forward cache?
>> > That's incompatible with onunload having side-effects.
>> >
>> > I know the BF-cache is off-spec, but it's extremely useful and will
>> > hopefully find its way into the standard some day, so it'd be nice to
>> keep
>> > it in mind.
>> >
>> > I suppose the browser would discard whole pages from the BF-cache on
>> demand
>> > if required by a setVersion call.
>>
>> That's exactly what we do in Firefox. Implementations have to be able
>> to throw things out of the BF cache on command anyway (since you
>> generally want to limit the number of pages living in BF cache, and so
>> loading a new page often causes other pages to be thrown out), so it's
>> just a matter of calling into the same code here.
>>
>> / Jonas
>>
>>
>

Re: [IndexedDB] setVersion blocked on uncollected garbage IDBDatabases

[Drew Wilson]

This discussion reminds me of a similar issue with MessagePorts. The
original MessagePort spec exposed GC behavior through the use of onclose
events/closed attributes on MessagePorts. It turns out that on Chromium,
there are situations where it's very difficult for us to GC MessagePorts (a
port's reachability depends on the reachability of the entangled port on an
entirely separate process), and so we just don't.

My concern is that there may be situations like this with IDB - if at some
point it's possible for events to be fired on an IDB instance (if we support
triggers), you'll have a situation where the reachability of an IDB instance
may depend on the reachability of that same DB in other processes. The net
effect is that on multi-process/multi-heap platforms, we may not be able to
GC databases, while on other platforms (which have a unified heap) we will
be able to GC them. This will end up being a source of cross-browser
incompatibility, because code will work just fine on platforms that are able
to deterministically GC databases, but then will break on other platforms
that cannot.

(As an aside, Jeremy mentions that there may already be situations where we
cannot GC databases today - I don't know the spec well enough to comment,
though, so perhaps he can elaborate).

In any case, I don't think that IDB should be the first place in the entire
web platform where we expose GC behavior to clients.

-atw

On Tue, Feb 8, 2011 at 4:43 PM, Jonas Sicking  wrote:

> On Tue, Feb 8, 2011 at 3:31 PM, Glenn Maynard  wrote:
> > On Tue, Feb 8, 2011 at 4:01 PM, Jeremy Orlow 
> wrote:
> >>
> >> I talked it over with Darin (Fisher), and he largely agreed with you
> guys.
> >>  I'll file a bug saying that after unload, all IDBDatabases attached to
> that
> >> document should be closed.
> >
> > What happens if a database is open in a page in the back-forward cache?
> > That's incompatible with onunload having side-effects.
> >
> > I know the BF-cache is off-spec, but it's extremely useful and will
> > hopefully find its way into the standard some day, so it'd be nice to
> keep
> > it in mind.
> >
> > I suppose the browser would discard whole pages from the BF-cache on
> demand
> > if required by a setVersion call.
>
> That's exactly what we do in Firefox. Implementations have to be able
> to throw things out of the BF cache on command anyway (since you
> generally want to limit the number of pages living in BF cache, and so
> loading a new page often causes other pages to be thrown out), so it's
> just a matter of calling into the same code here.
>
> / Jonas
>
>

Re: [IndexedDB] setVersion blocked on uncollected garbage IDBDatabases

[Jonas Sicking]

On Tue, Feb 8, 2011 at 3:31 PM, Glenn Maynard  wrote:
> On Tue, Feb 8, 2011 at 4:01 PM, Jeremy Orlow  wrote:
>>
>> I talked it over with Darin (Fisher), and he largely agreed with you guys.
>>  I'll file a bug saying that after unload, all IDBDatabases attached to that
>> document should be closed.
>
> What happens if a database is open in a page in the back-forward cache?
> That's incompatible with onunload having side-effects.
>
> I know the BF-cache is off-spec, but it's extremely useful and will
> hopefully find its way into the standard some day, so it'd be nice to keep
> it in mind.
>
> I suppose the browser would discard whole pages from the BF-cache on demand
> if required by a setVersion call.

That's exactly what we do in Firefox. Implementations have to be able
to throw things out of the BF cache on command anyway (since you
generally want to limit the number of pages living in BF cache, and so
loading a new page often causes other pages to be thrown out), so it's
just a matter of calling into the same code here.

/ Jonas

Re: [IndexedDB] setVersion blocked on uncollected garbage IDBDatabases

[Glenn Maynard]

On Tue, Feb 8, 2011 at 4:01 PM, Jeremy Orlow  wrote:

> I talked it over with Darin (Fisher), and he largely agreed with you guys.
>  I'll file a bug saying that after unload, all IDBDatabases attached to that
> document should be closed.


What happens if a database is open in a page in the back-forward cache?
That's incompatible with onunload having side-effects.

I know the BF-cache is off-spec, but it's extremely useful and will
hopefully find its way into the standard some day, so it'd be nice to keep
it in mind.

I suppose the browser would discard whole pages from the BF-cache on demand
if required by a setVersion call.

-- 
Glenn Maynard

Re: [IndexedDB] setVersion blocked on uncollected garbage IDBDatabases

[Jeremy Orlow]

On Tue, Feb 8, 2011 at 3:31 PM, Glenn Maynard  wrote:

> On Tue, Feb 8, 2011 at 4:01 PM, Jeremy Orlow  wrote:
>
>> I talked it over with Darin (Fisher), and he largely agreed with you guys.
>>  I'll file a bug saying that after unload, all IDBDatabases attached to that
>> document should be closed.
>
>
> What happens if a database is open in a page in the back-forward cache?
> That's incompatible with onunload having side-effects.
>
> I know the BF-cache is off-spec, but it's extremely useful and will
> hopefully find its way into the standard some day, so it'd be nice to keep
> it in mind.
>

As long as the behavior as far as developers can tell matches the spec, you
can do just about whatever you'd like behind the scenes.


> I suppose the browser would discard whole pages from the BF-cache on demand
> if required by a setVersion call.
>

That sounds like it'd more or less work.

J

Re: [IndexedDB] setVersion blocked on uncollected garbage IDBDatabases

[Jeremy Orlow]

I talked it over with Darin (Fisher), and he largely agreed with you guys.
 I'll file a bug saying that after unload, all IDBDatabases attached to that
document should be closed.

J

On Tue, Feb 8, 2011 at 11:51 AM, ben turner  wrote:

> I'm actually fine with keeping the setVersion from proceeding until
> the old database is collected. First, this is probably a bug in the
> web page, and the page should be fixed. Second, the new database that
> is waiting for setVersion to proceed will get an onblocked event, so
> the page should know that something is wrong.
>
> I really don't think this is that big of a deal, and certainly not
> worth changing the opt-in vs. opt-out behavior that we've settled on.
>
> -Ben
>

Re: [IndexedDB] setVersion blocked on uncollected garbage IDBDatabases

[Shawn Wilsher]

On 2/8/2011 11:51 AM, ben turner wrote:

I'm actually fine with keeping the setVersion from proceeding until
the old database is collected. First, this is probably a bug in the
web page, and the page should be fixed. Second, the new database that
is waiting for setVersion to proceed will get an onblocked event, so
the page should know that something is wrong.

I really don't think this is that big of a deal, and certainly not
worth changing the opt-in vs. opt-out behavior that we've settled on.

Agreed.

Cheers,

Shawn



smime.p7s
Description: S/MIME Cryptographic Signature

Re: [IndexedDB] setVersion blocked on uncollected garbage IDBDatabases

[Jonas Sicking]

2011/2/8 Jeremy Orlow :
> On Tue, Feb 8, 2011 at 10:36 AM, Jonas Sicking  wrote:
>>
>> On Tue, Feb 8, 2011 at 9:26 AM, Jeremy Orlow  wrote:
>> > On Tue, Feb 8, 2011 at 3:36 AM, João Eiras  wrote:
>> >>
>> >> > Unless by "certain GC behavior" mean
>> >>
>> >> I referred to
>> >>
>> >> # The only solution I can think of is to require (or recommend) that
>> >> implementations run the garbage collector
>> >>
>> >> The GC is transparent and a spec cannot expect that it runs at
>> >> specific times or demand it.
>> >
>> > Yeah, Jonas.  We can't reasonably expect any behavior from the garbage
>> > collectors.  I can't think of any other precedent for this.  And as
>> > collectors become more complicated, doing a gc that catches _every_
>> > piece of
>> > garbage is becoming harder or even impossible (not aware of any GC's
>> > where
>> > it is "impossible" in specific cases, but it wouldn't surprise me).  The
>> > v8
>> > guys simply won't let us do this.  :-)
>> > And saying that at the worst case your setVersion transaction will stall
>> > possibly forever just doesn't seem like a good solution either.
>>
>> Huh? It seems like a very strange GC implementation that not only
>> doesn't allow you to do a full search for garbage, even
>> asynchronously, but then can't even guarantee that a given object will
>> eventually be freed.
>>
>> I'm all for not relying on GC behavior, but not even relying on it to
>> collect garbage? That seems a bit extreme to me. It's also not a GC
>> strategy I've ever heard of, so yes, it would surprise me if there are
>> GC strategies out there that doesn't free up objects sooner or later.
>> How is that different from a GC strategy that is simply leaking?
>
> I meant that it wouldn't be able to collect on demand like that.  Or that it
> would at least be prohibitively expensive.

That I can understand, which is why I said "or recommend". I suspect
that in firefox it will get harder to do a synchronous GC, but a
asynchronous one seems like it will remain possible. But even if you
don't do anything, we'll always end up running GC at *some* point.

>> > What if we made the default for onsetversion to be calling close?  I.e.
>> > instead of the close behavior being opt-out, it'd be opt-in?  I know we
>> > made
>> > a conscious decision originally of it being opt-in, but I don't see how
>> > that'll work.
>>
>> This flips the model completely on its head since you're now forced to
>> implement a more advanced version upgrade strategies or suffer your
>> pages breaking.
>>
>> The worst case scenario isn't even that bad IMHO. Say that you have a
>> GC strategy which truly never frees the unreferenced DB object. That
>> is no worse than the page simply holding a reference DB object and
>> making the version upgrade wait for the user to close old tabs.
>> Something that is bound to happen anyway if authors take the simple
>> path of not listening to "blocked" or "versionchange" events.
>
> You're assuming implementations have one heap per tab.

Not at all. I'm just assuming GC will run in all heaps at some point.

> We could cheat and kill things when the document goes away, I suppose.
>  Still not very excited about that tho.  (Especially since even those
> semantics can be a bit tricky, at least in WebKit.)

Wait, you're worried that you won't even run GC once the user leaves
the page? I thought that in that case chrome just killed the process.
Surely that will close any open databases?

/ Jonas

Re: [IndexedDB] setVersion blocked on uncollected garbage IDBDatabases

[ben turner]

I'm actually fine with keeping the setVersion from proceeding until
the old database is collected. First, this is probably a bug in the
web page, and the page should be fixed. Second, the new database that
is waiting for setVersion to proceed will get an onblocked event, so
the page should know that something is wrong.

I really don't think this is that big of a deal, and certainly not
worth changing the opt-in vs. opt-out behavior that we've settled on.

-Ben

Re: [IndexedDB] setVersion blocked on uncollected garbage IDBDatabases

[Jeremy Orlow]

On Tue, Feb 8, 2011 at 10:36 AM, Jonas Sicking  wrote:

> On Tue, Feb 8, 2011 at 9:26 AM, Jeremy Orlow  wrote:
> > On Tue, Feb 8, 2011 at 3:36 AM, João Eiras  wrote:
> >>
> >> > Unless by "certain GC behavior" mean
> >>
> >> I referred to
> >>
> >> # The only solution I can think of is to require (or recommend) that
> >> implementations run the garbage collector
> >>
> >> The GC is transparent and a spec cannot expect that it runs at
> >> specific times or demand it.
> >
> > Yeah, Jonas.  We can't reasonably expect any behavior from the garbage
> > collectors.  I can't think of any other precedent for this.  And as
> > collectors become more complicated, doing a gc that catches _every_ piece
> of
> > garbage is becoming harder or even impossible (not aware of any GC's
> where
> > it is "impossible" in specific cases, but it wouldn't surprise me).  The
> v8
> > guys simply won't let us do this.  :-)
> > And saying that at the worst case your setVersion transaction will stall
> > possibly forever just doesn't seem like a good solution either.
>
> Huh? It seems like a very strange GC implementation that not only
> doesn't allow you to do a full search for garbage, even
> asynchronously, but then can't even guarantee that a given object will
> eventually be freed.
>
> I'm all for not relying on GC behavior, but not even relying on it to
> collect garbage? That seems a bit extreme to me. It's also not a GC
> strategy I've ever heard of, so yes, it would surprise me if there are
> GC strategies out there that doesn't free up objects sooner or later.
> How is that different from a GC strategy that is simply leaking?
>

I meant that it wouldn't be able to collect on demand like that.  Or that it
would at least be prohibitively expensive.


> > What if we made the default for onsetversion to be calling close?  I.e.
> > instead of the close behavior being opt-out, it'd be opt-in?  I know we
> made
> > a conscious decision originally of it being opt-in, but I don't see how
> > that'll work.
>
> This flips the model completely on its head since you're now forced to
> implement a more advanced version upgrade strategies or suffer your
> pages breaking.
>
> The worst case scenario isn't even that bad IMHO. Say that you have a
> GC strategy which truly never frees the unreferenced DB object. That
> is no worse than the page simply holding a reference DB object and
> making the version upgrade wait for the user to close old tabs.
> Something that is bound to happen anyway if authors take the simple
> path of not listening to "blocked" or "versionchange" events.
>

You're assuming implementations have one heap per tab.

We could cheat and kill things when the document goes away, I suppose.
 Still not very excited about that tho.  (Especially since even those
semantics can be a bit tricky, at least in WebKit.)

I'd like to hear other peoples' thoughts on this.

J

Re: [IndexedDB] setVersion blocked on uncollected garbage IDBDatabases

[Jonas Sicking]

On Tue, Feb 8, 2011 at 9:26 AM, Jeremy Orlow  wrote:
> On Tue, Feb 8, 2011 at 3:36 AM, João Eiras  wrote:
>>
>> > Unless by "certain GC behavior" mean
>>
>> I referred to
>>
>> # The only solution I can think of is to require (or recommend) that
>> implementations run the garbage collector
>>
>> The GC is transparent and a spec cannot expect that it runs at
>> specific times or demand it.
>
> Yeah, Jonas.  We can't reasonably expect any behavior from the garbage
> collectors.  I can't think of any other precedent for this.  And as
> collectors become more complicated, doing a gc that catches _every_ piece of
> garbage is becoming harder or even impossible (not aware of any GC's where
> it is "impossible" in specific cases, but it wouldn't surprise me).  The v8
> guys simply won't let us do this.  :-)
> And saying that at the worst case your setVersion transaction will stall
> possibly forever just doesn't seem like a good solution either.

Huh? It seems like a very strange GC implementation that not only
doesn't allow you to do a full search for garbage, even
asynchronously, but then can't even guarantee that a given object will
eventually be freed.

I'm all for not relying on GC behavior, but not even relying on it to
collect garbage? That seems a bit extreme to me. It's also not a GC
strategy I've ever heard of, so yes, it would surprise me if there are
GC strategies out there that doesn't free up objects sooner or later.
How is that different from a GC strategy that is simply leaking?

> What if we made the default for onsetversion to be calling close?  I.e.
> instead of the close behavior being opt-out, it'd be opt-in?  I know we made
> a conscious decision originally of it being opt-in, but I don't see how
> that'll work.

This flips the model completely on its head since you're now forced to
implement a more advanced version upgrade strategies or suffer your
pages breaking.

The worst case scenario isn't even that bad IMHO. Say that you have a
GC strategy which truly never frees the unreferenced DB object. That
is no worse than the page simply holding a reference DB object and
making the version upgrade wait for the user to close old tabs.
Something that is bound to happen anyway if authors take the simple
path of not listening to "blocked" or "versionchange" events.

/ Jonas

Re: [IndexedDB] setVersion blocked on uncollected garbage IDBDatabases

[Jeremy Orlow]

On Tue, Feb 8, 2011 at 3:36 AM, João Eiras  wrote:

> > Unless by "certain GC behavior" mean
>
> I referred to
>
> # The only solution I can think of is to require (or recommend) that
> implementations run the garbage collector
>
> The GC is transparent and a spec cannot expect that it runs at
> specific times or demand it.
>

Yeah, Jonas.  We can't reasonably expect any behavior from the garbage
collectors.  I can't think of any other precedent for this.  And as
collectors become more complicated, doing a gc that catches _every_ piece of
garbage is becoming harder or even impossible (not aware of any GC's where
it is "impossible" in specific cases, but it wouldn't surprise me).  The v8
guys simply won't let us do this.  :-)

And saying that at the worst case your setVersion transaction will stall
possibly forever just doesn't seem like a good solution either.

What if we made the default for onsetversion to be calling close?  I.e.
instead of the close behavior being opt-out, it'd be opt-in?  I know we made
a conscious decision originally of it being opt-in, but I don't see how
that'll work.

J

Re: [IndexedDB] setVersion blocked on uncollected garbage IDBDatabases

[João Eiras]

> Unless by "certain GC behavior" mean

I referred to

# The only solution I can think of is to require (or recommend) that
implementations run the garbage collector

The GC is transparent and a spec cannot expect that it runs at
specific times or demand it.

Re: [IndexedDB] setVersion blocked on uncollected garbage IDBDatabases

[Jonas Sicking]

On Tue, Feb 8, 2011 at 3:26 AM, Jonas Sicking  wrote:
> On Tue, Feb 8, 2011 at 3:20 AM, João Eiras  wrote:
>> On Tue, Feb 8, 2011 at 10:52 AM, Jonas Sicking  wrote:
>>> On Tue, Feb 8, 2011 at 2:45 AM, João Eiras  wrote:
> The only solution I can think of is to require (or recommend) that
> implementations run the garbage collector in a context after firing
> the "versionchange" event if the database still isn't closed. This
> should be a rare occurrence simply because setVersion should be rare.

 That would be a hack in the implementation and a hack in the spec.
>>>
>>> Why? And what are you suggesting instead?
>>
>> Don't have a solution, but expecting certain GC behavior on a
>> specification is far from sane.
>
> No one is suggesting that. I'll also point out that even if the
> implementation doesn't GC the object immediately things will still
> work fine. You'll just receive the "success" event later, and possibly
> also a "blocked" event while waiting. As I'm sure you already know.

Unless by "certain GC behavior" mean "free objects that are no longer
used". If that is indeed what you meant then we'll have to agree to
disagree what is "far from sane".

/ Jonas

Re: [IndexedDB] setVersion blocked on uncollected garbage IDBDatabases

[Jonas Sicking]

On Tue, Feb 8, 2011 at 3:20 AM, João Eiras  wrote:
> On Tue, Feb 8, 2011 at 10:52 AM, Jonas Sicking  wrote:
>> On Tue, Feb 8, 2011 at 2:45 AM, João Eiras  wrote:
 The only solution I can think of is to require (or recommend) that
 implementations run the garbage collector in a context after firing
 the "versionchange" event if the database still isn't closed. This
 should be a rare occurrence simply because setVersion should be rare.
>>>
>>> That would be a hack in the implementation and a hack in the spec.
>>
>> Why? And what are you suggesting instead?
>
> Don't have a solution, but expecting certain GC behavior on a
> specification is far from sane.

No one is suggesting that. I'll also point out that even if the
implementation doesn't GC the object immediately things will still
work fine. You'll just receive the "success" event later, and possibly
also a "blocked" event while waiting. As I'm sure you already know.

/ Jonas

Re: [IndexedDB] setVersion blocked on uncollected garbage IDBDatabases

[João Eiras]

On Tue, Feb 8, 2011 at 10:52 AM, Jonas Sicking  wrote:
> On Tue, Feb 8, 2011 at 2:45 AM, João Eiras  wrote:
>>> The only solution I can think of is to require (or recommend) that
>>> implementations run the garbage collector in a context after firing
>>> the "versionchange" event if the database still isn't closed. This
>>> should be a rare occurrence simply because setVersion should be rare.
>>
>> That would be a hack in the implementation and a hack in the spec.
>
> Why? And what are you suggesting instead?

Don't have a solution, but expecting certain GC behavior on a
specification is far from sane.

Re: [IndexedDB] setVersion blocked on uncollected garbage IDBDatabases

[Jonas Sicking]

On Tue, Feb 8, 2011 at 2:45 AM, João Eiras  wrote:
>> The only solution I can think of is to require (or recommend) that
>> implementations run the garbage collector in a context after firing
>> the "versionchange" event if the database still isn't closed. This
>> should be a rare occurrence simply because setVersion should be rare.
>
> That would be a hack in the implementation and a hack in the spec.

Why? And what are you suggesting instead? The above email is very hard
to act on.

/ Jonas

Re: [IndexedDB] setVersion blocked on uncollected garbage IDBDatabases

[João Eiras]

> The only solution I can think of is to require (or recommend) that
> implementations run the garbage collector in a context after firing
> the "versionchange" event if the database still isn't closed. This
> should be a rare occurrence simply because setVersion should be rare.
>

That would be a hack in the implementation and a hack in the spec.

Re: [IndexedDB] setVersion blocked on uncollected garbage IDBDatabases

[Jonas Sicking]

On Mon, Feb 7, 2011 at 8:09 PM, Jeremy Orlow  wrote:
> We're currently implementing the onblocked/setVersion semantics and ran into
> an interesting problem: if you don't call .close() on a database and simply
> expect it to be collected, then you ever being able to run a setVersion
> transaction is at the mercy of the garbage collecter doing a collection.
>  Otherwise implementations will assume the database is still open...right?
> If so, this seems bad.  But I can't think of any way to get around it.
>  Thoughts?

Hmm.. never thought about this problem. But yes, I suspect this issue
comes up in our implementation.

The only solution I can think of is to require (or recommend) that
implementations run the garbage collector in a context after firing
the "versionchange" event if the database still isn't closed. This
should be a rare occurrence simply because setVersion should be rare.

/ Jonas