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

Side by Side Diff: net/http/http_cache_transaction.cc

Issue 1476123002: Bounce all requests off the cache lock after 25ms. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: mmenke comments Created 5 years 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 unified diff | Download patch
« no previous file with comments | « net/http/http_cache_transaction.h ('k') | net/http/http_cache_unittest.cc » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 // Copyright (c) 2012 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "net/http/http_cache_transaction.h" 5 #include "net/http/http_cache_transaction.h"
6 6
7 #include "build/build_config.h" // For OS_POSIX 7 #include "build/build_config.h" // For OS_POSIX
8 8
9 #if defined(OS_POSIX) 9 #if defined(OS_POSIX)
10 #include <unistd.h> 10 #include <unistd.h>
(...skipping 247 matching lines...) Expand 10 before | Expand all | Expand 10 after
258 reading_(false), 258 reading_(false),
259 invalid_range_(false), 259 invalid_range_(false),
260 truncated_(false), 260 truncated_(false),
261 is_sparse_(false), 261 is_sparse_(false),
262 range_requested_(false), 262 range_requested_(false),
263 handling_206_(false), 263 handling_206_(false),
264 cache_pending_(false), 264 cache_pending_(false),
265 done_reading_(false), 265 done_reading_(false),
266 vary_mismatch_(false), 266 vary_mismatch_(false),
267 couldnt_conditionalize_request_(false), 267 couldnt_conditionalize_request_(false),
268 bypass_lock_for_test_(false),
269 fail_conditionalization_for_test_(false), 268 fail_conditionalization_for_test_(false),
270 io_buf_len_(0), 269 io_buf_len_(0),
271 read_offset_(0), 270 read_offset_(0),
272 effective_load_flags_(0), 271 effective_load_flags_(0),
273 write_len_(0), 272 write_len_(0),
274 transaction_pattern_(PATTERN_UNDEFINED), 273 transaction_pattern_(PATTERN_UNDEFINED),
275 total_received_bytes_(0), 274 total_received_bytes_(0),
276 total_sent_bytes_(0), 275 total_sent_bytes_(0),
277 websocket_handshake_stream_base_create_helper_(NULL), 276 websocket_handshake_stream_base_create_helper_(NULL),
278 weak_factory_(this) { 277 weak_factory_(this) {
(...skipping 881 matching lines...) Expand 10 before | Expand all | Expand 10 after
1160 1159
1161 int HttpCache::Transaction::DoAddToEntry() { 1160 int HttpCache::Transaction::DoAddToEntry() {
1162 DCHECK(new_entry_); 1161 DCHECK(new_entry_);
1163 cache_pending_ = true; 1162 cache_pending_ = true;
1164 next_state_ = STATE_ADD_TO_ENTRY_COMPLETE; 1163 next_state_ = STATE_ADD_TO_ENTRY_COMPLETE;
1165 net_log_.BeginEvent(NetLog::TYPE_HTTP_CACHE_ADD_TO_ENTRY); 1164 net_log_.BeginEvent(NetLog::TYPE_HTTP_CACHE_ADD_TO_ENTRY);
1166 DCHECK(entry_lock_waiting_since_.is_null()); 1165 DCHECK(entry_lock_waiting_since_.is_null());
1167 entry_lock_waiting_since_ = TimeTicks::Now(); 1166 entry_lock_waiting_since_ = TimeTicks::Now();
1168 int rv = cache_->AddTransactionToEntry(new_entry_, this); 1167 int rv = cache_->AddTransactionToEntry(new_entry_, this);
1169 if (rv == ERR_IO_PENDING) { 1168 if (rv == ERR_IO_PENDING) {
1170 if (bypass_lock_for_test_) { 1169 // Quickly timeout and bypass the cache if the blocked by the
1171 OnAddToEntryTimeout(entry_lock_waiting_since_); 1170 // reader/writer lock. Metrics indicate that only 0.89% of requests are
1172 } else { 1171 // blocked on the cache lock for longer than 25ms.
1173 int timeout_milliseconds = 20 * 1000; 1172 //
1174 if (partial_ && new_entry_->writer && 1173 // Bypassing the cache is not ideal, as we are now ignoring the cache
1175 new_entry_->writer->range_requested_) { 1174 // entirely for requests which hit a resource in
1176 // Quickly timeout and bypass the cache if we're a range request and 1175 // parallel. https://crbug.com/472740 tracks a better design for the
1177 // we're blocked by the reader/writer lock. Doing so eliminates a long 1176 // system.
1178 // running issue, http://crbug.com/31014, where two of the same media 1177 //
1179 // resources could not be played back simultaneously due to one locking 1178 // In the meantime, having a cache lock causes numerous priority inversion
1180 // the cache entry until the entire video was downloaded. 1179 // problems and deadlocks, so it is better to ensure the system can make
1181 // 1180 // forward progress. (See https://crbug.com/31014,
1182 // Bypassing the cache is not ideal, as we are now ignoring the cache 1181 // https://crbug.com/458620, https://crbug.com/535793, and
1183 // entirely for all range requests to a resource beyond the first. This 1182 // https://crbug.com/6697.)
1184 // is however a much more succinct solution than the alternatives, which 1183 //
1185 // would require somewhat significant changes to the http caching logic. 1184 // Allow some timeout slack for the entry addition to complete in case the
1186 // 1185 // writer lock is imminently released; we want to avoid skipping the cache
1187 // Allow some timeout slack for the entry addition to complete in case 1186 // if at all possible. See http://crbug.com/408765
1188 // the writer lock is imminently released; we want to avoid skipping 1187 base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
1189 // the cache if at all possible. See http://crbug.com/408765 1188 FROM_HERE,
1190 timeout_milliseconds = 25; 1189 base::Bind(&HttpCache::Transaction::OnAddToEntryTimeout,
1191 } 1190 weak_factory_.GetWeakPtr(), entry_lock_waiting_since_),
1192 base::ThreadTaskRunnerHandle::Get()->PostDelayedTask( 1191 TimeDelta::FromMilliseconds(25));
1193 FROM_HERE,
1194 base::Bind(&HttpCache::Transaction::OnAddToEntryTimeout,
1195 weak_factory_.GetWeakPtr(), entry_lock_waiting_since_),
1196 TimeDelta::FromMilliseconds(timeout_milliseconds));
1197 }
1198 } 1192 }
1199 return rv; 1193 return rv;
1200 } 1194 }
1201 1195
1202 int HttpCache::Transaction::DoAddToEntryComplete(int result) { 1196 int HttpCache::Transaction::DoAddToEntryComplete(int result) {
1203 net_log_.EndEventWithNetErrorCode(NetLog::TYPE_HTTP_CACHE_ADD_TO_ENTRY, 1197 net_log_.EndEventWithNetErrorCode(NetLog::TYPE_HTTP_CACHE_ADD_TO_ENTRY,
1204 result); 1198 result);
1205 const TimeDelta entry_lock_wait = 1199 const TimeDelta entry_lock_wait =
1206 TimeTicks::Now() - entry_lock_waiting_since_; 1200 TimeTicks::Now() - entry_lock_waiting_since_;
1207 UMA_HISTOGRAM_TIMES("HttpCache.EntryLockWait", entry_lock_wait); 1201 UMA_HISTOGRAM_TIMES("HttpCache.EntryLockWait", entry_lock_wait);
(...skipping 1729 matching lines...) Expand 10 before | Expand all | Expand 10 after
2937 default: 2931 default:
2938 NOTREACHED(); 2932 NOTREACHED();
2939 } 2933 }
2940 } 2934 }
2941 2935
2942 void HttpCache::Transaction::OnIOComplete(int result) { 2936 void HttpCache::Transaction::OnIOComplete(int result) {
2943 DoLoop(result); 2937 DoLoop(result);
2944 } 2938 }
2945 2939
2946 } // namespace net 2940 } // namespace net
OLDNEW
« no previous file with comments | « net/http/http_cache_transaction.h ('k') | net/http/http_cache_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698