Index: net/http/http_cache_transaction.cc |
diff --git a/net/http/http_cache_transaction.cc b/net/http/http_cache_transaction.cc |
index ede3541aa4010750256d9acb89f3140bf6938def..c83a25aa03adb0a1a9721b6188c4d779465583e4 100644 |
--- a/net/http/http_cache_transaction.cc |
+++ b/net/http/http_cache_transaction.cc |
@@ -181,6 +181,7 @@ HttpCache::Transaction::Transaction(RequestPriority priority, HttpCache* cache) |
couldnt_conditionalize_request_(false), |
bypass_lock_for_test_(false), |
fail_conditionalization_for_test_(false), |
+ write_this_response_(false), |
io_buf_len_(0), |
read_offset_(0), |
effective_load_flags_(0), |
@@ -600,7 +601,7 @@ size_t HttpCache::Transaction::EstimateMemoryUsage() const { |
// Start(): |
// GetBackend* -> InitEntry -> OpenEntry* -> AddToEntry* -> CacheReadResponse* |
// -> CacheDispatchValidation -> BeginPartialCacheValidation() -> |
-// BeginCacheValidation() -> SetupEntryForRead() |
+// BeginCacheValidation() -> SetupEntryForRead() -> WaitBeforeRead* |
// |
// Read(): |
// CacheReadData* |
@@ -612,7 +613,7 @@ size_t HttpCache::Transaction::EstimateMemoryUsage() const { |
// BeginCacheValidation() -> SendRequest* -> SuccessfulSendRequest -> |
// UpdateCachedResponse -> CacheWriteUpdatedResponse* -> |
// UpdateCachedResponseComplete -> OverwriteCachedResponse -> |
-// PartialHeadersReceived |
+// PartialHeadersReceived -> WaitBeforeRead* |
// |
// Read(): |
// CacheReadData* |
@@ -712,6 +713,30 @@ size_t HttpCache::Transaction::EstimateMemoryUsage() const { |
// Like examples 2-4, only CacheToggleUnusedSincePrefetch* is inserted between |
// CacheReadResponse* and CacheDispatchValidation. |
int HttpCache::Transaction::DoLoop(int result) { |
+ int rv = DoLoopImpl(result); |
+ |
+ // If its the end of the Start() state machine, the transaction might have to |
+ // wait before it can read from the cache if there is another transaction |
+ // writing to the cache currently. This is being done to enable parallelizing |
+ // the validation phase of a transaction while another transaction is still |
+ // writing the response to the cache. |
+ // Checking reading_ will make sure that we do not enter the wait state for |
+ // partial requests that have already started reading and re-entered the |
+ // start() state machine. |
+ if (!reading_ && rv == OK) { |
jkarlin
2017/03/07 16:45:42
How many ways can we get here? Can we just call 'r
shivanisha
2017/03/08 21:42:13
There are a lot of conditional paths that can lead
|
+ next_state_ = STATE_WAIT_BEFORE_READ; |
+ rv = DoLoopImpl(rv); |
+ } |
+ |
+ if (rv != ERR_IO_PENDING && !callback_.is_null()) { |
+ read_buf_ = NULL; // Release the buffer before invoking the callback. |
jkarlin
2017/03/07 16:45:42
s/NULL/nullptr/
shivanisha
2017/03/08 21:42:13
done
|
+ base::ResetAndReturn(&callback_).Run(rv); |
+ } |
+ |
+ return rv; |
+} |
+ |
+int HttpCache::Transaction::DoLoopImpl(int result) { |
DCHECK(next_state_ != STATE_NONE); |
int rv = result; |
@@ -851,6 +876,13 @@ int HttpCache::Transaction::DoLoop(int result) { |
case STATE_CACHE_READ_METADATA_COMPLETE: |
rv = DoCacheReadMetadataComplete(rv); |
break; |
+ case STATE_WAIT_BEFORE_READ: |
+ DCHECK_EQ(OK, rv); |
+ rv = DoWaitBeforeRead(); |
+ break; |
+ case STATE_WAIT_BEFORE_READ_COMPLETE: |
+ rv = DoWaitBeforeReadComplete(rv); |
+ break; |
case STATE_NETWORK_READ: |
DCHECK_EQ(OK, rv); |
rv = DoNetworkRead(); |
@@ -885,11 +917,6 @@ int HttpCache::Transaction::DoLoop(int result) { |
} |
} while (rv != ERR_IO_PENDING && next_state_ != STATE_NONE); |
- if (rv != ERR_IO_PENDING && !callback_.is_null()) { |
- read_buf_ = NULL; // Release the buffer before invoking the callback. |
- base::ResetAndReturn(&callback_).Run(rv); |
- } |
- |
return rv; |
} |
@@ -1497,7 +1524,7 @@ int HttpCache::Transaction::DoSuccessfulSendRequest() { |
int ret = cache_->DoomEntry(cache_key_, NULL); |
DCHECK_EQ(OK, ret); |
} |
- cache_->DoneWritingToEntry(entry_, true); |
+ cache_->DoneWritingToEntry(entry_, true, this); |
entry_ = NULL; |
mode_ = NONE; |
} |
@@ -1530,6 +1557,24 @@ int HttpCache::Transaction::DoSuccessfulSendRequest() { |
} |
next_state_ = STATE_OVERWRITE_CACHED_RESPONSE; |
+ |
+ if (!entry_) |
+ return OK; |
+ |
+ if (entry_->writer == this && request_->method != "HEAD") { |
+ write_this_response_ = true; |
+ return OK; |
+ } |
+ |
+ // Invalidate any current entry with a successful response. |
+ if (new_response->headers->response_code() != 304) { |
+ DCHECK_EQ(entry_->validating_transaction, this); |
+ cache_->OnValidationNoMatch(entry_, this); |
+ entry_ = nullptr; |
+ mode_ = NONE; |
+ return OK; |
+ } |
+ |
return OK; |
} |
@@ -1599,8 +1644,8 @@ int HttpCache::Transaction::DoUpdateCachedResponseComplete(int result) { |
} else if (entry_ && !handling_206_) { |
DCHECK_EQ(READ_WRITE, mode_); |
if (!partial_ || partial_->IsLastRange()) { |
- cache_->ConvertWriterToReader(entry_); |
- mode_ = READ; |
+ if (cache_->ConvertWriterToReader(entry_, this)) |
+ mode_ = READ; |
} |
// We no longer need the network transaction, so destroy it. |
ResetNetworkTransaction(); |
@@ -1736,6 +1781,60 @@ int HttpCache::Transaction::DoPartialHeadersReceived() { |
return OK; |
} |
+int HttpCache::Transaction::DoWaitBeforeRead() { |
+ if (!entry_) |
+ return OK; |
+ |
+ next_state_ = STATE_WAIT_BEFORE_READ_COMPLETE; |
+ |
+ // If this is the writer, give chance to another transaction to start |
+ // validation. |
+ if (write_this_response_) { |
jkarlin
2017/03/07 16:45:42
Seems clearer to call entry_->writer == this && re
shivanisha
2017/03/08 21:42:13
Apart from the conditions entry_->writer == this &
|
+ cache_->OnValidationMatch(entry_, this, true); |
jkarlin
2017/03/07 16:45:42
OnValidationMatch(entry_, this, true /* write_this
shivanisha
2017/03/08 21:42:13
N/A after the code refactoring of OnValidationMatc
|
+ return OK; |
+ } |
+ |
+ // If the transaction was converted to a reader but not in readers yet, then |
+ // it must be added to the queue. |
+ bool should_wait = false; |
+ if (entry_->writer != this && entry_->validating_transaction != this && |
+ !entry_->IsReader(this)) |
+ should_wait = true; |
+ |
+ // Notify cache_ about completed validation so that another transaction can |
+ // start validation. |
+ if (entry_->validating_transaction == this && response_.headers && |
+ response_.headers->response_code() == 304) |
+ should_wait = true; |
+ |
+ if (should_wait) |
+ return cache_->OnValidationMatch(entry_, this, false); |
jkarlin
2017/03/07 16:45:42
OnValidationMatch(entry_, this, false /* write_thi
shivanisha
2017/03/08 21:42:13
N/A after the code refactoring of OnValidationMatc
|
+ |
+ return OK; |
+} |
+ |
+int HttpCache::Transaction::DoWaitBeforeReadComplete(int rv) { |
+ if (rv == ERR_CACHE_RACE) { |
+ next_state_ = STATE_INIT_ENTRY; |
+ cache_entry_status_ = CacheEntryStatus::ENTRY_UNDEFINED; |
+ entry_ = nullptr; |
+ return OK; |
+ } |
+ |
+ if (rv != OK) |
+ return rv; |
+ |
+ // If successful then this must be either be the writer or a reader as the |
+ // response must have completely been written to the cache. |
+ if (entry_->writer == this) |
+ return OK; |
+ |
+ mode_ = READ; |
jkarlin
2017/03/07 16:45:42
I'd have thought that the existing code would have
shivanisha
2017/03/08 21:42:12
Simplified DoWaitBeforeRead to not have an additio
|
+ if (network_trans_) |
+ ResetNetworkTransaction(); |
jkarlin
2017/03/07 16:45:42
Likewise, shouldn't this be taken care of already?
shivanisha
2017/03/08 21:42:13
I am looking into this. I am not sure if the could
|
+ return OK; |
+} |
+ |
int HttpCache::Transaction::DoCacheReadMetadata() { |
TRACE_EVENT0("io", "HttpCacheTransaction::DoCacheReadMetadata"); |
DCHECK(entry_); |
@@ -2565,8 +2664,12 @@ int HttpCache::Transaction::SetupEntryForRead() { |
partial_.reset(); |
} |
} |
- cache_->ConvertWriterToReader(entry_); |
- mode_ = READ; |
+ |
+ // This might or might not be able to add it as an active reader based on |
+ // whether this is the active writer or not. If not, it will be added to a |
+ // wait queue in DoWaitBeforeRead. |
+ if (cache_->ConvertWriterToReader(entry_, this)) |
jkarlin
2017/03/07 16:45:42
I see, this is why we had to change mode_ = READ i
shivanisha
2017/03/08 21:42:13
Simplified ConvertWriterToReader so it doesn't do
|
+ mode_ = READ; |
if (request_->method == "HEAD") |
FixHeadersForHead(); |
@@ -2651,7 +2754,7 @@ void HttpCache::Transaction::DoneWritingToEntry(bool success) { |
RecordHistograms(); |
- cache_->DoneWritingToEntry(entry_, success); |
+ cache_->DoneWritingToEntry(entry_, success, this); |
entry_ = NULL; |
mode_ = NONE; // switch to 'pass through' mode |
} |