- Revision
- 151795
- Author
- beid...@apple.com
- Date
- 2013-06-20 13:12:35 -0700 (Thu, 20 Jun 2013)
Log Message
WebProcess hangs loading eff.org (Waiting forever on a sync XHR, NetworkProcess unable to service it).
<rdar://problem/14112329> and https://bugs.webkit.org/show_bug.cgi?id=117842
Reviewed by Alexey Proskuryakov.
* NetworkProcess/HostRecord.cpp:
(WebKit::removeLoaderFromQueue): Utility to clear a Loader out of a LoaderQueue.
(WebKit::HostRecord::removeLoader): Use removeLoaderFromQueue, and also try to clear the loader from m_syncLoadersPending.
(WebKit::HostRecord::servePendingRequestsForQueue): Small refactoring/cleanup.
(WebKit::HostRecord::limitsRequests): Change so that if the number of loaders in flight is exactly equal to the limit, then
allow serving a synchronous loader to go exactly one beyond the limit.
* NetworkProcess/HostRecord.h:
Make the static maxRequestsInFlightPerHost a member of the NetworkResourceLoadScheduler, instead:
* NetworkProcess/NetworkResourceLoadScheduler.cpp:
(WebKit::NetworkResourceLoadScheduler::NetworkResourceLoadScheduler):
(WebKit::NetworkResourceLoadScheduler::hostForURL):
* NetworkProcess/NetworkResourceLoadScheduler.h:
* NetworkProcess/mac/NetworkResourceLoadSchedulerMac.mm:
(WebKit::NetworkResourceLoadScheduler::platformInitializeMaximumHTTPConnectionCountPerHost): Tell CFNetwork to allow
7 connections per host instead of 6, but tell the scheduler that 6 is still the normal limit.
Modified Paths
Diff
Modified: trunk/Source/WebKit2/ChangeLog (151794 => 151795)
--- trunk/Source/WebKit2/ChangeLog 2013-06-20 20:12:28 UTC (rev 151794)
+++ trunk/Source/WebKit2/ChangeLog 2013-06-20 20:12:35 UTC (rev 151795)
@@ -1,3 +1,28 @@
+2013-06-20 Brady Eidson <beid...@apple.com>
+
+ WebProcess hangs loading eff.org (Waiting forever on a sync XHR, NetworkProcess unable to service it).
+ <rdar://problem/14112329> and https://bugs.webkit.org/show_bug.cgi?id=117842
+
+ Reviewed by Alexey Proskuryakov.
+
+ * NetworkProcess/HostRecord.cpp:
+ (WebKit::removeLoaderFromQueue): Utility to clear a Loader out of a LoaderQueue.
+ (WebKit::HostRecord::removeLoader): Use removeLoaderFromQueue, and also try to clear the loader from m_syncLoadersPending.
+ (WebKit::HostRecord::servePendingRequestsForQueue): Small refactoring/cleanup.
+ (WebKit::HostRecord::limitsRequests): Change so that if the number of loaders in flight is exactly equal to the limit, then
+ allow serving a synchronous loader to go exactly one beyond the limit.
+ * NetworkProcess/HostRecord.h:
+
+ Make the static maxRequestsInFlightPerHost a member of the NetworkResourceLoadScheduler, instead:
+ * NetworkProcess/NetworkResourceLoadScheduler.cpp:
+ (WebKit::NetworkResourceLoadScheduler::NetworkResourceLoadScheduler):
+ (WebKit::NetworkResourceLoadScheduler::hostForURL):
+ * NetworkProcess/NetworkResourceLoadScheduler.h:
+
+ * NetworkProcess/mac/NetworkResourceLoadSchedulerMac.mm:
+ (WebKit::NetworkResourceLoadScheduler::platformInitializeMaximumHTTPConnectionCountPerHost): Tell CFNetwork to allow
+ 7 connections per host instead of 6, but tell the scheduler that 6 is still the normal limit.
+
2013-06-20 Grzegorz Czajkowski <g.czajkow...@samsung.com>
[EFL][WK2] Update text checker documentation.
Modified: trunk/Source/WebKit2/NetworkProcess/HostRecord.cpp (151794 => 151795)
--- trunk/Source/WebKit2/NetworkProcess/HostRecord.cpp 2013-06-20 20:12:28 UTC (rev 151794)
+++ trunk/Source/WebKit2/NetworkProcess/HostRecord.cpp 2013-06-20 20:12:35 UTC (rev 151795)
@@ -76,6 +76,19 @@
loader->setHostRecord(this);
}
+inline bool removeLoaderFromQueue(SchedulableLoader* loader, LoaderQueue& queue)
+{
+ LoaderQueue::iterator end = queue.end();
+ for (LoaderQueue::iterator it = queue.begin(); it != end; ++it) {
+ if (it->get() == loader) {
+ loader->setHostRecord(0);
+ queue.remove(it);
+ return true;
+ }
+ }
+ return false;
+}
+
void HostRecord::removeLoader(SchedulableLoader* loader)
{
ASSERT(isMainThread());
@@ -89,16 +102,13 @@
m_loadersInProgress.remove(i);
return;
}
+
+ if (removeLoaderFromQueue(loader, m_syncLoadersPending))
+ return;
- for (int priority = ResourceLoadPriorityHighest; priority >= ResourceLoadPriorityLowest; --priority) {
- LoaderQueue::iterator end = m_loadersPending[priority].end();
- for (LoaderQueue::iterator it = m_loadersPending[priority].begin(); it != end; ++it) {
- if (it->get() == loader) {
- it->get()->setHostRecord(0);
- m_loadersPending[priority].remove(it);
- return;
- }
- }
+ for (int priority = ResourceLoadPriorityHighest; priority >= ResourceLoadPriorityLowest; --priority) {
+ if (removeLoaderFromQueue(loader, m_loadersPending[priority]))
+ return;
}
}
@@ -132,6 +142,23 @@
void HostRecord::servePendingRequestsForQueue(LoaderQueue& queue, ResourceLoadPriority priority)
{
+ // We only enforce the connection limit for http(s) hosts, which are the only ones with names.
+ bool shouldLimitRequests = !name().isNull();
+
+ // For non-named hosts - everything but http(s) - we should only enforce the limit if the document
+ // isn't done parsing and we don't know all stylesheets yet.
+
+ // FIXME (NetworkProcess): The above comment about document parsing and stylesheets is a holdover
+ // from the WebCore::ResourceLoadScheduler.
+ // The behavior described was at one time important for WebCore's single threadedness.
+ // It's possible that we don't care about it with the NetworkProcess.
+ // We should either decide it's not important and change the above comment, or decide it is
+ // still important and somehow account for it.
+
+ // Very low priority loaders are only handled when no other loaders are in progress.
+ if (shouldLimitRequests && priority == ResourceLoadPriorityVeryLow && !m_loadersInProgress.isEmpty())
+ return;
+
while (!queue.isEmpty()) {
RefPtr<SchedulableLoader> loader = queue.first();
ASSERT(loader->hostRecord() == this);
@@ -143,19 +170,7 @@
continue;
}
- // For named hosts - which are only http(s) hosts - we should always enforce the connection limit.
- // For non-named hosts - everything but http(s) - we should only enforce the limit if the document
- // isn't done parsing and we don't know all stylesheets yet.
-
- // FIXME (NetworkProcess): The above comment about document parsing and stylesheets is a holdover
- // from the WebCore::ResourceLoadScheduler.
- // The behavior described was at one time important for WebCore's single threadedness.
- // It's possible that we don't care about it with the NetworkProcess.
- // We should either decide it's not important and change the above comment, or decide it is
- // still important and somehow account for it.
-
- bool shouldLimitRequests = !name().isNull();
- if (shouldLimitRequests && limitsRequests(priority, loader->connectionToWebProcess()->isSerialLoadingEnabled()))
+ if (shouldLimitRequests && limitsRequests(priority, loader.get()))
return;
m_loadersInProgress.add(loader);
@@ -178,12 +193,37 @@
servePendingRequestsForQueue(m_loadersPending[priority], (ResourceLoadPriority)priority);
}
-bool HostRecord::limitsRequests(ResourceLoadPriority priority, bool serialLoadingEnabled) const
+bool HostRecord::limitsRequests(ResourceLoadPriority priority, SchedulableLoader* loader) const
{
+ ASSERT(loader);
+ ASSERT(loader->connectionToWebProcess());
+
if (priority == ResourceLoadPriorityVeryLow && !m_loadersInProgress.isEmpty())
return true;
- return m_loadersInProgress.size() >= (serialLoadingEnabled ? 1 : m_maxRequestsInFlight);
+ if (loader->connectionToWebProcess()->isSerialLoadingEnabled() && m_loadersInProgress.size() >= 1)
+ return true;
+
+ // If we're exactly at the limit for requests in flight, and this loader is asynchronous, then we're done serving new requests.
+ // The synchronous loader exception handles the case where a sync XHR is made while 6 other requests are already in flight.
+ if (m_loadersInProgress.size() == m_maxRequestsInFlight && !loader->isSynchronous())
+ return true;
+
+ // If we're already past the limit of the number of loaders in flight, we won't even serve new synchronous requests right now.
+ if (m_loadersInProgress.size() > m_maxRequestsInFlight) {
+#ifndef NDEBUG
+ // If we have more loaders in progress than we should, at least one of them had better be synchronous.
+ SchedulableLoaderSet::iterator i = m_loadersInProgress.begin();
+ SchedulableLoaderSet::iterator end = m_loadersInProgress.end();
+ for (; i != end; ++i) {
+ if (i->get()->isSynchronous())
+ break;
+ }
+ ASSERT(i != end);
+#endif
+ return true;
+ }
+ return false;
}
} // namespace WebKit
Modified: trunk/Source/WebKit2/NetworkProcess/HostRecord.h (151794 => 151795)
--- trunk/Source/WebKit2/NetworkProcess/HostRecord.h 2013-06-20 20:12:28 UTC (rev 151794)
+++ trunk/Source/WebKit2/NetworkProcess/HostRecord.h 2013-06-20 20:12:35 UTC (rev 151795)
@@ -39,6 +39,8 @@
class NetworkResourceLoader;
class SchedulableLoader;
class SyncNetworkResourceLoader;
+
+typedef Deque<RefPtr<SchedulableLoader>> LoaderQueue;
typedef uint64_t ResourceLoadIdentifier;
class HostRecord : public RefCounted<HostRecord> {
@@ -64,10 +66,8 @@
private:
HostRecord(const String& name, int maxRequestsInFlight);
- typedef Deque<RefPtr<SchedulableLoader>> LoaderQueue;
-
void servePendingRequestsForQueue(LoaderQueue&, WebCore::ResourceLoadPriority);
- bool limitsRequests(WebCore::ResourceLoadPriority, bool serialLoadingEnabled) const;
+ bool limitsRequests(WebCore::ResourceLoadPriority, SchedulableLoader*) const;
LoaderQueue m_loadersPending[WebCore::ResourceLoadPriorityHighest + 1];
LoaderQueue m_syncLoadersPending;
Modified: trunk/Source/WebKit2/NetworkProcess/NetworkResourceLoadScheduler.cpp (151794 => 151795)
--- trunk/Source/WebKit2/NetworkProcess/NetworkResourceLoadScheduler.cpp 2013-06-20 20:12:28 UTC (rev 151794)
+++ trunk/Source/WebKit2/NetworkProcess/NetworkResourceLoadScheduler.cpp 2013-06-20 20:12:35 UTC (rev 151795)
@@ -17,14 +17,13 @@
namespace WebKit {
static const unsigned maxRequestsInFlightForNonHTTPProtocols = 20;
-static unsigned maxRequestsInFlightPerHost;
NetworkResourceLoadScheduler::NetworkResourceLoadScheduler()
: m_nonHTTPProtocolHost(HostRecord::create(String(), maxRequestsInFlightForNonHTTPProtocols))
, m_requestTimer(this, &NetworkResourceLoadScheduler::requestTimerFired)
{
- maxRequestsInFlightPerHost = platformInitializeMaximumHTTPConnectionCountPerHost();
+ platformInitializeMaximumHTTPConnectionCountPerHost();
}
void NetworkResourceLoadScheduler::scheduleServePendingRequests()
@@ -68,7 +67,7 @@
String hostName = url.host();
HostRecord* host = m_hosts.get(hostName);
if (!host && createHostPolicy == CreateIfNotFound) {
- RefPtr<HostRecord> newHost = HostRecord::create(hostName, maxRequestsInFlightPerHost);
+ RefPtr<HostRecord> newHost = HostRecord::create(hostName, m_maxRequestsInFlightPerHost);
host = newHost.get();
m_hosts.add(hostName, newHost.release());
}
Modified: trunk/Source/WebKit2/NetworkProcess/NetworkResourceLoadScheduler.h (151794 => 151795)
--- trunk/Source/WebKit2/NetworkProcess/NetworkResourceLoadScheduler.h 2013-06-20 20:12:28 UTC (rev 151794)
+++ trunk/Source/WebKit2/NetworkProcess/NetworkResourceLoadScheduler.h 2013-06-20 20:12:35 UTC (rev 151795)
@@ -40,8 +40,8 @@
namespace WebKit {
+class HostRecord;
class SchedulableLoader;
-class HostRecord;
class NetworkResourceLoadScheduler {
WTF_MAKE_NONCOPYABLE(NetworkResourceLoadScheduler); WTF_MAKE_FAST_ALLOCATED;
@@ -78,7 +78,7 @@
void scheduleServePendingRequests();
void requestTimerFired(WebCore::Timer<NetworkResourceLoadScheduler>*);
- unsigned platformInitializeMaximumHTTPConnectionCountPerHost();
+ void platformInitializeMaximumHTTPConnectionCountPerHost();
static void removeScheduledLoaders(void* context);
void removeScheduledLoaders();
@@ -97,6 +97,8 @@
Mutex m_loadersToRemoveMutex;
Vector<RefPtr<SchedulableLoader>> m_loadersToRemove;
+
+ unsigned m_maxRequestsInFlightPerHost;
};
} // namespace WebKit
Modified: trunk/Source/WebKit2/NetworkProcess/mac/NetworkResourceLoadSchedulerMac.mm (151794 => 151795)
--- trunk/Source/WebKit2/NetworkProcess/mac/NetworkResourceLoadSchedulerMac.mm 2013-06-20 20:12:28 UTC (rev 151794)
+++ trunk/Source/WebKit2/NetworkProcess/mac/NetworkResourceLoadSchedulerMac.mm 2013-06-20 20:12:35 UTC (rev 151795)
@@ -37,13 +37,15 @@
namespace WebKit {
-unsigned NetworkResourceLoadScheduler::platformInitializeMaximumHTTPConnectionCountPerHost()
+void NetworkResourceLoadScheduler::platformInitializeMaximumHTTPConnectionCountPerHost()
{
wkInitializeMaximumHTTPConnectionCountPerHost = WKInitializeMaximumHTTPConnectionCountPerHost;
wkSetHTTPPipeliningMaximumPriority = WKSetHTTPPipeliningMaximumPriority;
wkSetHTTPPipeliningMinimumFastLanePriority = WKSetHTTPPipeliningMinimumFastLanePriority;
- static const unsigned preferredConnectionCount = 6;
+ // Our preferred connection-per-host limit is the standard 6, but we need to let CFNetwork handle a 7th
+ // in case a synchronous XHRs is made while 6 loads are already outstanding.
+ static const unsigned preferredConnectionCount = 7;
static const unsigned unlimitedConnectionCount = 10000;
// Always set the connection count per host, even when pipelining.
@@ -60,10 +62,13 @@
wkSetHTTPPipeliningMinimumFastLanePriority(toHTTPPipeliningPriority(ResourceLoadPriorityMedium));
// When pipelining do not rate-limit requests sent from WebCore since CFNetwork handles that.
- return unlimitedConnectionCount;
+ m_maxRequestsInFlightPerHost = unlimitedConnectionCount;
+
+ return;
}
- return maximumHTTPConnectionCountPerHost;
+ // We've asked for one more connection per host than we intend to use in most cases so synch XHRs can bypass that limit.
+ m_maxRequestsInFlightPerHost = maximumHTTPConnectionCountPerHost - 1;
}
} // namespace WebKit
