2008-12-18 Darin Adler <darin@apple.com>

        Reviewed by Sam Weinig.

        - fix <rdar://problem/6449841> reduce memory use of ResourceResponseBase by removing two maps

        We were parsing the cache-control and pragma header field values into maps.
        I changed that so instead we only cache two bits with the data we were actually
        using. Later we might want to move this responsibility out of this class entirely;
        we can cache it at the higher level instead.

        * loader/CachedResource.cpp:
        (WebCore::CachedResource::mustRevalidate): Changed to call a specific API to get at
        the bits in quest instead of a general "cache control directives" API.

        * platform/network/ResourceResponseBase.cpp:
        (WebCore::ResourceResponseBase::setHTTPHeaderField): Remove the logic for the
        pragma header field since no one is using this for now.
        (WebCore::ResourceResponseBase::parseCacheControlDirectives): Eliminated the return
        value and made this function have side effects only. Changed it so it's the caller's
        responsibility to check m_haveParsedCacheControl. Set m_cacheControlContainsNoCache
        and m_cacheControlContainsMustRevalidate rather than keeping a map around.

        * platform/network/ResourceResponseBase.h:
        (WebCore::ResourceResponseBase::cacheControlContainsNoCache): Added.
        (WebCore::ResourceResponseBase::cacheControlContainsMustRevalidate): Added.
        (WebCore::ResourceResponseBase::ResourceResponseBase): Updated since I removed
        m_haveParsedCacheControl and renamed m_haveParsedCacheControlHeader to remove
        the imprecise use of the term "header".

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@39383 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index ecf0826..6adc009 100644 (file)
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,33 @@
+2008-12-18  Darin Adler  <darin@apple.com>
+
+        Reviewed by Sam Weinig.
+
+        - fix <rdar://problem/6449841> reduce memory use of ResourceResponseBase by removing two maps
+
+        We were parsing the cache-control and pragma header field values into maps.
+        I changed that so instead we only cache two bits with the data we were actually
+        using. Later we might want to move this responsibility out of this class entirely;
+        we can cache it at the higher level instead.
+
+        * loader/CachedResource.cpp:
+        (WebCore::CachedResource::mustRevalidate): Changed to call a specific API to get at
+        the bits in quest instead of a general "cache control directives" API.
+
+        * platform/network/ResourceResponseBase.cpp:
+        (WebCore::ResourceResponseBase::setHTTPHeaderField): Remove the logic for the
+        pragma header field since no one is using this for now.
+        (WebCore::ResourceResponseBase::parseCacheControlDirectives): Eliminated the return
+        value and made this function have side effects only. Changed it so it's the caller's
+        responsibility to check m_haveParsedCacheControl. Set m_cacheControlContainsNoCache
+        and m_cacheControlContainsMustRevalidate rather than keeping a map around.
+
+        * platform/network/ResourceResponseBase.h:
+        (WebCore::ResourceResponseBase::cacheControlContainsNoCache): Added.
+        (WebCore::ResourceResponseBase::cacheControlContainsMustRevalidate): Added.
+        (WebCore::ResourceResponseBase::ResourceResponseBase): Updated since I removed
+        m_haveParsedCacheControl and renamed m_haveParsedCacheControlHeader to remove
+        the imprecise use of the term "header".
+
 2008-12-18  Steve Falkenburg  <sfalken@apple.com>
 
         Build fix.

diff --git a/WebCore/loader/CachedResource.cpp b/WebCore/loader/CachedResource.cpp
index 9b8f6d1..a481223 100644 (file)
--- a/WebCore/loader/CachedResource.cpp
+++ b/WebCore/loader/CachedResource.cpp
@@ -3,7 +3,7 @@
     Copyright (C) 2001 Dirk Mueller (mueller@kde.org)
     Copyright (C) 2002 Waldo Bastian (bastian@kde.org)
     Copyright (C) 2006 Samuel Weinig (sam.weinig@gmail.com)
-    Copyright (C) 2004, 2005, 2006, 2007 Apple Inc. All rights reserved.
+    Copyright (C) 2004, 2005, 2006, 2007, 2008 Apple Inc. All rights reserved.
 
     This library is free software; you can redistribute it and/or
     modify it under the terms of the GNU Library General Public
@@ -310,12 +310,10 @@ bool CachedResource::mustRevalidate(CachePolicy cachePolicy) const
     if (m_loading)
         return false;
 
-    const CacheControlDirectiveMap& cacheControlDirectives = m_response.parseCacheControlDirectives();
-
     // FIXME: Also look at max-age, min-fresh, max-stale in Cache-Control
     if (cachePolicy == CachePolicyCache)
-        return !cacheControlDirectives.isEmpty() && (cacheControlDirectives.contains("no-cache") || (isExpired() && cacheControlDirectives.contains("must-revalidate")));
-    return isExpired() || cacheControlDirectives.contains("no-cache");
+        return m_response.cacheControlContainsNoCache() || (isExpired() && m_response.cacheControlContainsMustRevalidate());
+    return isExpired() || m_response.cacheControlContainsNoCache();
 }
 
 bool CachedResource::makePurgeable(bool purgeable) 

diff --git a/WebCore/platform/network/ResourceResponseBase.cpp b/WebCore/platform/network/ResourceResponseBase.cpp
index 6d90efa..fb7f5c7 100644 (file)
--- a/WebCore/platform/network/ResourceResponseBase.cpp
+++ b/WebCore/platform/network/ResourceResponseBase.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2006 Apple Computer, Inc.  All rights reserved.
+ * Copyright (C) 2006, 2008 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -25,6 +25,7 @@
 
 #include "config.h"
 #include "ResourceResponseBase.h"
+
 #include "ResourceResponse.h"
 
 namespace WebCore {
@@ -157,10 +158,7 @@ void ResourceResponseBase::setHTTPHeaderField(const AtomicString& name, const St
     lazyInit();
 
     if (equalIgnoringCase(name, "cache-control"))
-        m_haveParsedCacheControlHeader = false;
-    else if (equalIgnoringCase(name, "pragma"))
-        m_haveParsedPragmaHeader = false;
-
+        m_haveParsedCacheControl = false;
     m_httpHeaderFields.set(name, value);
 }
 
@@ -168,73 +166,42 @@ const HTTPHeaderMap& ResourceResponseBase::httpHeaderFields() const
 {
     lazyInit();
 
-    return m_httpHeaderFields; 
+    return m_httpHeaderFields;
 }
 
-const PragmaDirectiveMap& ResourceResponseBase::parsePragmaDirectives() const
+void ResourceResponseBase::parseCacheControlDirectives() const
 {
-    lazyInit();
-
-    if (m_haveParsedPragmaHeader)
-        return m_pragmaDirectiveMap;
-
-    m_pragmaDirectiveMap.clear();
-    m_haveParsedPragmaHeader = true;
-
-    const String pragmaHeader = httpHeaderField("Pragma");
-    if (pragmaHeader.isEmpty())
-        return m_pragmaDirectiveMap;
-
-    Vector<pair<String, String> > directives;
-    parseCacheHeader(pragmaHeader, directives);
+    ASSERT(!m_haveParsedCacheControl);
 
-    unsigned max = directives.size();
-    for (unsigned i = 0; i < max; ++i)
-        m_pragmaDirectiveMap.set(directives[i].first, directives[i].second);
-
-    return m_pragmaDirectiveMap;
-}
-
-const CacheControlDirectiveMap& ResourceResponseBase::parseCacheControlDirectives() const
-{
     lazyInit();
 
-    if (m_haveParsedCacheControlHeader)
-        return m_cacheControlDirectiveMap;
+    m_haveParsedCacheControl = true;
+    m_cacheControlContainsMustRevalidate = false;
+    m_cacheControlContainsNoCache = false;
 
-    m_cacheControlDirectiveMap.clear();
-    m_haveParsedCacheControlHeader = true;
+    String cacheControlValue = httpHeaderField("cache-control");
+    if (cacheControlValue.isEmpty())
+        return;
 
-    const String cacheControlHeader = httpHeaderField("Cache-Control");
-    if (cacheControlHeader.isEmpty())
-        return m_cacheControlDirectiveMap;
+    // FIXME: It would probably be much more efficient to parse this without creating all these data structures.
 
     Vector<pair<String, String> > directives;
-    parseCacheHeader(cacheControlHeader, directives);
+    parseCacheHeader(cacheControlValue, directives);
 
-    unsigned iMax = directives.size();
-    for (unsigned i = 0; i < iMax; ++i) {
+    size_t directivesSize = directives.size();
+    for (size_t i = 0; i < directivesSize; ++i) {
         Vector<String> directiveValues;
         if ((equalIgnoringCase(directives[i].first, "private") || equalIgnoringCase(directives[i].first, "no-cache")) && !directives[i].second.isEmpty())
             parseCacheControlDirectiveValues(directives[i].second, directiveValues);
         else
             directiveValues.append(directives[i].second);
-
-        if (m_cacheControlDirectiveMap.contains(directives[i].first)) {
-            CacheControlDirectiveMap::iterator entry = m_cacheControlDirectiveMap.find(directives[i].first);
-            unsigned jMax = directiveValues.size();
-            for (unsigned j = 0; j < jMax; ++j)
-                entry->second.add(directiveValues[j]);
-        } else {
-            HashSet<String> values;
-            unsigned jMax = directiveValues.size();
-            for (unsigned j = 0; j < jMax; ++j)
-                values.add(directiveValues[j]);
-            m_cacheControlDirectiveMap.set(directives[i].first, values);
+        for (size_t i = 0; i < directiveValues.size(); ++i) {
+            if (equalIgnoringCase(directiveValues[i], "no-cache"))
+                m_cacheControlContainsNoCache = true;
+            else if (equalIgnoringCase(directiveValues[i], "must-revalidate"))
+                m_cacheControlContainsMustRevalidate = true;
         }
     }
-
-    return m_cacheControlDirectiveMap;
 }
 
 bool ResourceResponseBase::isAttachment() const

diff --git a/WebCore/platform/network/ResourceResponseBase.h b/WebCore/platform/network/ResourceResponseBase.h
index 69492b9..5f91d05 100644 (file)
--- a/WebCore/platform/network/ResourceResponseBase.h
+++ b/WebCore/platform/network/ResourceResponseBase.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2006 Apple Computer, Inc.  All rights reserved.
+ * Copyright (C) 2006, 2008 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -31,15 +31,11 @@
 
 namespace WebCore {
 
-typedef HashMap<String, HashSet<String>, CaseFoldingHash> CacheControlDirectiveMap;
-typedef HashMap<String, String, CaseFoldingHash> PragmaDirectiveMap;
-
 class ResourceResponse;
 
 // Do not use this class directly, use the class ResponseResponse instead
 class ResourceResponseBase {
- public:
-
+public:
     bool isNull() const { return m_isNull; }
     bool isHTTP() const;
 
@@ -69,9 +65,6 @@ class ResourceResponseBase {
     void setHTTPHeaderField(const AtomicString& name, const String& value);
     const HTTPHeaderMap& httpHeaderFields() const;
 
-    const PragmaDirectiveMap& parsePragmaDirectives() const;
-    const CacheControlDirectiveMap& parseCacheControlDirectives() const;
-
     bool isMultipart() const { return mimeType() == "multipart/x-mixed-replace"; }
 
     bool isAttachment() const;
@@ -82,17 +75,29 @@ class ResourceResponseBase {
     void setLastModifiedDate(time_t);
     time_t lastModifiedDate() const;
 
+    bool cacheControlContainsNoCache() const
+    {
+        if (!m_haveParsedCacheControl)
+            parseCacheControlDirectives();
+        return m_cacheControlContainsMustRevalidate;
+    }
+    bool cacheControlContainsMustRevalidate() const
+    {
+        if (!m_haveParsedCacheControl)
+            parseCacheControlDirectives();
+        return m_cacheControlContainsMustRevalidate;
+    }
+
     static bool compare(const ResourceResponse& a, const ResourceResponse& b);
 
- protected:
+protected:
     ResourceResponseBase()  
         : m_expectedContentLength(0)
         , m_httpStatusCode(0)
         , m_expirationDate(0)
         , m_lastModifiedDate(0)
         , m_isNull(true)
-        , m_haveParsedCacheControlHeader(false)
-        , m_haveParsedPragmaHeader(false)
+        , m_haveParsedCacheControl(false)
     {
     }
 
@@ -106,15 +111,14 @@ class ResourceResponseBase {
         , m_expirationDate(0)
         , m_lastModifiedDate(0)
         , m_isNull(false)
-        , m_haveParsedCacheControlHeader(false)
-        , m_haveParsedPragmaHeader(false)
+        , m_haveParsedCacheControl(false)
     {
     }
 
     void lazyInit() const;
 
     // The ResourceResponse subclass may "shadow" this method to lazily initialize platform specific fields
-    void platformLazyInit() {}
+    void platformLazyInit() { }
 
     // The ResourceResponse subclass may "shadow" this method to compare platform specific fields
     static bool platformCompare(const ResourceResponse& a, const ResourceResponse& b) { return true; }
@@ -129,11 +133,14 @@ class ResourceResponseBase {
     HTTPHeaderMap m_httpHeaderFields;
     time_t m_expirationDate;
     time_t m_lastModifiedDate;
-    bool m_isNull:1;
-    mutable bool m_haveParsedCacheControlHeader:1;
-    mutable bool m_haveParsedPragmaHeader:1;
-    mutable CacheControlDirectiveMap m_cacheControlDirectiveMap;
-    mutable PragmaDirectiveMap m_pragmaDirectiveMap;
+    bool m_isNull : 1;
+
+private:
+    void parseCacheControlDirectives() const;
+
+    mutable bool m_haveParsedCacheControl : 1;
+    mutable bool m_cacheControlContainsMustRevalidate : 1;
+    mutable bool m_cacheControlContainsNoCache : 1;
 };
 
 inline bool operator==(const ResourceResponse& a, const ResourceResponse& b) { return ResourceResponseBase::compare(a, b); }