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

Changeset 242726 in webkit


Ignore:
Timestamp:
Mar 11, 2019, 12:30:42 PM (7 years ago)
Author:
Chris Dumez
Message:

WebProcessCache should keep track of processes being added
​https://bugs.webkit.org/show_bug.cgi?id=195538

Reviewed by Geoffrey Garen.

WebProcessCache should keep track of processes being added, while they are being
checked for responsiveness. This is useful so that:

  • Requests to clear the cache also clear processes being added
  • Requests to remove a given process from the cache (either because it crashed or because it is being used for a history navigation) actually remove the process if it is still being checked for responsiveness.
  • The cached process eviction timer applies to such processes in case something goes wrong with the code and the pending request does not get processed.
  • UIProcess/WebProcessCache.cpp:

(WebKit::generateAddRequestIdentifier):
(WebKit::WebProcessCache::addProcessIfPossible):
(WebKit::WebProcessCache::addProcess):
(WebKit::WebProcessCache::clear):
(WebKit::WebProcessCache::clearAllProcessesForSession):
(WebKit::WebProcessCache::removeProcess):
(WebKit::WebProcessCache::CachedProcess::evictionTimerFired):
(WebKit::WebProcessCache::evictProcess): Deleted.

  • UIProcess/WebProcessCache.h:

(WebKit::WebProcessCache::size const):

  • UIProcess/WebProcessPool.cpp:

(WebKit::WebProcessPool::processForNavigationInternal):

  • UIProcess/WebProcessProxy.cpp:

(WebKit::WebProcessProxy::processDidTerminateOrFailedToLaunch):

Location:
trunk/Source/WebKit
Files:
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebKit/ChangeLog

    r242723 r242726  
     12019-03-11  Chris Dumez  <cdumez@apple.com>
     2
     3        WebProcessCache should keep track of processes being added
     4        https://bugs.webkit.org/show_bug.cgi?id=195538
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        WebProcessCache should keep track of processes being added, while they are being
     9        checked for responsiveness. This is useful so that:
     10        - Requests to clear the cache also clear processes being added
     11        - Requests to remove a given process from the cache (either because it crashed
     12          or because it is being used for a history navigation) actually remove the
     13          process if it is still being checked for responsiveness.
     14        - The cached process eviction timer applies to such processes in case something
     15          goes wrong with the code and the pending request does not get processed.
     16
     17        * UIProcess/WebProcessCache.cpp:
     18        (WebKit::generateAddRequestIdentifier):
     19        (WebKit::WebProcessCache::addProcessIfPossible):
     20        (WebKit::WebProcessCache::addProcess):
     21        (WebKit::WebProcessCache::clear):
     22        (WebKit::WebProcessCache::clearAllProcessesForSession):
     23        (WebKit::WebProcessCache::removeProcess):
     24        (WebKit::WebProcessCache::CachedProcess::evictionTimerFired):
     25        (WebKit::WebProcessCache::evictProcess): Deleted.
     26        * UIProcess/WebProcessCache.h:
     27        (WebKit::WebProcessCache::size const):
     28        * UIProcess/WebProcessPool.cpp:
     29        (WebKit::WebProcessPool::processForNavigationInternal):
     30        * UIProcess/WebProcessProxy.cpp:
     31        (WebKit::WebProcessProxy::processDidTerminateOrFailedToLaunch):
     32
    1332019-03-11  Alex Christensen  <achristensen@webkit.org>
    234

  • trunk/Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm

    r242371 r242726  
    477477- (size_t)_webProcessCountIgnoringPrewarmedAndCached
    478478{
    479     return [self _webProcessCount] - ([self _hasPrewarmedWebProcess] ? 1 : 0) - _processPool->webProcessCache().size();
     479    size_t count = 0;
     480    for (auto& process : _processPool->processes()) {
     481        if (!process->isInProcessCache() && !process->isPrewarmed())
     482            ++count;
     483    }
     484    return count;
    480485}
    481486

  • trunk/Source/WebKit/UIProcess/WebProcessCache.cpp

    r242652 r242726  
    3838Seconds WebProcessCache::clearingDelayAfterApplicationResignsActive { 5_min };
    3939
     40static uint64_t generateAddRequestIdentifier()
     41{
     42    static uint64_t identifier = 0;
     43    return ++identifier;
     44}
     45
    4046WebProcessCache::WebProcessCache(WebProcessPool& processPool)
    4147    : m_evictionTimer(RunLoop::main(), this, &WebProcessCache::clear)
    … …  
    7480        return false;
    7581
     82    uint64_t requestIdentifier = generateAddRequestIdentifier();
     83    m_pendingAddRequests.add(requestIdentifier, std::make_unique<CachedProcess>(process.copyRef()));
     84
    7685    RELEASE_LOG(ProcessSwapping, "%p - WebProcessCache::addProcessIfPossible(): Checking if process %i is responsive before caching it...", this, process->processIdentifier());
    77     process->setIsInProcessCache(true);
    78     process->isResponsive([process = process.copyRef(), processPool = makeRef(process->processPool()), registrableDomain](bool isResponsive) {
    79         process->setIsInProcessCache(false);
     86    process->isResponsive([this, processPool = makeRef(process->processPool()), requestIdentifier](bool isResponsive) {
     87        auto cachedProcess = m_pendingAddRequests.take(requestIdentifier);
     88        if (!cachedProcess)
     89            return;
     90
    8091        if (!isResponsive) {
    81             RELEASE_LOG_ERROR(ProcessSwapping, "%p - WebProcessCache::addProcessIfPossible(): Not caching process %i because it is not responsive", &process->processPool().webProcessCache(), process->processIdentifier());
    82             process->shutDown();
     92            RELEASE_LOG_ERROR(ProcessSwapping, "%p - WebProcessCache::addProcessIfPossible(): Not caching process %i because it is not responsive", &processPool->webProcessCache(), cachedProcess->process().processIdentifier());
    8393            return;
    8494        }
    85         if (!processPool->webProcessCache().addProcess(registrableDomain, process.copyRef()))
    86             process->shutDown();
     95        processPool->webProcessCache().addProcess(WTFMove(cachedProcess));
    8796    });
    8897    return true;
    8998}
    9099
    91 bool WebProcessCache::addProcess(const String& registrableDomain, Ref<WebProcessProxy>&& process)
    92 {
    93     ASSERT(!process->pageCount());
    94     ASSERT(!process->provisionalPageCount());
    95     ASSERT(!process->suspendedPageCount());
    96 
    97     if (!canCacheProcess(process))
    98         return false;
     100bool WebProcessCache::addProcess(std::unique_ptr<CachedProcess>&& cachedProcess)
     101{
     102    ASSERT(!cachedProcess->process().pageCount());
     103    ASSERT(!cachedProcess->process().provisionalPageCount());
     104    ASSERT(!cachedProcess->process().suspendedPageCount());
     105
     106    if (!canCacheProcess(cachedProcess->process()))
     107        return false;
     108
     109    auto registrableDomain = cachedProcess->process().registrableDomain();
     110    RELEASE_ASSERT(!registrableDomain.isEmpty());
     111
     112    if (auto previousProcess = m_processesPerRegistrableDomain.take(registrableDomain))
     113        RELEASE_LOG(ProcessSwapping, "%p - WebProcessCache::addProcess(): Evicting process %i from WebProcess cache because a new process was added for the same domain", this, previousProcess->process().processIdentifier());
    99114
    100115    while (m_processesPerRegistrableDomain.size() >= capacity()) {
    101116        auto it = m_processesPerRegistrableDomain.random();
    102         RELEASE_LOG(ProcessSwapping, "%p - WebProcessCache::addProcess(): Evicting process %i from WebProcess cache", this, it->value->process().processIdentifier());
     117        RELEASE_LOG(ProcessSwapping, "%p - WebProcessCache::addProcess(): Evicting process %i from WebProcess cache because capacity was reached", this, it->value->process().processIdentifier());
    103118        m_processesPerRegistrableDomain.remove(it);
    104119    }
    105120
    106     m_processesPerRegistrableDomain.set(registrableDomain, std::make_unique<CachedProcess>(process.copyRef()));
    107     RELEASE_LOG(ProcessSwapping, "%p - WebProcessCache::addProcess: Adding process %i to WebProcess cache, cache size: [%u / %u]", this, process->processIdentifier(), size(), capacity());
     121    RELEASE_LOG(ProcessSwapping, "%p - WebProcessCache::addProcess: Added process %i to WebProcess cache, cache size: [%u / %u]", this, cachedProcess->process().processIdentifier(), size() + 1, capacity());
     122    m_processesPerRegistrableDomain.add(registrableDomain, WTFMove(cachedProcess));
    108123
    109124    return true;
    … …  
    156171void WebProcessCache::clear()
    157172{
    158     if (m_processesPerRegistrableDomain.isEmpty())
     173    if (m_pendingAddRequests.isEmpty() && m_processesPerRegistrableDomain.isEmpty())
    159174        return;
    160175
    161     RELEASE_LOG(ProcessSwapping, "%p - WebProcessCache::clear() evicting %u processes", this, m_processesPerRegistrableDomain.size());
     176    RELEASE_LOG(ProcessSwapping, "%p - WebProcessCache::clear() evicting %u processes", this, m_pendingAddRequests.size() + m_processesPerRegistrableDomain.size());
     177    m_pendingAddRequests.clear();
    162178    m_processesPerRegistrableDomain.clear();
    163179}
    … …  
    174190    for (auto& key : keysToRemove)
    175191        m_processesPerRegistrableDomain.remove(key);
     192
     193    Vector<uint64_t> pendingRequestsToRemove;
     194    for (auto& pair : m_pendingAddRequests) {
     195        if (pair.value->process().websiteDataStore().sessionID() == sessionID) {
     196            RELEASE_LOG(ProcessSwapping, "%p - WebProcessCache::clearAllProcessesForSession() evicting process %i because its session was destroyed", this, pair.value->process().processIdentifier());
     197            pendingRequestsToRemove.append(pair.key);
     198        }
     199    }
     200    for (auto& key : pendingRequestsToRemove)
     201        m_pendingAddRequests.remove(key);
    176202}
    177203
    … …  
    185211}
    186212
    187 void WebProcessCache::evictProcess(WebProcessProxy& process)
     213void WebProcessCache::removeProcess(WebProcessProxy& process, ShouldShutDownProcess shouldShutDownProcess)
    188214{
    189215    RELEASE_ASSERT(!process.registrableDomain().isEmpty());
     216    RELEASE_LOG(ProcessSwapping, "%p - WebProcessCache::evictProcess(): Evicting process %i from WebProcess cache because it expired", this, process.processIdentifier());
     217
     218    std::unique_ptr<CachedProcess> cachedProcess;
    190219    auto it = m_processesPerRegistrableDomain.find(process.registrableDomain());
    191     ASSERT(it != m_processesPerRegistrableDomain.end());
    192     ASSERT(&it->value->process() == &process);
    193 
    194     RELEASE_LOG(ProcessSwapping, "%p - WebProcessCache::evictProcess(): Evicting process %i from WebProcess cache because it expired", this, process.processIdentifier());
    195 
    196     m_processesPerRegistrableDomain.remove(it);
     220    if (it != m_processesPerRegistrableDomain.end() && &it->value->process() == &process) {
     221        cachedProcess = WTFMove(it->value);
     222        m_processesPerRegistrableDomain.remove(it);
     223    } else {
     224        for (auto& pair : m_pendingAddRequests) {
     225            if (&pair.value->process() == &process) {
     226                cachedProcess = WTFMove(pair.value);
     227                m_pendingAddRequests.remove(pair.key);
     228                break;
     229            }
     230        }
     231    }
     232    ASSERT(cachedProcess);
     233    if (!cachedProcess)
     234        return;
     235
     236    ASSERT(&cachedProcess->process() == &process);
     237    if (shouldShutDownProcess == ShouldShutDownProcess::No)
     238        cachedProcess->takeProcess();
    197239}
    198240
    … …  
    229271{
    230272    ASSERT(m_process);
    231     m_process->processPool().webProcessCache().evictProcess(*m_process);
     273    m_process->processPool().webProcessCache().removeProcess(*m_process, ShouldShutDownProcess::Yes);
    232274}
    233275

  • trunk/Source/WebKit/UIProcess/WebProcessCache.h

    r242496 r242726  
    5656    void clearAllProcessesForSession(PAL::SessionID);
    5757
     58    enum class ShouldShutDownProcess { No, Yes };
     59    void removeProcess(WebProcessProxy&, ShouldShutDownProcess);
     60
    5861private:
    5962    static Seconds cachedProcessLifetime;
    6063    static Seconds clearingDelayAfterApplicationResignsActive;
    61 
    62     bool canCacheProcess(WebProcessProxy&) const;
    63     void evictProcess(WebProcessProxy&);
    64     void platformInitialize();
    65     bool addProcess(const String& registrableDomain, Ref<WebProcessProxy>&&);
    66 
    67     unsigned m_capacity { 0 };
    6864
    6965    class CachedProcess {
    … …  
    8379    };
    8480
     81    bool canCacheProcess(WebProcessProxy&) const;
     82    void platformInitialize();
     83    bool addProcess(std::unique_ptr<CachedProcess>&&);
     84
     85    unsigned m_capacity { 0 };
     86
     87    HashMap<uint64_t, std::unique_ptr<CachedProcess>> m_pendingAddRequests;
    8588    HashMap<String, std::unique_ptr<CachedProcess>> m_processesPerRegistrableDomain;
    8689    RunLoop::Timer<WebProcessCache> m_evictionTimer;

  • trunk/Source/WebKit/UIProcess/WebProcessPool.cpp

    r242712 r242726  
    22612261            // Make sure we remove the process from the cache if it is in there since we're about to use it.
    22622262            if (process->isInProcessCache()) {
    2263                 auto removedProcess = webProcessCache().takeProcess(process->registrableDomain(), process->websiteDataStore());
    2264                 ASSERT_UNUSED(removedProcess, removedProcess.get() == process.get());
     2263                webProcessCache().removeProcess(*process, WebProcessCache::ShouldShutDownProcess::No);
     2264                ASSERT(!process->isInProcessCache());
    22652265            }
    22662266

  • trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp

    r242652 r242726  
    650650
    651651    if (m_isInProcessCache) {
    652         auto removedProcess = processPool().webProcessCache().takeProcess(registrableDomain(), websiteDataStore());
    653         ASSERT_UNUSED(removedProcess, removedProcess.get() == this);
     652        processPool().webProcessCache().removeProcess(*this, WebProcessCache::ShouldShutDownProcess::No);
     653        ASSERT(!m_isInProcessCache);
    654654    }
    655655
Note: See TracChangeset for help on using the changeset viewer.