Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(177)

Unified Diff: net/http/http_cache.h

Issue 2721933002: HttpCache::Transaction layer allowing parallel validation (Closed)
Patch Set: Fixed data race Created 3 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | net/http/http_cache.cc » ('j') | net/http/http_cache.cc » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: net/http/http_cache.h
diff --git a/net/http/http_cache.h b/net/http/http_cache.h
index 635cb92d557d17c6c60ca459f4117deb075f71f7..c0379f608eb2b6b9ed3ff496cbdc3dfec7fd84ed 100644
--- a/net/http/http_cache.h
+++ b/net/http/http_cache.h
@@ -244,10 +244,34 @@ class NET_EXPORT HttpCache : public HttpTransactionFactory,
friend class ViewCacheHelper;
struct PendingOp; // Info for an entry under construction.
+ // To help with testing.
+ friend class MockHttpCache;
+
using TransactionList = std::list<Transaction*>;
using TransactionSet = std::unordered_set<Transaction*>;
typedef std::list<std::unique_ptr<WorkItem>> WorkItemList;
+ // We implement a basic reader/writer lock for the disk cache entry. If there
+ // is a writer, then all transactions must wait to read the body. But the
+ // waiting transactions can start their headers phase in parallel. Headers
+ // phase is allowed for one transaction at a time so that if it doesn't match
+ // the existing headers, remaining transactions do not also try to match the
+ // existing entry in parallel leading to wasted network requests. If the
+ // headers do not match, this entry will be doomed.
+ //
+ // A transaction goes through these state transitions.
+ //
+ // Write mode transactions:
+ // add_to_entry_queue-> headers_transaction -> writer (this is not added to
+ // done_headers_queue for performance and consumer's expectations to be
+ // synchronous in knowing that headers have been received)
+ // add_to_entry_queue-> headers_transaction -> done_headers_queue -> readers
+ // (once the data is written to the cache by another writer)
+ //
+ // Read only transactions:
+ // add_to_entry_queue-> headers_transaction -> done_headers_queue -> readers
+ // (once the data is written to the cache by the writer)
+
struct ActiveEntry {
explicit ActiveEntry(disk_cache::Entry* entry);
~ActiveEntry();
@@ -256,12 +280,36 @@ class NET_EXPORT HttpCache : public HttpTransactionFactory,
// Returns true if no transactions are associated with this entry.
bool HasNoTransactions();
- disk_cache::Entry* disk_entry;
- Transaction* writer;
+ // Returns true if no active readers/writer transactions are associated
+ // with this entry.
+ bool HasNoActiveTransactions();
+
+ disk_cache::Entry* disk_entry = nullptr;
+
+ // Transactions waiting to be added to entry.
+ TransactionList add_to_entry_queue;
+
+ // Transaction currently in the headers phase, either validating the
+ // response or getting new headers. This can exist simultaneously with
+ // writer or readers while validating existing headers.
+ Transaction* headers_transaction = nullptr;
+
+ // Transactions that have completed their headers phase and are waiting
+ // to read the response body or write the response body.
+ TransactionList done_headers_queue;
+
+ // Transaction currently reading from the network and writing to the cache.
+ Transaction* writer = nullptr;
+
+ // Transactions that can only read from the cache. Only one of writer or
+ // readers can exist at a time.
TransactionSet readers;
- TransactionList pending_queue;
- bool will_process_pending_queue;
- bool doomed;
+
+ // The following variables are true if OnProcessQueuedTransactions is posted
+ bool will_process_queued_transactions = false;
+
+ // True if entry is doomed.
+ bool doomed = false;
};
using ActiveEntriesMap =
@@ -342,28 +390,73 @@ class NET_EXPORT HttpCache : public HttpTransactionFactory,
// Destroys an ActiveEntry (active or doomed).
void DestroyEntry(ActiveEntry* entry);
- // Adds a transaction to an ActiveEntry. If this method returns ERR_IO_PENDING
- // the transaction will be notified about completion via its IO callback. This
- // method returns ERR_CACHE_RACE to signal the transaction that it cannot be
- // added to the provided entry, and it should retry the process with another
- // one (in this case, the entry is no longer valid).
- int AddTransactionToEntry(ActiveEntry* entry, Transaction* trans);
+ // Adds a transaction to an ActiveEntry. This method returns ERR_IO_PENDING
+ // and the transaction will be notified about completion via its IO callback.
+ // In a failure case, the callback will be invoked with ERR_CACHE_RACE.
+ int AddTransactionToEntry(ActiveEntry* entry, Transaction* transaction);
+
+ // Transaction invokes this when its response headers phase is complete
+ // If the transaction is responsible for writing the response body,
+ // it becomes the writer and returns OK. In other cases the transaction it
+ // returns ERR_IO_PENDING and the transaction will be notified about
+ // completion via its IO callback. In a failure case, the callback will be
+ // invoked with ERR_CACHE_RACE.
+ int DoneWithResponseHeaders(ActiveEntry* entry, Transaction* transaction);
// Called when the transaction has finished working with this entry. |cancel|
// is true if the operation was cancelled by the caller instead of running
// to completion.
- void DoneWithEntry(ActiveEntry* entry, Transaction* trans, bool cancel);
+ void DoneWithEntry(ActiveEntry* entry, Transaction* transaction, bool cancel);
// Called when the transaction has finished writing to this entry. |success|
// is false if the cache entry should be deleted.
- void DoneWritingToEntry(ActiveEntry* entry, bool success);
+ void DoneWritingToEntry(ActiveEntry* entry,
+ bool success,
+ Transaction* transaction);
// Called when the transaction has finished reading from this entry.
- void DoneReadingFromEntry(ActiveEntry* entry, Transaction* trans);
-
- // Converts the active writer transaction to a reader so that other
- // transactions can start reading from this entry.
- void ConvertWriterToReader(ActiveEntry* entry);
+ void DoneReadingFromEntry(ActiveEntry* entry, Transaction* transaction);
+
+ // Removes and returns all queued transactions in |entry| in FIFO order. This
+ // includes transactions that have completed the headers phase and those that
+ // have not been added to the entry yet in that order. |list| is the output
+ // argument.
+ void RemoveAllQueuedTransactions(ActiveEntry* entry, TransactionList* list);
+
+ // Processes either writer's failure to write response body or
+ // headers_transactions's failure to write headers. Also invoked when headers
+ // transaction's validation result is not a match.
+ void ProcessEntryFailure(ActiveEntry* entry);
+
+ // Resumes processing the queued transactions of |entry|.
+ void ProcessQueuedTransactions(ActiveEntry* entry);
+
+ // Checks if a transaction can be added to the entry. If yes, it will
+ // invoke the IO callback of the transaction. This is a helper function for
+ // OnProcessQueuedTransactions. It will take a transaction from
+ // add_to_entry_queue and make it a headers_transaction, if one doesn't exist
+ // already.
+ void ProcessAddToEntryQueue(ActiveEntry* entry);
+
+ // Invoked when a transaction that has already completed the response headers
+ // phase can resume reading/writing the response body. It will invoke the IO
+ // callback of the transaction. This is a helper function for
+ // OnProcessQueuedTransactions.
+ void ProcessDoneHeadersQueue(ActiveEntry* entry);
+
+ // Returns true if this transaction can write headers to the entry.
+ bool CanTransactionWriteResponseHeaders(ActiveEntry* entry,
+ Transaction* transaction,
+ int response_code) const;
+
+ // Returns true if |transaction| is about to start writing response body or
+ // already started but not yet finished.
+ bool IsTransactionWritingIncomplete(ActiveEntry* entry,
+ Transaction* transaction,
+ const std::string& method) const;
+
+ // Returns true if a transaction is currently writing the response body.
+ bool IsWritingInProgress(ActiveEntry* entry) const;
// Returns the LoadState of the provided pending transaction.
LoadState GetLoadStateForPendingTransaction(const Transaction* trans);
@@ -379,12 +472,10 @@ class NET_EXPORT HttpCache : public HttpTransactionFactory,
// Removes the transaction |trans|, from the pending list of |pending_op|.
bool RemovePendingTransactionFromPendingOp(PendingOp* pending_op,
Transaction* trans);
- // Resumes processing the pending list of |entry|.
- void ProcessPendingQueue(ActiveEntry* entry);
// Events (called via PostTask) ---------------------------------------------
- void OnProcessPendingQueue(ActiveEntry* entry);
+ void OnProcessQueuedTransactions(ActiveEntry* entry);
// Callbacks ----------------------------------------------------------------
« no previous file with comments | « no previous file | net/http/http_cache.cc » ('j') | net/http/http_cache.cc » ('J')

Powered by Google App Engine
This is Rietveld 408576698