Changeset 241232 in webkit


Ignore:
Timestamp:
Feb 8, 2019 8:36:57 PM (5 years ago)
Author:
Chris Dumez
Message:

[WK2][macOS] Avoid creating new CVDisplayLink objects for each WebProcess
​https://bugs.webkit.org/show_bug.cgi?id=194463

Reviewed by Tim Horton.

Avoid creating new CVDisplayLink objects for each WebProcess. We really only need one per
display, creating such object is expensive and it is even worse in a PSON world where we
swap process on navigation.

This patch moves the DisplayLink storing from WebProcessProxy to WebProcessPool. Also,
a DisplayLink can now be associated to several IPC connections instead of having a 1:1
mapping. When a DisplayLink no longer has any observers, we now merely stop it instead
of destroying it.

  • UIProcess/Cocoa/WebProcessPoolCocoa.mm:

(WebKit::WebProcessPool::startDisplayLink):
(WebKit::WebProcessPool::stopDisplayLink):
(WebKit::WebProcessPool::stopDisplayLinks):

  • UIProcess/WebProcessPool.h:
  • UIProcess/WebProcessProxy.cpp:

(WebKit::WebProcessProxy::~WebProcessProxy):
(WebKit::WebProcessProxy::processWillShutDown):
(WebKit::WebProcessProxy::shutDown):

  • UIProcess/WebProcessProxy.h:
  • UIProcess/mac/DisplayLink.cpp:

(WebKit::DisplayLink::DisplayLink):
(WebKit::DisplayLink::addObserver):
(WebKit::DisplayLink::removeObserver):
(WebKit::DisplayLink::removeObservers):
(WebKit::DisplayLink::hasObservers const):
(WebKit::DisplayLink::displayLinkCallback):

  • UIProcess/mac/DisplayLink.h:
  • UIProcess/mac/WebProcessProxyMac.mm:

(WebKit::WebProcessProxy::startDisplayLink):
(WebKit::WebProcessProxy::stopDisplayLink):

Location:
trunk/Source/WebKit
Files:
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebKit/ChangeLog

    r241224 r241232  
     12019-02-08  Chris Dumez  <cdumez@apple.com>
     2
     3        [WK2][macOS] Avoid creating new CVDisplayLink objects for each WebProcess
     4        https://bugs.webkit.org/show_bug.cgi?id=194463
     5
     6        Reviewed by Tim Horton.
     7
     8        Avoid creating new CVDisplayLink objects for each WebProcess. We really only need one per
     9        display, creating such object is expensive and it is even worse in a PSON world where we
     10        swap process on navigation.
     11
     12        This patch moves the DisplayLink storing from WebProcessProxy to WebProcessPool. Also,
     13        a DisplayLink can now be associated to several IPC connections instead of having a 1:1
     14        mapping. When a DisplayLink no longer has any observers, we now merely stop it instead
     15        of destroying it.
     16
     17        * UIProcess/Cocoa/WebProcessPoolCocoa.mm:
     18        (WebKit::WebProcessPool::startDisplayLink):
     19        (WebKit::WebProcessPool::stopDisplayLink):
     20        (WebKit::WebProcessPool::stopDisplayLinks):
     21        * UIProcess/WebProcessPool.h:
     22        * UIProcess/WebProcessProxy.cpp:
     23        (WebKit::WebProcessProxy::~WebProcessProxy):
     24        (WebKit::WebProcessProxy::processWillShutDown):
     25        (WebKit::WebProcessProxy::shutDown):
     26        * UIProcess/WebProcessProxy.h:
     27        * UIProcess/mac/DisplayLink.cpp:
     28        (WebKit::DisplayLink::DisplayLink):
     29        (WebKit::DisplayLink::addObserver):
     30        (WebKit::DisplayLink::removeObserver):
     31        (WebKit::DisplayLink::removeObservers):
     32        (WebKit::DisplayLink::hasObservers const):
     33        (WebKit::DisplayLink::displayLinkCallback):
     34        * UIProcess/mac/DisplayLink.h:
     35        * UIProcess/mac/WebProcessProxyMac.mm:
     36        (WebKit::WebProcessProxy::startDisplayLink):
     37        (WebKit::WebProcessProxy::stopDisplayLink):
     38
    1392019-02-08  Alexander Mikhaylenko  <exalm7659@gmail.com>
    240

  • trunk/Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm

    r241224 r241232  
    11/*
    2  * Copyright (C) 2010-2018 Apple Inc. All rights reserved.
     2 * Copyright (C) 2010-2019 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
    … …  
    469469}
    470470
     471#if PLATFORM(MAC) && ENABLE(WEBPROCESS_WINDOWSERVER_BLOCKING)
     472void WebProcessPool::startDisplayLink(IPC::Connection& connection, unsigned observerID, uint32_t displayID)
     473{
     474    for (auto& displayLink : m_displayLinks) {
     475        if (displayLink->displayID() == displayID) {
     476            displayLink->addObserver(connection, observerID);
     477            return;
     478        }
     479    }
     480    auto displayLink = std::make_unique<DisplayLink>(displayID);
     481    displayLink->addObserver(connection, observerID);
     482    m_displayLinks.append(WTFMove(displayLink));
     483}
     484
     485void WebProcessPool::stopDisplayLink(IPC::Connection& connection, unsigned observerID, uint32_t displayID)
     486{
     487    for (auto& displayLink : m_displayLinks) {
     488        if (displayLink->displayID() == displayID) {
     489            displayLink->removeObserver(connection, observerID);
     490            return;
     491        }
     492    }
     493}
     494
     495void WebProcessPool::stopDisplayLinks(IPC::Connection& connection)
     496{
     497    for (auto& displayLink : m_displayLinks)
     498        displayLink->removeObservers(connection);
     499}
     500#endif
     501
    471502// FIXME: Deprecated. Left here until a final decision is made.
    472503void WebProcessPool::setCookieStoragePartitioningEnabled(bool enabled)

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

    r241223 r241232  
    7272#endif
    7373
     74#if PLATFORM(MAC) && ENABLE(WEBPROCESS_WINDOWSERVER_BLOCKING)
     75#include "DisplayLink.h"
     76#endif
     77
    7478namespace API {
    7579class AutomationClient;
    … …  
    207211    void clearPluginClientPolicies();
    208212    const HashMap<String, HashMap<String, HashMap<String, uint8_t>>>& pluginLoadClientPolicies() const { return m_pluginLoadClientPolicies; }
     213#endif
     214
     215#if PLATFORM(MAC) && ENABLE(WEBPROCESS_WINDOWSERVER_BLOCKING)
     216    void startDisplayLink(IPC::Connection&, unsigned observerID, uint32_t displayID);
     217    void stopDisplayLink(IPC::Connection&, unsigned observerID, uint32_t displayID);
     218    void stopDisplayLinks(IPC::Connection&);
    209219#endif
    210220
    … …  
    731741    HashMap<String, std::unique_ptr<WebCore::PrewarmInformation>> m_prewarmInformationPerRegistrableDomain;
    732742
     743#if PLATFORM(MAC) && ENABLE(WEBPROCESS_WINDOWSERVER_BLOCKING)
     744    Vector<std::unique_ptr<DisplayLink>> m_displayLinks;
     745#endif
     746
    733747#if PLATFORM(GTK) || PLATFORM(WPE)
    734748    bool m_sandboxEnabled { false };

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

    r241132 r241232  
    163163    WebPasteboardProxy::singleton().removeWebProcessProxy(*this);
    164164
     165#if PLATFORM(MAC) && ENABLE(WEBPROCESS_WINDOWSERVER_BLOCKING)
     166    if (state() == State::Running)
     167        processPool().stopDisplayLinks(*connection());
     168#endif
     169
    165170    if (m_webConnection)
    166171        m_webConnection->invalidate();
    … …  
    240245{
    241246    ASSERT_UNUSED(connection, this->connection() == &connection);
     247
     248#if PLATFORM(MAC) && ENABLE(WEBPROCESS_WINDOWSERVER_BLOCKING)
     249    processPool().stopDisplayLinks(connection);
     250#endif
    242251
    243252    for (auto& page : m_pageMap.values())

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

    r241113 r241232  
    5151#include <wtf/RefPtr.h>
    5252
    53 #if PLATFORM(MAC) && ENABLE(WEBPROCESS_WINDOWSERVER_BLOCKING)
    54 #include "DisplayLink.h"
    55 #endif
    56 
    5753namespace API {
    5854class Navigation;
    … …  
    422418    ProcessThrottler::BackgroundActivityToken m_backgroundActivityTokenForFullscreenFormControls;
    423419#endif
    424 
    425 #if PLATFORM(MAC) && ENABLE(WEBPROCESS_WINDOWSERVER_BLOCKING)
    426     Vector<std::unique_ptr<DisplayLink>> m_displayLinks;
    427 #endif
    428420};
    429421

  • trunk/Source/WebKit/UIProcess/mac/DisplayLink.cpp

    r239747 r241232  
    11/*
    2  * Copyright (C) 2018 Apple Inc. All rights reserved.
     2 * Copyright (C) 2018-2019 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
    … …  
    2929#if ENABLE(WEBPROCESS_WINDOWSERVER_BLOCKING)
    3030
    31 #include "DrawingAreaMessages.h"
    3231#include "WebProcessMessages.h"
    33 #include "WebProcessProxy.h"
    3432#include <wtf/ProcessPrivilege.h>
    3533
    3634namespace WebKit {
    3735   
    38 DisplayLink::DisplayLink(WebCore::PlatformDisplayID displayID, WebProcessProxy& webProcessProxy)
    39     : m_connection(webProcessProxy.connection())
    40     , m_displayID(displayID)
     36DisplayLink::DisplayLink(WebCore::PlatformDisplayID displayID)
     37    : m_displayID(displayID)
    4138{
    4239    ASSERT(hasProcessPrivilege(ProcessPrivilege::CanCommunicateWithWindowServer));
    … …  
    5249        return;
    5350    }
    54 
    55     error = CVDisplayLinkStart(m_displayLink);
    56     if (error)
    57         WTFLogAlways("Could not start the display link: %d", error);
    5851}
    5952
    … …  
    6962}
    7063
    71 void DisplayLink::addObserver(unsigned observerID)
     64void DisplayLink::addObserver(IPC::Connection& connection, unsigned observerID)
    7265{
    73     m_observers.add(observerID);
     66    ASSERT(RunLoop::isMain());
     67    bool isRunning = !m_observers.isEmpty();
     68
     69    {
     70        LockHolder locker(m_observersLock);
     71        m_observers.ensure(&connection, [] {
     72            return Vector<unsigned> { };
     73        }).iterator->value.append(observerID);
     74    }
     75
     76    if (!isRunning) {
     77        CVReturn error = CVDisplayLinkStart(m_displayLink);
     78        if (error)
     79            WTFLogAlways("Could not start the display link: %d", error);
     80    }
    7481}
    7582
    76 void DisplayLink::removeObserver(unsigned observerID)
     83void DisplayLink::removeObserver(IPC::Connection& connection, unsigned observerID)
    7784{
    78     m_observers.remove(observerID);
     85    ASSERT(RunLoop::isMain());
     86
     87    {
     88        LockHolder locker(m_observersLock);
     89
     90        auto it = m_observers.find(&connection);
     91        if (it == m_observers.end())
     92            return;
     93        bool removed = it->value.removeFirst(observerID);
     94        ASSERT_UNUSED(removed, removed);
     95        if (it->value.isEmpty())
     96            m_observers.remove(it);
     97    }
     98
     99    if (m_observers.isEmpty())
     100        CVDisplayLinkStop(m_displayLink);
     101}
     102
     103void DisplayLink::removeObservers(IPC::Connection& connection)
     104{
     105    ASSERT(RunLoop::isMain());
     106
     107    {
     108        LockHolder locker(m_observersLock);
     109        m_observers.remove(&connection);
     110    }
     111
     112    if (m_observers.isEmpty())
     113        CVDisplayLinkStop(m_displayLink);
    79114}
    80115
    81116bool DisplayLink::hasObservers() const
    82117{
     118    ASSERT(RunLoop::isMain());
    83119    return !m_observers.isEmpty();
    84120}
    … …  
    86122CVReturn DisplayLink::displayLinkCallback(CVDisplayLinkRef displayLinkRef, const CVTimeStamp*, const CVTimeStamp*, CVOptionFlags, CVOptionFlags*, void* data)
    87123{
    88     DisplayLink* displayLink = static_cast<DisplayLink*>(data);
    89     displayLink->m_connection->send(Messages::WebProcess::DisplayWasRefreshed(displayLink->m_displayID), 0);
     124    auto* displayLink = static_cast<DisplayLink*>(data);
     125    LockHolder locker(displayLink->m_observersLock);
     126    for (auto& connection : displayLink->m_observers.keys())
     127        connection->send(Messages::WebProcess::DisplayWasRefreshed(displayLink->m_displayID), 0);
    90128    return kCVReturnSuccess;
    91129}
    92130
    93 }
     131} // namespace WebKit
    94132
    95133#endif

  • trunk/Source/WebKit/UIProcess/mac/DisplayLink.h

    r241129 r241232  
    11/*
    2  * Copyright (C) 2018 Apple Inc. All rights reserved.
     2 * Copyright (C) 2018-2019 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
    … …  
    2929
    3030#include <CoreVideo/CVDisplayLink.h>
    31 
    3231#include <WebCore/PlatformScreen.h>
    33 #include <wtf/HashSet.h>
     32#include <wtf/HashMap.h>
     33#include <wtf/Lock.h>
    3434
    3535namespace IPC {
    … …  
    3838
    3939namespace WebKit {
    40 
    41 class WebProcessProxy;
    4240   
    4341class DisplayLink {
    4442    WTF_MAKE_FAST_ALLOCATED;
    4543public:
    46     DisplayLink(WebCore::PlatformDisplayID, WebProcessProxy&);
     44    explicit DisplayLink(WebCore::PlatformDisplayID);
    4745    ~DisplayLink();
    4846   
    49     void addObserver(unsigned observerID);
    50     void removeObserver(unsigned observerID);
     47    void addObserver(IPC::Connection&, unsigned observerID);
     48    void removeObserver(IPC::Connection&, unsigned observerID);
     49    void removeObservers(IPC::Connection&);
    5150    bool hasObservers() const;
    5251
    … …  
    5756   
    5857    CVDisplayLinkRef m_displayLink { nullptr };
    59     HashSet<unsigned> m_observers;
    60     RefPtr<IPC::Connection> m_connection;
    61     WebCore::PlatformDisplayID m_displayID { 0 };
     58    Lock m_observersLock;
     59    HashMap<RefPtr<IPC::Connection>, Vector<unsigned>> m_observers;
     60    WebCore::PlatformDisplayID m_displayID;
    6261};
    6362
    64 }
     63} // namespace WebKit
    6564
    6665#endif

  • trunk/Source/WebKit/UIProcess/mac/WebProcessProxyMac.mm

    r238434 r241232  
    7575{
    7676    ASSERT(hasProcessPrivilege(ProcessPrivilege::CanCommunicateWithWindowServer));
    77     for (auto& displayLink : m_displayLinks) {
    78         if (displayLink->displayID() == displayID) {
    79             displayLink->addObserver(observerID);
    80             return;
    81         }
    82     }
    83     auto displayLink = std::make_unique<DisplayLink>(displayID, *this);
    84     displayLink->addObserver(observerID);
    85     m_displayLinks.append(WTFMove(displayLink));
     77    ASSERT(connection());
     78    processPool().startDisplayLink(*connection(), observerID, displayID);
    8679}
    8780
    8881void WebProcessProxy::stopDisplayLink(unsigned observerID, uint32_t displayID)
    8982{
    90     size_t pos = 0;
    91     for (auto& displayLink : m_displayLinks) {
    92         if (displayLink->displayID() == displayID) {
    93             displayLink->removeObserver(observerID);
    94             if (!displayLink->hasObservers())
    95                 m_displayLinks.remove(pos);
    96             return;
    97         }
    98         pos++;
    99     }
     83    ASSERT(connection());
     84    processPool().stopDisplayLink(*connection(), observerID, displayID);
    10085}
    10186#endif
Note: See TracChangeset for help on using the changeset viewer.