Reviewed by Alice
Added a truth value to setIconURLForPageURL so WebKit can avoid sending a notification
This is a win on the iBench
* bridge/mac/WebCoreIconDatabaseBridge.h:
* bridge/mac/WebCoreIconDatabaseBridge.mm:
(-[WebCoreIconDatabaseBridge _setIconURL:forPageURL:]):
* loader/icon/IconDatabase.cpp:
(WebCore::IconDatabase::setIconURLForPageURL):
* loader/icon/IconDatabase.h:
WebKit:
Reviewed by Alice
Added a truth value check for to setIconURL:forURL so WebKit can avoid sending a notification
This is a win on the iBench
* Misc/WebIconDatabase.m:
(-[WebIconDatabase _setIconURL:forURL:]):
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@16110
268f45cc-cd09-0410-ab3c-
d52691b4dbfc
+2006-08-29 Brady Eidson <beidson@apple.com>
+
+ Reviewed by Alice
+
+ Added a truth value to setIconURLForPageURL so WebKit can avoid sending a notification
+ This is a win on the iBench
+
+ * bridge/mac/WebCoreIconDatabaseBridge.h:
+ * bridge/mac/WebCoreIconDatabaseBridge.mm:
+ (-[WebCoreIconDatabaseBridge _setIconURL:forPageURL:]):
+ * loader/icon/IconDatabase.cpp:
+ (WebCore::IconDatabase::setIconURLForPageURL):
+ * loader/icon/IconDatabase.h:
+
2006-08-29 Alice Liu <alice.liu@apple.com>
Reviewed by Brady.
- (void)_setIconData:(NSData *)data forIconURL:(NSString *)iconURL;
- (void)_setHaveNoIconForIconURL:(NSString *)iconURL;
-- (void)_setIconURL:(NSString *)iconURL forPageURL:(NSString *)pageURL;
+- (BOOL)_setIconURL:(NSString *)iconURL forPageURL:(NSString *)pageURL;
- (BOOL)_hasEntryForIconURL:(NSString *)iconURL;
- (BOOL)_isEmpty;
_iconDB->setHaveNoIconForIconURL(String(iconURL));
}
-- (void)_setIconURL:(NSString *)iconURL forPageURL:(NSString *)pageURL
+- (BOOL)_setIconURL:(NSString *)iconURL forPageURL:(NSString *)pageURL
{
ASSERT(_iconDB);
ASSERT(iconURL);
ASSERT(pageURL);
- _iconDB->setIconURLForPageURL(String(iconURL), String(pageURL));
+ return _iconDB->setIconURLForPageURL(String(iconURL), String(pageURL));
}
- (BOOL)_hasEntryForIconURL:(NSString *)iconURL
setIconDataForIconURL(0, 0, iconURL);
}
-void IconDatabase::setIconURLForPageURL(const String& iconURL, const String& pageURL)
+bool IconDatabase::setIconURLForPageURL(const String& iconURL, const String& pageURL)
{
ASSERT(!iconURL.isEmpty());
ASSERT(!pageURL.isEmpty());
// If the urls already map to each other, bail.
// This happens surprisingly often, and seems to cream iBench performance
if (m_pageURLToIconURLMap.get(pageURL) == iconURL)
- return;
+ return false;
// If this pageURL is retained, we have some work to do on the IconURL retain counts
if (m_pageURLToRetainCount.contains(pageURL)) {
// Then start the timer to commit this change - or further delay the timer if it
// was already started
m_updateTimer.startOneShot(updateTimerDelay);
+
+ return true;
}
void IconDatabase::setIconURLForPageURLInDatabase(const String& iconURL, const String& pageURL)
bool isIconExpiredForIconURL(const String&);
- // TODO - The following 3 methods were considered private in WebKit - analyze the impact of making them
- // public here in WebCore - I don't see any real badness with doing that... after all if Chuck Norris wants to muck
- // around with the icons in his database, he's going to anyway
void setIconDataForIconURL(const void* data, int size, const String&);
void setHaveNoIconForIconURL(const String&);
- void setIconURLForPageURL(const String& iconURL, const String& pageURL);
+
+ // Returns true if the set actually took place, false if the mapping already existed
+ bool setIconURLForPageURL(const String& iconURL, const String& pageURL);
static const String& defaultDatabaseFilename();
+2006-08-29 Brady Eidson <beidson@apple.com>
+
+ Reviewed by Alice
+
+ Added a truth value check for to setIconURL:forURL so WebKit can avoid sending a notification
+ This is a win on the iBench
+
+ * Misc/WebIconDatabase.m:
+ (-[WebIconDatabase _setIconURL:forURL:]):
+
2006-08-29 Brady Eidson <beidson@apple.com>
Reviewed by Tim Hatchers rubber stamp
ASSERT(URL);
ASSERT([self _isEnabled]);
- // FIXME - The following comment is a holdover from the old icon DB, which handled missing icons
- // differently than the new db. It would return early if the icon is in the negative cache,
- // avoiding the notification. We should explore and see if a similar optimization can take place-
- // If the icon is in the negative cache (ie, there is no icon), avoid the
- // work of delivering a notification for it or saving it to disk. This is a significant
- // win on the iBench HTML test.
-
- // FIXME - The following comment is also a holdover - if the iconURL->pageURL mapping was already the
- // same, the notification would again be avoided - we should try to do this, too.
- // Don't do any work if the icon URL is already bound to the site URL
-
- // A possible solution for both of these is to have the bridge method return a BOOL saying "Yes, notify" or
- // "no, don't bother notifying"
-
- [_private->databaseBridge _setIconURL:iconURL forPageURL:URL];
- [self _sendNotificationForURL:URL];
+ // If this iconURL already maps to this pageURL, don't bother sending the notification
+ // The WebCore::IconDatabase returns TRUE if we should send the notification, and false if we shouldn't.
+ // This is a measurable win on the iBench - about 1% worth on average
+ if ([_private->databaseBridge _setIconURL:iconURL forPageURL:URL])
+ [self _sendNotificationForURL:URL];
}
- (BOOL)_hasEntryForIconURL:(NSString *)iconURL;