+2007-07-17 Antti <antti@apple.com>
+
+ Reviewed by home-bradee.
+
+ <rdar://problem/5336372>
+ Icon database uses too much memory
+
+ XRaying Safari startup memory consumption revealed that icon database is eating quite
+ a bit of RAM if Icon.db is large (which it probably is if it has been in use for a while,
+ mine used for getting figures below was 2.6MB).
+
+ Note that the wins are less impressive with smaller Icon.db.
+
+ This patch addresses three separate issues
+
+ - SQLite fails to free the memory used by temporary tables. Icon database uses a temporary table
+ on startup for pruning unused page urls. This wastes around 1MB. Addressed by rewriting
+ pruning so it does not need a temporary table. The new method is also quite a bit faster speeding
+ up Safari launch time by around 100ms
+ - SQLite has it's own memory cache limited by default to 3MB. Icon database does not really need that much.
+ Dropped the cache size to 300kB saving ~1MB on startup.
+ Smaller cache slows down startup by ~30ms (more than compensated by faster pruning above)
+ - Don't populate m_pageURLToIconURLMap with all urls from database on startup, instead let it get populated
+ when urls are accessed (user opens history menu for example). This shouldn't have any real performance impact
+ as the accesses are icon loads that need to hit the database anyway. This saves ~700kB.
+
+ All in all with this Icon.db these changes reduce allocated memory by around 2.7MB on startup. Release build
+ Safari RPRVT (empty start page) goes from 12.4MB to 10.4MB (TCMalloc pooling probably explaining why the win
+ looks bit smaller here).
+
+ * loader/icon/IconDatabase.cpp:
+ (WebCore::IconDatabase::IconDatabase):
+ (WebCore::IconDatabase::open):
+ (WebCore::IconDatabase::deleteAllPreparedStatements):
+ (WebCore::IconDatabase::retainIconForPageURL):
+ (WebCore::IconDatabase::releaseIconForPageURL):
+ (WebCore::IconDatabase::establishIconIDForIconURL):
+ (WebCore::IconDatabase::pruneUnretainedIconsOnStartup):
+ * loader/icon/IconDatabase.h:
+
2007-07-17 Darin Adler <darin@apple.com>
Reviewed by Mitz.
, m_startupTimer(this, &IconDatabase::pruneUnretainedIconsOnStartup)
, m_updateTimer(this, &IconDatabase::updateDatabase)
, m_initialPruningComplete(false)
- , m_initialPruningTransaction(0)
- , m_preparedPageRetainInsertStatement(0)
, m_imported(false)
, m_isImportedSet(false)
{
createDatabaseTables(m_mainDB);
}
- m_initialPruningTransaction = new SQLTransaction(m_mainDB);
- // We're going to track an icon's retain count in a temp table in memory so we can cross reference it to to the on disk tables
- bool result;
- result = m_mainDB.executeCommand("CREATE TEMP TABLE PageRetain (url TEXT);");
- // Creating an in-memory temp table should never, ever, ever fail
- ASSERT(result);
-
// These are actually two different SQLite config options - not my fault they are named confusingly ;)
m_mainDB.setSynchronous(SQLDatabase::SyncOff);
m_mainDB.setFullsync(false);
-
- m_initialPruningTransaction->begin();
+
+ // Reduce sqlite RAM cache size from default 2000 pages (~1.5kB per page). 3MB of cache for icon database is overkill
+ if (!SQLStatement(m_mainDB, "PRAGMA cache_size = 200;").executeCommand())
+ LOG_ERROR("SQLite database could not set cache_size");
// Open the in-memory table for private browsing
if (!m_privateBrowsingDB.open(":memory:"))
// B - Resetting the DB via removeAllIcons() - in this case, you *don't* want to sync, because it would be a waste of time
void IconDatabase::deleteAllPreparedStatements(bool withSync)
{
- // Must wipe the initial retain statement before the initial transaction
- delete m_preparedPageRetainInsertStatement;
- m_preparedPageRetainInsertStatement = 0;
- delete m_initialPruningTransaction;
- m_initialPruningTransaction = 0;
-
// Sync, if desired
if (withSync)
syncDatabase();
if (!(retainCount = m_pageURLToRetainCount.get(pageURL))) {
m_pageURLToRetainCount.set(pageURL, 1);
- // If we haven't done initial pruning, we store this retain record in the temporary in-memory table
- // Note we only keep the URL in the temporary table, not the full retain count, because for pruning-considerations
- // we only care *if* a pageURL is retained - not the full count. This call to retainIconForPageURL incremented the PageURL's
- // retain count from 0 to 1 therefore we may store it in the temporary table
- // Also, if we haven't done pruning yet, we want to avoid any pageURL->iconURL lookups and the pageURLsPendingDeletion is moot,
+ // If we haven't done pruning yet, we want to avoid any pageURL->iconURL lookups and the pageURLsPendingDeletion is moot,
// so we bail here and skip those steps
- if (!m_initialPruningComplete) {
- String escapedPageURL = escapeSQLString(pageURL);
- if (!m_preparedPageRetainInsertStatement) {
- m_preparedPageRetainInsertStatement = new SQLStatement(m_mainDB, "INSERT INTO PageRetain VALUES (?);");
- m_preparedPageRetainInsertStatement->prepare();
- }
- m_preparedPageRetainInsertStatement->reset();
- m_preparedPageRetainInsertStatement->bindText16(1, pageURL);
- if (m_preparedPageRetainInsertStatement->step() != SQLResultDone)
- LOG_ERROR("Failed to record icon retention in temporary table for IconURL %s", pageURL.ascii().data());
+ if (!m_initialPruningComplete)
return;
- }
// If this pageURL is marked for deletion, bring it back from the brink
m_pageURLsPendingDeletion.remove(pageURL);
// Otherwise, remove all record of the retain count
m_pageURLToRetainCount.remove(pageURL);
- // If we haven't done initial pruning, we remove this retain record from the temporary in-memory table
- // Note we only keep the URL in the temporary table, not the full retain count, because for pruning-considerations
- // we only care *if* a pageURL is retained - not the full count. This call to releaseIconForPageURL decremented the PageURL's
- // retain count from 1 to 0 therefore we may remove it from the temporary table
- // Also, if we haven't done pruning yet, we want to avoid any pageURL->iconURL lookups and the pageURLsPendingDeletion is moot,
+ // If we haven't done pruning yet, we want to avoid any pageURL->iconURL lookups and the pageURLsPendingDeletion is moot,
// so we bail here and skip those steps
- if (!m_initialPruningComplete) {
- String escapedPageURL = escapeSQLString(pageURL);
- if (!m_mainDB.executeCommand("DELETE FROM PageRetain WHERE url='" + escapedPageURL + "';"))
- LOG_ERROR("Failed to delete record of icon retention from temporary table for IconURL %s", pageURL.ascii().data());
+ if (!m_initialPruningComplete)
return;
- }
// Then mark this pageURL for deletion
}
int64_t IconDatabase::establishIconIDForIconURL(SQLDatabase& db, const String& iconURL, bool createIfNecessary)
-{
- String escapedIconURL = escapeSQLString(iconURL);
-
+{
// Get the iconID thats already in this database and return it - or return 0 if we're read-only
int64_t iconID = getIconIDForIconURLQuery(db, iconURL);
if (iconID || !createIfNecessary)
// rdar://4690949 - Need to prune unretained iconURLs here, then prune out all pageURLs that reference
// nonexistent icons
- // Finalize the PageRetain statement
- delete m_preparedPageRetainInsertStatement;
- m_preparedPageRetainInsertStatement = 0;
+ SQLTransaction pruningTransaction(m_mainDB);
+ pruningTransaction.begin();
- // Commit all of the PageRetains and start a new transaction for the pruning dirty-work
- m_initialPruningTransaction->commit();
- m_initialPruningTransaction->begin();
+ // Wipe all PageURLs that aren't retained
+ // Temporary tables in sqlite seem to lose memory permanently so do this by hand instead. This is faster too.
- // Then wipe all PageURLs and Icons that aren't retained
- if (!m_mainDB.executeCommand("DELETE FROM PageURL WHERE PageURL.url NOT IN (SELECT url FROM PageRetain);") ||
- !m_mainDB.executeCommand("DELETE FROM Icon WHERE Icon.iconID NOT IN (SELECT iconID FROM PageURL);") ||
- !m_mainDB.executeCommand("DROP TABLE PageRetain;"))
- LOG_ERROR("Failed to execute SQL to prune unretained pages and icons from the on-disk tables");
+ HashMap<String, int64_t> pageUrlsToDelete;
+ // First get the known PageURLs from the db
+ int result;
+ SQLStatement pageSQL(m_mainDB, "SELECT url, iconID FROM PageURL");
+ pageSQL.prepare();
+ while((result = pageSQL.step()) == SQLResultRow)
+ pageUrlsToDelete.set(pageSQL.getColumnText16(0), pageSQL.getColumnInt64(1));
+ pageSQL.finalize();
+ if (result != SQLResultDone)
+ LOG_ERROR("Error reading PageURL table from on-disk DB");
+
+ // Remove all urls we actually want to keep from the hash
+ HashMap<String, int>::iterator endit = m_pageURLToRetainCount.end();
+ for (HashMap<String, int>::iterator it = m_pageURLToRetainCount.begin(); it != endit; ++it)
+ pageUrlsToDelete.remove(it->first);
+
+ // Delete the rest, if any
+ if (pageUrlsToDelete.size()) {
+ SQLStatement pageDeleteSQL(m_mainDB, "DELETE FROM PageURL WHERE iconID = (?)");
+ pageDeleteSQL.prepare();
+ HashMap<String, int64_t>::iterator endit = pageUrlsToDelete.end();
+ for (HashMap<String, int64_t>::iterator it = pageUrlsToDelete.begin(); it != endit; ++it) {
+ LOG(IconDatabase, "Deleting %s from PageURL table\n", it->first.latin1().data());
+ pageDeleteSQL.bindInt64(1, it->second);
+ if (pageDeleteSQL.step() != SQLResultDone)
+ LOG_ERROR("Unable to delete %s from PageURL table", it->first.latin1().data());
+ pageDeleteSQL.reset();
+ }
+ pageDeleteSQL.finalize();
+ }
+
+ // Wipe Icons that aren't retained
+ if (!m_mainDB.executeCommand("DELETE FROM Icon WHERE Icon.iconID NOT IN (SELECT iconID FROM PageURL);"))
+ LOG_ERROR("Failed to execute SQL to prune unretained icons from the on-disk tables");
// Since we lazily retained the pageURLs without getting the iconURLs or retaining the iconURLs,
// we need to do that now
- // We now should be in a spiffy situation where we know every single pageURL left in the DB is retained, so we are interested
- // in the iconURLs for all remaining pageURLs
- // So we can simply add all the remaining mappings, and retain each pageURL's icon once
-
SQLStatement sql(m_mainDB, "SELECT PageURL.url, Icon.url FROM PageURL INNER JOIN Icon ON PageURL.iconID=Icon.iconID");
sql.prepare();
- int result;
while((result = sql.step()) == SQLResultRow) {
String iconURL = sql.getColumnText16(1);
- m_pageURLToIconURLMap.set(sql.getColumnText16(0), iconURL);
retainIconURL(iconURL);
LOG(IconDatabase, "Found a PageURL that mapped to %s", iconURL.ascii().data());
}
LOG_ERROR("Error reading PageURL->IconURL mappings from on-disk DB");
sql.finalize();
- // Commit the transaction and do some cleanup
- m_initialPruningTransaction->commit();
- delete m_initialPruningTransaction;
- m_initialPruningTransaction = 0;
+ pruningTransaction.commit();
m_initialPruningComplete = true;
// Handle dangling PageURLs, if any
checkForDanglingPageURLs(true);
-
+
#ifndef NDEBUG
timestamp = currentTime() - timestamp;
if (timestamp <= 1.0)
git://git.webkit.org / WebKit-https.git / commitdiff