Changeset 87901 in webkit


Ignore:
Timestamp:
Jun 2, 2011 5:52:37 AM (13 years ago)
Author:
commit-queue@webkit.org
Message:

2011-06-02 James Robinson <jamesr@chromium.org>

Reviewed by Brady Eidson.

DocumentLoader keeps a reference to all URL strings ever loaded leading to lots of memory waste
https://bugs.webkit.org/show_bug.cgi?id=61894

DocumentLoader::m_resourcesClientKnowsAbout is a set of all the URLs that have passed through
FrameLoader::dispatchWillSendRequest() and is used by FrameLoader::loadedResourceFromMemoryCached to decide
whether to inform the FrameLoader's m_client about this load. Unfortunately, this set holds a reference to the
URL string for every resource loaded, so on pages that use data URLs to "load" large amounts of data this leaks
lots of memory.

This set only has an effect on the Mac port, as the only two ports that implement
FrameLoaderClient::dispatchDidLoadResourceFromMemoryCache() are Chromium and Mac and the Chromium implementation
can correctly handle receiving multiple callbacks. This patch limits the set to only PLATFORM(MAC) so other
ports do not have to pay this memory cost. It's possible that a better fix exists specifically for the Mac port
implementation, but that would have to determined by someone who works on that port specifically.

  • loader/DocumentLoader.h: (WebCore::DocumentLoader::didTellClientAboutLoad): (WebCore::DocumentLoader::haveToldClientAboutLoad):
Location:
trunk/Source/WebCore
Files:
4 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r87897 r87901  
     12011-06-02  James Robinson  <jamesr@chromium.org>
     2
     3        Reviewed by Brady Eidson.
     4
     5        DocumentLoader keeps a reference to all URL strings ever loaded leading to lots of memory waste
     6        https://bugs.webkit.org/show_bug.cgi?id=61894
     7
     8        DocumentLoader::m_resourcesClientKnowsAbout is a set of all the URLs that have passed through
     9        FrameLoader::dispatchWillSendRequest() and is used by FrameLoader::loadedResourceFromMemoryCached to decide
     10        whether to inform the FrameLoader's m_client about this load.  Unfortunately, this set holds a reference to the
     11        URL string for every resource loaded, so on pages that use data URLs to "load" large amounts of data this leaks
     12        lots of memory.
     13
     14        This set only has an effect on the Mac port, as the only two ports that implement
     15        FrameLoaderClient::dispatchDidLoadResourceFromMemoryCache() are Chromium and Mac and the Chromium implementation
     16        can correctly handle receiving multiple callbacks.  This patch limits the set to only PLATFORM(MAC) so other
     17        ports do not have to pay this memory cost.  It's possible that a better fix exists specifically for the Mac port
     18        implementation, but that would have to determined by someone who works on that port specifically.
     19
     20        * loader/DocumentLoader.h:
     21        (WebCore::DocumentLoader::didTellClientAboutLoad):
     22        (WebCore::DocumentLoader::haveToldClientAboutLoad):
     23
    1242011-06-02  Aparna Nandyal  <aparna.nand@wipro.com>
    225
  • trunk/Source/WebCore/loader/DocumentLoader.h

    r87189 r87901  
    225225        bool deferMainResourceDataLoad() const { return m_deferMainResourceDataLoad; }
    226226       
     227#if PLATFORM(MAC)
    227228        void didTellClientAboutLoad(const String& url)
    228         { 
     229        {
    229230            if (!url.isEmpty())
    230231                m_resourcesClientKnowsAbout.add(url);
    231232        }
    232         bool haveToldClientAboutLoad(const String& url) { return m_resourcesClientKnowsAbout.contains(url); }
     233        bool haveToldClientAboutLoad(const String& url)
     234        {
     235            return m_resourcesClientKnowsAbout.contains(url);
     236        }
     237#endif
     238
    233239        void recordMemoryCacheLoadForFutureClientNotification(const String& url);
    234240        void takeMemoryCacheLoadsForClientNotification(Vector<String>& loads);
     
    329335#endif
    330336
     337#if PLATFORM(MAC)
    331338        HashSet<String> m_resourcesClientKnowsAbout;
     339#endif
    332340        Vector<String> m_resourcesLoadedFromMemoryCacheForClientNotification;
    333341       
  • trunk/Source/WebCore/loader/FrameLoader.cpp

    r87634 r87901  
    31423142        return;
    31433143
    3144     if (!resource->sendResourceLoadCallbacks() || m_documentLoader->haveToldClientAboutLoad(resource->url()))
    3145         return;
     3144    if (!resource->sendResourceLoadCallbacks())
     3145        return;
     3146
     3147#if PLATFORM(MAC)
     3148    if (m_documentLoader->haveToldClientAboutLoad(resource->url()))
     3149        return;
     3150#endif
    31463151
    31473152    if (!page->areMemoryCacheClientCallsEnabled()) {
    31483153        InspectorInstrumentation::didLoadResourceFromMemoryCache(page, m_documentLoader.get(), resource);
    31493154        m_documentLoader->recordMemoryCacheLoadForFutureClientNotification(resource->url());
     3155#if PLATFORM(MAC)
    31503156        m_documentLoader->didTellClientAboutLoad(resource->url());
     3157#endif
    31513158        return;
    31523159    }
     
    31553162    if (m_client->dispatchDidLoadResourceFromMemoryCache(m_documentLoader.get(), request, resource->response(), resource->encodedSize())) {
    31563163        InspectorInstrumentation::didLoadResourceFromMemoryCache(page, m_documentLoader.get(), resource);
     3164#if PLATFORM(MAC)
    31573165        m_documentLoader->didTellClientAboutLoad(resource->url());
     3166#endif
    31583167        return;
    31593168    }
  • trunk/Source/WebCore/loader/ResourceLoadNotifier.cpp

    r84260 r87901  
    108108void ResourceLoadNotifier::dispatchWillSendRequest(DocumentLoader* loader, unsigned long identifier, ResourceRequest& request, const ResourceResponse& redirectResponse)
    109109{
     110#if PLATFORM(MAC)
    110111    StringImpl* oldRequestURL = request.url().string().impl();
    111112    m_frame->loader()->documentLoader()->didTellClientAboutLoad(request.url());
     113#endif
    112114
    113115    m_frame->loader()->client()->dispatchWillSendRequest(loader, identifier, request, redirectResponse);
    114116
     117#if PLATFORM(MAC)
    115118    // If the URL changed, then we want to put that new URL in the "did tell client" set too.
    116119    if (!request.isNull() && oldRequestURL != request.url().string().impl())
    117120        m_frame->loader()->documentLoader()->didTellClientAboutLoad(request.url());
     121#endif
    118122
    119123    InspectorInstrumentation::willSendRequest(m_frame, identifier, loader, request, redirectResponse);
Note: See TracChangeset for help on using the changeset viewer.