Bug 30929 – Add function to allow Database objects to trim memory usage

• Status: RESOLVED
• Resolution: WONTFIX
• Component: WebCore Misc.

[Peter Kasting]

Databases have an associated sqlite database, which has an in-memory page cache.  Much like we have ways of clearing other caches when we need to trim memory footprint, it'd be useful to have a hook to do this with the sqlite page cache.

It turns out there is a function in sqlite, sqlite3_release_memory(), to do precisely this.  You need to compile sqlite with a particular define to use it, but we can at least expose a hook to this so platforms have the ability to make use of it.

I intend to plumb this up to the embedder in Chromium as soon as the Chromium-specific bits of the database tracker get in (bug 30701).

Simple patch coming shortly.

[Peter Kasting]

Created attachment 42146 [details]
patch v1

[Peter Kasting]

Created attachment 42155 [details]
patch v2

Now taking action on the correct thread.

[Michael Nordman]

It's intriguing that this api call does not take a sqlite3* parameter. Its seems like this doesn't need to be called for each active database thread or for each opened database... looks like this is a one-shot call that whacks at all active databases.

Apparently there's a noticeable run-time cost to enabling the MEM_MANAGEMENT flag associated with this API call especially when THREADING is also enabled.

http://www.mail-archive.com/[e-mail]/msg30586.html

Given that, not sure this is a good answer.

[Peter Kasting]

You're citing the wrong thread.  You should look at the thread from me two days ago instead of the thread from two years ago, or come talk to me or shess about it.  In short, there is absolutely no overhead or penalty for enabling this flag in the current codebase.

[Michael Nordman]

(In reply to comment #4)
> You're citing the wrong thread.  You should look at the thread from me two days
> ago instead of the thread from two years ago, or come talk to me or shess about
> it.  In short, there is absolutely no overhead or penalty for enabling this
> flag in the current codebase.

Glad to hear it. Can you forward the thread from two-days ago to me, I'm not finding anything in my inbox?

I did find this in my mail archive...

2007 Sep 04 (3.5.0) alpha
The sqlite3_release_memory(), sqlite3_soft_heap_limit(), and sqlite3_enable_shared_cache() interfaces now work cross all threads in the process, not just the single thread in which they are invoked.

... alas also two years old :)

So i guess my question is, do we have to call this function on each thread that's using SQLite independently, or is one shot all it takes?

[Peter Kasting]

How many times and places we should call this is a very good question.  I need to look at this a little more.

I can't forward you an entire thread (sorry, this isn't Wave), but you're welcome to come chat with me and Scott, or just grep the sqlite codebase for SQLITE_ENABLE_MEMORY_MANAGEMENT -- it's only present in a few places so it's pretty easy to see what it does.

[Michael Nordman]

(In reply to comment #6)
> How many times and places we should call this is a very good question.  I need
> to look at this a little more.
> 
> I can't forward you an entire thread (sorry, this isn't Wave), but you're
> welcome to come chat with me and Scott, or just grep the sqlite codebase for
> SQLITE_ENABLE_MEMORY_MANAGEMENT -- it's only present in a few places so it's
> pretty easy to see what it does.

I see... this comment is telling...
"Memory in use by any SQLite pager allocated by the current thread may be sqlite3_free()ed."
... looks like once per thread on which SQLite is used?

#ifdef SQLITE_ENABLE_MEMORY_MANAGEMENT
/*
** This function is called to free superfluous dynamically allocated memory
** held by the pager system. Memory in use by any SQLite pager allocated
** by the current thread may be sqlite3_free()ed.
**
** nReq is the number of bytes of memory required. Once this much has
** been released, the function returns. The return value is the total number 
** of bytes of memory released.
*/
int sqlite3PcacheReleaseMemory(int nReq){
  int nFree = 0;
  if( pcache1.pStart==0 ){
    PgHdr1 *p;
    pcache1EnterMutex();
    while( (nReq<0 || nFree<nReq) && (p=pcache1.pLruTail) ){
      nFree += sqlite3MallocSize(PGHDR1_TO_PAGE(p));
      pcache1PinPage(p);
      pcache1RemoveFromHash(p);
      pcache1FreePage(p);
    }
    pcache1LeaveMutex();
  }
  return nFree;
}
#endif /* SQLITE_ENABLE_MEMORY_MANAGEMENT */

[Peter Kasting]

I'm closing this.  After investigation, Scott and I are convinced that the comments lie, and this is actually per-process rather than per-thread.  Therefore, I can simply implement this directly in Chromium code for the renderer process without needing to plumb anything in WebCore.

[Michael Nordman]

(In reply to comment #8)
> I'm closing this.  After investigation, Scott and I are convinced that the
> comments lie, and this is actually per-process rather than per-thread. 
> Therefore, I can simply implement this directly in Chromium code for the
> renderer process without needing to plumb anything in WebCore.

Sweet, so we should be able to remove the collection of 'openDatabases' from chromium's tracker out for review in bug 30701.

[Eric Seidel (no email)]

Comment on attachment 42155 [details]
patch v2

Clearing review since the bug is closed.