⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 117501 in webkit


Ignore:
Timestamp:
May 17, 2012, 2:49:43 PM (14 years ago)
Author:
kling@webkit.org
Message:

IconDatabase: Move icon retain/release off of the main thread.
<​http://webkit.org/b/85799>
<rdar://problem/9507113>

Reviewed by Brady Eidson.

Batch up the retain/release operations and execute them as part of the sync thread loop.
The batch execution is guarded by a new mutex (m_urlsToRetainOrReleaseLock.)
This avoids blocking the main thread on m_urlAndIconLock for basic retain/release.

There is one exception; if there are pending retain/release operations in synchronousIconForPageURL,
it will acquire the lock and flush the operations.

There should be no behavior change, this is only meant to reduce lock contention.

  • loader/icon/PageURLRecord.h:

(WebCore::PageURLRecord::retain):
(WebCore::PageURLRecord::release):

Added a 'count' argument to these so we can batch up the operations in IconDatabase.

  • loader/icon/IconDatabase.h:
  • loader/icon/IconDatabase.cpp:

(WebCore::IconDatabase::performScheduleOrDeferSyncTimer):
(WebCore::IconDatabase::performScheduleOrDeferSyncTimerOnMainThread):
(WebCore::IconDatabase::scheduleOrDeferSyncTimer):

Perform the the timer scheduling on the main thread as it can be done on a different
thread by way of retainIconForPageURL or releaseIconForPageURL.

(WebCore::IconDatabase::synchronousIconForPageURL):
(WebCore::IconDatabase::retainIconForPageURL):
(WebCore::IconDatabase::performRetainIconForPageURL):
(WebCore::IconDatabase::releaseIconForPageURL):
(WebCore::IconDatabase::performReleaseIconForPageURL):
(WebCore::IconDatabase::retainedPageURLCount):
(WebCore::IconDatabase::IconDatabase):
(WebCore::IconDatabase::performURLImport):
(WebCore::IconDatabase::syncThreadMainLoop):
(WebCore::IconDatabase::performPendingRetainAndReleaseOperations):

Location:
trunk/Source/WebCore
Files:
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r117497 r117501  
     12012-05-15  Andreas Kling  <kling@webkit.org>
     2
     3        IconDatabase: Move icon retain/release off of the main thread.
     4        <http://webkit.org/b/85799>
     5        <rdar://problem/9507113>
     6
     7        Reviewed by Brady Eidson.
     8
     9        Batch up the retain/release operations and execute them as part of the sync thread loop.
     10        The batch execution is guarded by a new mutex (m_urlsToRetainOrReleaseLock.)
     11        This avoids blocking the main thread on m_urlAndIconLock for basic retain/release.
     12
     13        There is one exception; if there are pending retain/release operations in synchronousIconForPageURL,
     14        it will acquire the lock and flush the operations.
     15
     16        There should be no behavior change, this is only meant to reduce lock contention.
     17
     18        * loader/icon/PageURLRecord.h:
     19        (WebCore::PageURLRecord::retain):
     20        (WebCore::PageURLRecord::release):
     21
     22            Added a 'count' argument to these so we can batch up the operations in IconDatabase.
     23
     24        * loader/icon/IconDatabase.h:
     25        * loader/icon/IconDatabase.cpp:
     26        (WebCore::IconDatabase::performScheduleOrDeferSyncTimer):
     27        (WebCore::IconDatabase::performScheduleOrDeferSyncTimerOnMainThread):
     28        (WebCore::IconDatabase::scheduleOrDeferSyncTimer):
     29
     30            Perform the the timer scheduling on the main thread as it can be done on a different
     31            thread by way of retainIconForPageURL or releaseIconForPageURL.
     32
     33        (WebCore::IconDatabase::synchronousIconForPageURL):
     34        (WebCore::IconDatabase::retainIconForPageURL):
     35        (WebCore::IconDatabase::performRetainIconForPageURL):
     36        (WebCore::IconDatabase::releaseIconForPageURL):
     37        (WebCore::IconDatabase::performReleaseIconForPageURL):
     38        (WebCore::IconDatabase::retainedPageURLCount):
     39        (WebCore::IconDatabase::IconDatabase):
     40        (WebCore::IconDatabase::performURLImport):
     41        (WebCore::IconDatabase::syncThreadMainLoop):
     42        (WebCore::IconDatabase::performPendingRetainAndReleaseOperations):
     43
    1442012-05-17  Julien Chaffraix  <jchaffraix@webkit.org>
    245

  • trunk/Source/WebCore/loader/icon/IconDatabase.cpp

    r116970 r117501  
    223223
    224224    MutexLocker locker(m_urlAndIconLock);
     225
     226    if (m_retainOrReleaseIconRequested)
     227        performPendingRetainAndReleaseOperations();
    225228   
    226229    String pageURLCopy; // Creates a null string for easy testing
    … …  
    390393}
    391394
    392 
    393 void IconDatabase::retainIconForPageURL(const String& pageURLOriginal)
    394 {   
     395void IconDatabase::retainIconForPageURL(const String& pageURL)
     396{
    395397    ASSERT_NOT_SYNC_THREAD();
    396    
    397     // Cannot do anything with pageURLOriginal that would end up storing it without deep copying first
    398    
    399     if (!isEnabled() || !documentCanHaveIcon(pageURLOriginal))
     398
     399    if (!isEnabled() || !documentCanHaveIcon(pageURL))
    400400        return;
    401401       
    402     MutexLocker locker(m_urlAndIconLock);
    403 
     402    MutexLocker locker(m_urlsToRetainOrReleaseLock);
     403    m_urlsToRetain.add(pageURL);
     404    scheduleOrDeferSyncTimer();
     405}
     406
     407void IconDatabase::performRetainIconForPageURL(const String& pageURLOriginal, int retainCount)
     408{
    404409    PageURLRecord* record = m_pageURLToRecordMap.get(pageURLOriginal);
    405410   
    … …  
    413418    }
    414419   
    415     if (!record->retain()) {
     420    if (!record->retain(retainCount)) {
    416421        if (pageURL.isNull())
    417422            pageURL = pageURLOriginal.isolatedCopy();
    … …  
    435440}
    436441
    437 void IconDatabase::releaseIconForPageURL(const String& pageURLOriginal)
     442void IconDatabase::releaseIconForPageURL(const String& pageURL)
    438443{
    439444    ASSERT_NOT_SYNC_THREAD();
    … …  
    441446    // Cannot do anything with pageURLOriginal that would end up storing it without deep copying first
    442447   
    443     if (!isEnabled() || !documentCanHaveIcon(pageURLOriginal))
    444         return;
    445    
    446     MutexLocker locker(m_urlAndIconLock);
    447 
     448    if (!isEnabled() || !documentCanHaveIcon(pageURL))
     449        return;
     450
     451    MutexLocker locker(m_urlsToRetainOrReleaseLock);
     452    m_urlsToRelease.add(pageURL);
     453    m_retainOrReleaseIconRequested = true;
     454    scheduleOrDeferSyncTimer();
     455}
     456
     457void IconDatabase::performReleaseIconForPageURL(const String& pageURLOriginal, int releaseCount)
     458{
    448459    // Check if this pageURL is actually retained
    449460    if (!m_retainedPageURLs.contains(pageURLOriginal)) {
    … …  
    459470       
    460471    // If it still has a positive retain count, store the new count and bail
    461     if (pageRecord->release())
     472    if (pageRecord->release(releaseCount))
    462473        return;
    463474       
    … …  
    499510   
    500511    delete pageRecord;
    501 
    502     if (isOpen())
    503         scheduleOrDeferSyncTimer();
    504512}
    505513
    … …  
    737745{
    738746    MutexLocker locker(m_urlAndIconLock);
     747
     748    if (m_retainOrReleaseIconRequested)
     749        performPendingRetainAndReleaseOperations();
     750
    739751    return m_retainedPageURLs.size();
    740752}
    … …  
    763775    : m_syncTimer(this, &IconDatabase::syncTimerFired)
    764776    , m_syncThreadRunning(false)
     777    , m_scheduleOrDeferSyncTimerRequested(false)
    765778    , m_isEnabled(false)
    766779    , m_privateBrowsingEnabled(false)
    … …  
    824837}
    825838
     839void IconDatabase::performScheduleOrDeferSyncTimer()
     840{
     841    m_syncTimer.startOneShot(updateTimerDelay);
     842    m_scheduleOrDeferSyncTimerRequested = false;
     843}
     844
     845void IconDatabase::performScheduleOrDeferSyncTimerOnMainThread(void* context)
     846{
     847    static_cast<IconDatabase*>(context)->performScheduleOrDeferSyncTimer();
     848}
     849
    826850void IconDatabase::scheduleOrDeferSyncTimer()
    827851{
    828852    ASSERT_NOT_SYNC_THREAD();
    829853
    830     if (!m_syncTimer.isActive()) {
    831         // The following is balanced by the call to enableSuddenTermination in the
    832         // syncTimerFired function.
    833         disableSuddenTermination();
    834     }
    835 
    836     m_syncTimer.startOneShot(updateTimerDelay);
     854    if (m_scheduleOrDeferSyncTimerRequested)
     855        return;
     856
     857    // The following is balanced by the call to enableSuddenTermination in the
     858    // syncTimerFired function.
     859    disableSuddenTermination();
     860
     861    m_scheduleOrDeferSyncTimerRequested = true;
     862    callOnMainThread(performScheduleOrDeferSyncTimerOnMainThread, this);
    837863}
    838864
    … …  
    13071333    {
    13081334        MutexLocker locker(m_urlAndIconLock);
    1309        
     1335
     1336        if (m_retainOrReleaseIconRequested)
     1337            performPendingRetainAndReleaseOperations();
     1338
    13101339        for (unsigned i = 0; i < urls.size(); ++i) {
    13111340            if (!m_retainedPageURLs.contains(urls[i])) {
    … …  
    13881417        if (m_threadTerminationRequested)
    13891418            break;
     1419
     1420        if (m_retainOrReleaseIconRequested) {
     1421            MutexLocker locker(m_urlAndIconLock);
     1422            performPendingRetainAndReleaseOperations();
     1423        }
    13901424       
    13911425        bool didAnyWork = true;
    … …  
    14701504        m_disabledSuddenTerminationForSyncThread = false;
    14711505    }
     1506}
     1507
     1508void IconDatabase::performPendingRetainAndReleaseOperations()
     1509{
     1510    ASSERT(m_retainOrReleaseIconRequested);
     1511
     1512    // NOTE: The caller is assumed to hold m_urlAndIconLock.
     1513    ASSERT(!m_urlAndIconLock.tryLock());
     1514
     1515    MutexLocker vectorLocker(m_urlsToRetainOrReleaseLock);
     1516
     1517    for (HashCountedSet<String>::const_iterator it = m_urlsToRetain.begin(), end = m_urlsToRetain.end(); it != end; ++it)
     1518        performRetainIconForPageURL(it->first, it->second);
     1519    for (HashCountedSet<String>::const_iterator it = m_urlsToRelease.begin(), end = m_urlsToRelease.end(); it != end; ++it)
     1520        performReleaseIconForPageURL(it->first, it->second);
     1521
     1522    m_urlsToRetain.clear();
     1523    m_urlsToRelease.clear();
     1524    m_retainOrReleaseIconRequested = false;
    14721525}
    14731526

  • trunk/Source/WebCore/loader/icon/IconDatabase.h

    r116970 r117501  
    3030#include "IconDatabaseBase.h"
    3131#include "Timer.h"
     32#include <wtf/HashCountedSet.h>
    3233#include <wtf/HashMap.h>
    3334#include <wtf/HashSet.h>
    … …  
    136137    RefPtr<IconRecord> m_defaultIconRecord;
    137138
     139    static void performScheduleOrDeferSyncTimerOnMainThread(void*);
     140    void performScheduleOrDeferSyncTimer();
     141
     142    bool m_scheduleOrDeferSyncTimerRequested;
     143
    138144// *** Any Thread ***
    139145public:
    … …  
    160166    bool m_syncThreadHasWorkToDo;
    161167    bool m_disabledSuddenTerminationForSyncThread;
     168    bool m_retainOrReleaseIconRequested;
    162169
    163170    Mutex m_urlAndIconLock;
    … …  
    177184    HashSet<String> m_pageURLsInterestedInIcons;
    178185    HashSet<IconRecord*> m_iconsPendingReading;
     186
     187    Mutex m_urlsToRetainOrReleaseLock;
     188    // Holding m_urlsToRetainOrReleaseLock is required when accessing any of the following data structures.
     189    HashCountedSet<String> m_urlsToRetain;
     190    HashCountedSet<String> m_urlsToRelease;
    179191
    180192// *** Sync Thread Only ***
    … …  
    203215    void deleteAllPreparedStatements();
    204216    void* cleanupSyncThread();
     217    void performRetainIconForPageURL(const String&, int retainCount);
     218    void performReleaseIconForPageURL(const String&, int releaseCount);
    205219
    206220    // Record (on disk) whether or not Safari 2-style icons were imported (once per dataabse)
    … …  
    221235    void removeIconFromSQLDatabase(const String& iconURL);
    222236    void writeIconSnapshotToSQLDatabase(const IconSnapshot&);   
    223    
     237
     238    void performPendingRetainAndReleaseOperations();
     239
    224240    // Methods to dispatch client callbacks on the main thread
    225241    void dispatchDidImportIconURLForPageURLOnMainThread(const String&);

  • trunk/Source/WebCore/loader/icon/PageURLRecord.h

    r83234 r117501  
    7070
    7171    // Returns false if the page wasn't retained beforehand, true if the retain count was already 1 or higher
    72     inline bool retain() { return m_retainCount++; }
     72    bool retain(int count)
     73    {
     74        bool wasRetained = m_retainCount > 0;
     75        m_retainCount += count;
     76        return wasRetained;
     77    }
    7378
    7479    // Returns true if the page is still retained after the call.  False if the retain count just dropped to 0
    75     inline bool release()
     80    bool release(int count)
    7681    {
    77         ASSERT(m_retainCount > 0);
    78         return --m_retainCount;
     82        ASSERT(m_retainCount >= count);
     83        m_retainCount -= count;
     84        return m_retainCount > 0;
    7985    }
    8086
Note: See TracChangeset for help on using the changeset viewer.