Modified: trunk/Source/WebKit2/ChangeLog (187957 => 187958)
--- trunk/Source/WebKit2/ChangeLog 2015-08-05 10:25:41 UTC (rev 187957)
+++ trunk/Source/WebKit2/ChangeLog 2015-08-05 10:27:18 UTC (rev 187958)
@@ -1,3 +1,36 @@
+2015-08-04 Antti Koivisto <an...@apple.com>
+
+ Network cache fetches should have timeout
+ https://bugs.webkit.org/show_bug.cgi?id=147631
+
+ Reviewed by Andreas Kling.
+
+ System might be under heavy I/O pressure. If it takes long time to get data from disk we should load from network instead.
+
+ This patch introduces 1.5s timeout for disk reads. If the last dispatched cache read takes longer than that we cancel all
+ active and pending reads and just load those resources from network.
+
+ * NetworkProcess/cache/NetworkCacheStorage.cpp:
+ (WebKit::NetworkCache::Storage::ReadOperation::ReadOperation):
+ (WebKit::NetworkCache::Storage::ReadOperation::cancel):
+ (WebKit::NetworkCache::Storage::ReadOperation::finish):
+
+ Factor to functions.
+
+ (WebKit::NetworkCache::Storage::Storage):
+ (WebKit::NetworkCache::Storage::updateFileModificationTime):
+ (WebKit::NetworkCache::Storage::dispatchReadOperation):
+ (WebKit::NetworkCache::Storage::finishReadOperation):
+ (WebKit::NetworkCache::Storage::cancelAllReadOperations):
+ (WebKit::NetworkCache::Storage::dispatchPendingReadOperations):
+ (WebKit::NetworkCache::Storage::dispatchPendingWriteOperations):
+
+ Also make dispatch functions transfer the operation ownership.
+
+ (WebKit::NetworkCache::shouldStoreBodyAsBlob):
+ (WebKit::NetworkCache::Storage::dispatchWriteOperation):
+ * NetworkProcess/cache/NetworkCacheStorage.h:
+
2015-08-04 Matt Rajca <mra...@apple.com>
Media Session: add a focus manager that WebKit clients can use to access the focused media element
Modified: trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheStorage.cpp (187957 => 187958)
--- trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheStorage.cpp 2015-08-05 10:25:41 UTC (rev 187957)
+++ trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheStorage.cpp 2015-08-05 10:27:18 UTC (rev 187958)
@@ -56,6 +56,9 @@
, completionHandler(completionHandler)
{ }
+ void cancel();
+ bool finish();
+
const Key key;
const RetrieveCompletionHandler completionHandler;
@@ -63,8 +66,34 @@
SHA1::Digest expectedBodyHash;
BlobStorage::Blob resultBodyBlob;
std::atomic<unsigned> activeCount { 0 };
+ bool isCanceled { false };
};
+void Storage::ReadOperation::cancel()
+{
+ ASSERT(RunLoop::isMain());
+
+ if (isCanceled)
+ return;
+ isCanceled = true;
+ completionHandler(nullptr);
+}
+
+bool Storage::ReadOperation::finish()
+{
+ ASSERT(RunLoop::isMain());
+
+ if (isCanceled)
+ return false;
+ if (resultRecord && resultRecord->body.isNull()) {
+ if (resultBodyBlob.hash == expectedBodyHash)
+ resultRecord->body = resultBodyBlob.data;
+ else
+ resultRecord = nullptr;
+ }
+ return completionHandler(WTF::move(resultRecord));
+}
+
struct Storage::WriteOperation {
WTF_MAKE_FAST_ALLOCATED;
public:
@@ -147,6 +176,7 @@
Storage::Storage(const String& baseDirectoryPath)
: m_basePath(baseDirectoryPath)
, m_recordsPath(makeRecordsDirectoryPath(baseDirectoryPath))
+ , m_readOperationTimeoutTimer(*this, &Storage::cancelAllReadOperations)
, m_writeOperationDispatchTimer(*this, &Storage::dispatchPendingWriteOperations)
, m_ioQueue(WorkQueue::create("com.apple.WebKit.Cache.Storage", WorkQueue::Type::Concurrent))
, m_backgroundIOQueue(WorkQueue::create("com.apple.WebKit.Cache.Storage.background", WorkQueue::Type::Concurrent, WorkQueue::QOS::Background))
@@ -486,11 +516,17 @@
});
}
-void Storage::dispatchReadOperation(ReadOperation& readOperation)
+void Storage::dispatchReadOperation(std::unique_ptr<ReadOperation> readOperationPtr)
{
ASSERT(RunLoop::isMain());
- ASSERT(m_activeReadOperations.contains(&readOperation));
+ auto& readOperation = *readOperationPtr;
+ m_activeReadOperations.add(WTF::move(readOperationPtr));
+
+ // I/O pressure may make disk operations slow. If they start taking very long time we rather go to network.
+ const auto readTimeout = 1500_ms;
+ m_readOperationTimeoutTimer.startOneShot(readTimeout);
+
bool shouldGetBodyBlob = !m_bodyFilter || m_bodyFilter->mayContain(readOperation.key.hash());
ioQueue().dispatch([this, &readOperation, shouldGetBodyBlob] {
@@ -524,26 +560,43 @@
return;
RunLoop::main().dispatch([this, &readOperation] {
- if (readOperation.resultRecord && readOperation.resultRecord->body.isNull()) {
- if (readOperation.resultBodyBlob.hash == readOperation.expectedBodyHash)
- readOperation.resultRecord->body = readOperation.resultBodyBlob.data;
- else
- readOperation.resultRecord = nullptr;
- }
-
- bool success = readOperation.completionHandler(WTF::move(readOperation.resultRecord));
+ bool success = readOperation.finish();
if (success)
updateFileModificationTime(recordPathForKey(readOperation.key));
- else
+ else if (!readOperation.isCanceled)
remove(readOperation.key);
+
ASSERT(m_activeReadOperations.contains(&readOperation));
m_activeReadOperations.remove(&readOperation);
+
+ if (m_activeReadOperations.isEmpty())
+ m_readOperationTimeoutTimer.stop();
+
dispatchPendingReadOperations();
LOG(NetworkCacheStorage, "(NetworkProcess) read complete success=%d", success);
});
}
+void Storage::cancelAllReadOperations()
+{
+ ASSERT(RunLoop::isMain());
+
+ for (auto& readOperation : m_activeReadOperations)
+ readOperation->cancel();
+
+ size_t pendingCount = 0;
+ for (int priority = maximumRetrievePriority; priority >= 0; --priority) {
+ auto& pendingRetrieveQueue = m_pendingReadOperationsByPriority[priority];
+ pendingCount += pendingRetrieveQueue.size();
+ for (auto it = pendingRetrieveQueue.rbegin(), end = pendingRetrieveQueue.rend(); it != end; ++it)
+ (*it)->cancel();
+ pendingRetrieveQueue.clear();
+ }
+
+ LOG(NetworkCacheStorage, "(NetworkProcess) retrieve timeout, canceled %u active and %zu pending", m_activeReadOperations.size(), pendingCount);
+}
+
void Storage::dispatchPendingReadOperations()
{
ASSERT(RunLoop::isMain());
@@ -558,10 +611,7 @@
auto& pendingRetrieveQueue = m_pendingReadOperationsByPriority[priority];
if (pendingRetrieveQueue.isEmpty())
continue;
- auto readOperation = pendingRetrieveQueue.takeLast();
- auto& read = *readOperation;
- m_activeReadOperations.add(WTF::move(readOperation));
- dispatchReadOperation(read);
+ dispatchReadOperation(pendingRetrieveQueue.takeLast());
}
}
@@ -591,11 +641,7 @@
LOG(NetworkCacheStorage, "(NetworkProcess) limiting parallel writes");
return;
}
- auto writeOperation = m_pendingWriteOperations.takeLast();
- auto& write = *writeOperation;
- m_activeWriteOperations.add(WTF::move(writeOperation));
-
- dispatchWriteOperation(write);
+ dispatchWriteOperation(m_pendingWriteOperations.takeLast());
}
}
@@ -605,11 +651,13 @@
return bodyData.size() > maximumInlineBodySize;
}
-void Storage::dispatchWriteOperation(WriteOperation& writeOperation)
+void Storage::dispatchWriteOperation(std::unique_ptr<WriteOperation> writeOperationPtr)
{
ASSERT(RunLoop::isMain());
- ASSERT(m_activeWriteOperations.contains(&writeOperation));
+ auto& writeOperation = *writeOperationPtr;
+ m_activeWriteOperations.add(WTF::move(writeOperationPtr));
+
// This was added already when starting the store but filter might have been wiped.
addToRecordFilter(writeOperation.record.key);
Modified: trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheStorage.h (187957 => 187958)
--- trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheStorage.h 2015-08-05 10:25:41 UTC (rev 187957)
+++ trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheStorage.h 2015-08-05 10:27:18 UTC (rev 187958)
@@ -107,12 +107,13 @@
void shrink();
struct ReadOperation;
- void dispatchReadOperation(ReadOperation&);
+ void dispatchReadOperation(std::unique_ptr<ReadOperation>);
void dispatchPendingReadOperations();
void finishReadOperation(ReadOperation&);
+ void cancelAllReadOperations();
struct WriteOperation;
- void dispatchWriteOperation(WriteOperation&);
+ void dispatchWriteOperation(std::unique_ptr<WriteOperation>);
void dispatchPendingWriteOperations();
void finishWriteOperation(WriteOperation&);
@@ -150,6 +151,7 @@
static const int maximumRetrievePriority = 4;
Deque<std::unique_ptr<ReadOperation>> m_pendingReadOperationsByPriority[maximumRetrievePriority + 1];
HashSet<std::unique_ptr<ReadOperation>> m_activeReadOperations;
+ WebCore::Timer m_readOperationTimeoutTimer;
Deque<std::unique_ptr<WriteOperation>> m_pendingWriteOperations;
HashSet<std::unique_ptr<WriteOperation>> m_activeWriteOperations;
