Index: net/disk_cache/simple/simple_synchronous_entry.cc |
diff --git a/net/disk_cache/simple/simple_synchronous_entry.cc b/net/disk_cache/simple/simple_synchronous_entry.cc |
index dc27812c71382e95de0430d79a699a080174899d..4457312246fd19d7af79359d136d7d85ab5dabd7 100644 |
--- a/net/disk_cache/simple/simple_synchronous_entry.cc |
+++ b/net/disk_cache/simple/simple_synchronous_entry.cc |
@@ -13,6 +13,7 @@ |
#include "base/files/file_util.h" |
#include "base/hash.h" |
#include "base/location.h" |
+#include "base/memory/ptr_util.h" |
#include "base/metrics/histogram_macros.h" |
#include "base/numerics/safe_conversions.h" |
#include "base/sha1.h" |
@@ -22,6 +23,7 @@ |
#include "net/base/io_buffer.h" |
#include "net/base/net_errors.h" |
#include "net/disk_cache/simple/simple_backend_version.h" |
+#include "net/disk_cache/simple/simple_experiment.h" |
#include "net/disk_cache/simple/simple_histogram_macros.h" |
#include "net/disk_cache/simple/simple_util.h" |
#include "third_party/zlib/zlib.h" |
@@ -118,6 +120,10 @@ void RecordKeySHA256Result(net::CacheType cache_type, KeySHA256Result result) { |
static_cast<int>(KeySHA256Result::MAX)); |
} |
+void RecordSyncOpenPrefetchStatus(net::CacheType cache_type, bool result) { |
pasko
2017/07/18 13:46:31
my favorite: nit on naming, because it is always p
Maks Orlovich
2017/07/21 18:37:33
Done'ish
|
+ SIMPLE_CACHE_UMA(BOOLEAN, "SyncOpenDidPrefetch", cache_type, result); |
+} |
+ |
bool CanOmitEmptyFile(int file_index) { |
DCHECK_GE(file_index, 0); |
DCHECK_LT(file_index, disk_cache::kSimpleEntryFileCount); |
@@ -269,7 +275,8 @@ void SimpleSynchronousEntry::OpenEntry( |
new SimpleSynchronousEntry(cache_type, path, key, entry_hash, had_index); |
out_results->result = sync_entry->InitializeForOpen( |
&out_results->entry_stat, &out_results->stream_0_data, |
- &out_results->stream_0_crc32); |
+ &out_results->stream_0_crc32, &out_results->stream_1_data, |
+ &out_results->stream_1_crc32); |
pasko
2017/07/18 13:46:31
too many output arguments some set under non-trivi
Maks Orlovich
2017/07/28 17:27:35
Ended up grouping into array of structs, also simp
|
if (out_results->result != net::OK) { |
sync_entry->Doom(); |
delete sync_entry; |
@@ -692,19 +699,17 @@ void SimpleSynchronousEntry::GetAvailableRange( |
int SimpleSynchronousEntry::CheckEOFRecord(int index, |
const SimpleEntryStat& entry_stat, |
- uint32_t expected_crc32) const { |
+ uint32_t expected_crc32) { |
DCHECK(initialized_); |
- uint32_t crc32; |
- bool has_crc32; |
- bool has_key_sha256; |
- int32_t stream_size; |
- int rv = GetEOFRecordData(index, entry_stat, &has_crc32, &has_key_sha256, |
- &crc32, &stream_size); |
+ SimpleFileEOF eof_record; |
+ int rv = GetEOFRecordData(nullptr, 0, index, entry_stat, &eof_record); |
+ |
if (rv != net::OK) { |
Doom(); |
return rv; |
} |
- if (has_crc32 && crc32 != expected_crc32) { |
+ if ((eof_record.flags & SimpleFileEOF::FLAG_HAS_CRC32) && |
+ eof_record.data_crc32 != expected_crc32) { |
DVLOG(1) << "EOF record had bad crc."; |
RecordCheckEOFResult(cache_type_, CHECK_EOF_RESULT_CRC_MISMATCH); |
Doom(); |
@@ -714,6 +719,41 @@ int SimpleSynchronousEntry::CheckEOFRecord(int index, |
return net::OK; |
} |
+int SimpleSynchronousEntry::PreReadStreamPayload( |
pasko
2017/07/18 13:46:31
Why Pre? It just reads, no?
morlovich
2017/07/18 14:32:31
Because it's only used in the prefetch path?
pasko
2017/07/19 16:28:26
ah, ok
Maks Orlovich
2017/07/21 18:37:34
Acknowledged.
|
+ char* prefetch_buf, |
+ int file_size, |
+ int stream_index, |
+ int extra_size, |
+ const SimpleEntryStat& entry_stat, |
+ const SimpleFileEOF& eof_record, |
+ scoped_refptr<net::GrowableIOBuffer>* stream_data, |
+ uint32_t* out_crc32) { |
+ int stream_size = entry_stat.data_size(stream_index); |
+ int read_size = stream_size + extra_size; |
+ *stream_data = new net::GrowableIOBuffer(); |
+ (*stream_data)->SetCapacity(read_size); |
+ int file_offset = entry_stat.GetOffsetInFile(key_.size(), 0, stream_index); |
+ if (!ReadFromFileOrPrefetched(prefetch_buf, file_size, file_offset, read_size, |
+ (*stream_data)->data())) |
+ return net::ERR_FAILED; |
+ |
+ // Check the CRC32. |
+ uint32_t expected_crc32 = |
+ stream_size == 0 |
+ ? crc32(0, Z_NULL, 0) |
+ : crc32(crc32(0, Z_NULL, 0), |
+ reinterpret_cast<const Bytef*>((*stream_data)->data()), |
pasko
2017/07/19 16:28:26
In case you have similar allergies to reinterpret_
Maks Orlovich
2017/07/25 16:06:29
Split out into https://chromium-review.googlesourc
|
+ stream_size); |
+ if ((eof_record.flags & SimpleFileEOF::FLAG_HAS_CRC32) && |
+ eof_record.data_crc32 != expected_crc32) { |
+ DVLOG(1) << "EOF record had bad crc."; |
+ RecordCheckEOFResult(cache_type_, CHECK_EOF_RESULT_CRC_MISMATCH); |
pasko
2017/07/19 16:28:26
just realized that this might slightly increase th
Maks Orlovich
2017/07/21 18:37:34
Acknowledged.
|
+ return net::ERR_CACHE_CHECKSUM_MISMATCH; |
+ } |
+ *out_crc32 = expected_crc32; |
+ return net::OK; |
+} |
+ |
void SimpleSynchronousEntry::Close( |
const SimpleEntryStat& entry_stat, |
std::unique_ptr<std::vector<CRCRecord>> crc32s_to_write, |
@@ -1096,7 +1136,9 @@ bool SimpleSynchronousEntry::CheckHeaderAndKey(int file_index) { |
int SimpleSynchronousEntry::InitializeForOpen( |
SimpleEntryStat* out_entry_stat, |
scoped_refptr<net::GrowableIOBuffer>* stream_0_data, |
- uint32_t* out_stream_0_crc32) { |
+ uint32_t* out_stream_0_crc32, |
+ scoped_refptr<net::GrowableIOBuffer>* stream_1_data, |
+ uint32_t* out_stream_1_crc32) { |
DCHECK(!initialized_); |
if (!OpenFiles(out_entry_stat)) { |
DLOG(WARNING) << "Could not open platform files for entry."; |
@@ -1124,9 +1166,9 @@ int SimpleSynchronousEntry::InitializeForOpen( |
if (i == 0) { |
// File size for stream 0 has been stored temporarily in data_size[1]. |
- int ret_value_stream_0 = |
- ReadAndValidateStream0(out_entry_stat->data_size(1), out_entry_stat, |
- stream_0_data, out_stream_0_crc32); |
+ int ret_value_stream_0 = ReadAndValidateStream0( |
+ out_entry_stat->data_size(1), out_entry_stat, stream_0_data, |
+ out_stream_0_crc32, stream_1_data, out_stream_1_crc32); |
if (ret_value_stream_0 != net::OK) |
return ret_value_stream_0; |
} else { |
@@ -1221,7 +1263,23 @@ int SimpleSynchronousEntry::ReadAndValidateStream0( |
int file_size, |
SimpleEntryStat* out_entry_stat, |
scoped_refptr<net::GrowableIOBuffer>* stream_0_data, |
- uint32_t* out_stream_0_crc32) { |
+ uint32_t* out_stream_0_crc32, |
+ scoped_refptr<net::GrowableIOBuffer>* stream_1_data, |
+ uint32_t* out_stream_1_crc32) { |
+ // If the file is sufficiently small, we will prefetch everything -- |
+ // in which case |prefetch_buf| will be non-null, and we should look at it |
+ // rather than call ::Read for the bits. |
+ std::unique_ptr<char[]> prefetch_buf; |
+ |
+ if (file_size <= GetSimpleCachePrefetchSize()) { |
+ RecordSyncOpenPrefetchStatus(cache_type_, true); |
+ prefetch_buf = base::MakeUnique<char[]>(file_size); |
+ if (files_[0].Read(0, prefetch_buf.get(), file_size) != file_size) |
+ return net::ERR_FAILED; |
+ } else { |
+ RecordSyncOpenPrefetchStatus(cache_type_, false); |
pasko
2017/07/18 13:46:31
nit: it is usually easier to read if the short bra
Maks Orlovich
2017/07/21 18:37:33
[Citation needed], but done.
pasko
2017/07/24 12:22:45
unfortunately, could not find a good one, and now
|
+ } |
+ |
// Pretend this file has a null stream zero, and contains the optional key |
// SHA256. This is good enough to read the EOF record on the file, which gives |
// the actual size of stream 0. |
@@ -1230,49 +1288,56 @@ int SimpleSynchronousEntry::ReadAndValidateStream0( |
out_entry_stat->set_data_size( |
1, temp_data_size - sizeof(net::SHA256HashValue) - sizeof(SimpleFileEOF)); |
- bool has_crc32; |
- bool has_key_sha256; |
- uint32_t read_crc32; |
- int32_t stream_0_size; |
- int ret_value_crc32 = |
- GetEOFRecordData(0, *out_entry_stat, &has_crc32, &has_key_sha256, |
- &read_crc32, &stream_0_size); |
- if (ret_value_crc32 != net::OK) |
- return ret_value_crc32; |
+ SimpleFileEOF stream_0_eof; |
+ int rv = GetEOFRecordData(prefetch_buf.get(), file_size, 0, *out_entry_stat, |
+ &stream_0_eof); |
+ if (rv != net::OK) |
+ return rv; |
+ |
+ bool has_key_sha256 = |
+ (stream_0_eof.flags & SimpleFileEOF::FLAG_HAS_KEY_SHA256) == |
+ SimpleFileEOF::FLAG_HAS_KEY_SHA256; |
+ int32_t stream_0_size = stream_0_eof.stream_size; |
// Calculate and set the real values for the two streams. |
int32_t total_size = out_entry_stat->data_size(1); |
if (!has_key_sha256) |
total_size += sizeof(net::SHA256HashValue); |
+ |
+ // Sanity-check: don't want to be allocating a gigantic buffer > file size |
+ // just because of a header field. |
if (stream_0_size > total_size) |
return net::ERR_FAILED; |
out_entry_stat->set_data_size(0, stream_0_size); |
out_entry_stat->set_data_size(1, total_size - stream_0_size); |
- // Put stream 0 data in memory. |
- *stream_0_data = new net::GrowableIOBuffer(); |
- (*stream_0_data)->SetCapacity(stream_0_size + sizeof(net::SHA256HashValue)); |
- int file_offset = out_entry_stat->GetOffsetInFile(key_.size(), 0, 0); |
- int read_size = stream_0_size; |
+ // Put stream 0 data in memory --- plus maybe the sha256(key) footer. |
+ int extra_stream_0_read = 0; |
if (has_key_sha256) |
- read_size += sizeof(net::SHA256HashValue); |
- if (files_[0].Read(file_offset, (*stream_0_data)->data(), read_size) != |
- read_size) |
- return net::ERR_FAILED; |
+ extra_stream_0_read += sizeof(net::SHA256HashValue); |
+ rv = PreReadStreamPayload(prefetch_buf.get(), file_size, 0, |
+ extra_stream_0_read, *out_entry_stat, stream_0_eof, |
+ stream_0_data, out_stream_0_crc32); |
+ if (rv != net::OK) |
+ return rv; |
- // Check the CRC32. |
- uint32_t expected_crc32 = |
- stream_0_size == 0 |
- ? crc32(0, Z_NULL, 0) |
- : crc32(crc32(0, Z_NULL, 0), |
- reinterpret_cast<const Bytef*>((*stream_0_data)->data()), |
- stream_0_size); |
- if (has_crc32 && read_crc32 != expected_crc32) { |
- DVLOG(1) << "EOF record had bad crc."; |
- RecordCheckEOFResult(cache_type_, CHECK_EOF_RESULT_CRC_MISMATCH); |
- return net::ERR_FAILED; |
+ // If prefetch buffer is available, and we have sha256(key) (so we don't need |
+ // to look at the header), extract out stream 1 info as well. |
+ if (prefetch_buf && has_key_sha256) { |
+ SimpleFileEOF stream_1_eof; |
+ rv = GetEOFRecordData(prefetch_buf.get(), file_size, 1, *out_entry_stat, |
+ &stream_1_eof); |
+ if (rv != net::OK) |
+ return rv; |
+ |
+ rv = PreReadStreamPayload(prefetch_buf.get(), file_size, 1, 0, |
+ *out_entry_stat, stream_1_eof, stream_1_data, |
+ out_stream_1_crc32); |
+ if (rv != net::OK) |
+ return rv; |
+ } else { |
+ *stream_1_data = nullptr; |
} |
- *out_stream_0_crc32 = expected_crc32; |
// If present, check the key SHA256. |
if (has_key_sha256) { |
@@ -1300,40 +1365,66 @@ int SimpleSynchronousEntry::ReadAndValidateStream0( |
return net::OK; |
} |
-int SimpleSynchronousEntry::GetEOFRecordData(int index, |
+bool SimpleSynchronousEntry::ReadFromFileOrPrefetched(char* prefetch_buf, |
+ int file_size, |
+ int offset, |
+ int size, |
+ char* dest) { |
+ if (!prefetch_buf) { |
+ return files_[0].Read(offset, dest, size) == size; |
+ } else { |
+ if (offset < 0 || size < 0) |
+ return false; |
+ base::CheckedNumeric<int> end = |
+ base::CheckedNumeric<int>(offset) + size - 1; |
+ if (!end.IsValid()) |
+ return false; |
+ if (offset >= file_size || end.ValueOrDie() >= file_size) |
pasko
2017/07/18 13:46:31
feel free to insert a DCHECK for the overflow, but
morlovich
2017/07/18 14:32:31
See the conditional in line 1380, this can't actua
pasko
2017/07/19 16:28:26
Ah, missed that. AssignIfValid() looks lengthy, I'
Maks Orlovich
2017/07/21 18:37:33
Ended up with AssignIfValid anyway, but I think it
|
+ return false; |
+ |
+ memcpy(dest, prefetch_buf + offset, size); |
+ return true; |
+ } |
+} |
+ |
+int SimpleSynchronousEntry::GetEOFRecordData(char* prefetch_buf, |
+ int file_size, |
+ int stream_index, |
const SimpleEntryStat& entry_stat, |
- bool* out_has_crc32, |
- bool* out_has_key_sha256, |
- uint32_t* out_crc32, |
- int32_t* out_data_size) const { |
pasko
2017/07/18 13:46:31
Following how the state migrates from disk to the
morlovich
2017/07/18 14:32:31
So my idea was that ReadFromFileOrPrefetched would
pasko
2017/07/19 16:28:26
Yeah, I understand your way to put ReadFromFileOrP
|
- SimpleFileEOF eof_record; |
- int file_offset = entry_stat.GetEOFOffsetInFile(key_.size(), index); |
- int file_index = GetFileIndexFromStreamIndex(index); |
- File* file = const_cast<File*>(&files_[file_index]); |
- if (file->Read(file_offset, reinterpret_cast<char*>(&eof_record), |
- sizeof(eof_record)) != |
- sizeof(eof_record)) { |
+ SimpleFileEOF* eof_record) { |
+ int file_offset = entry_stat.GetEOFOffsetInFile(key_.size(), stream_index); |
+ int file_index = GetFileIndexFromStreamIndex(stream_index); |
+ |
+ // Should only have prefetch_buf when reading things in file 0. |
+ DCHECK(file_index == 0 || !prefetch_buf); |
+ bool ok; |
+ if (prefetch_buf) |
+ ok = ReadFromFileOrPrefetched(prefetch_buf, file_size, file_offset, |
+ sizeof(SimpleFileEOF), |
+ reinterpret_cast<char*>(eof_record)); |
+ else |
+ // Need to support files_[1], so can't use ReadFromFileOrPrefetched |
+ // in this case. |
+ ok = files_[file_index].Read( |
+ file_offset, reinterpret_cast<char*>(eof_record), |
+ sizeof(SimpleFileEOF)) == sizeof(SimpleFileEOF); |
+ |
+ if (!ok) { |
RecordCheckEOFResult(cache_type_, CHECK_EOF_RESULT_READ_FAILURE); |
return net::ERR_CACHE_CHECKSUM_READ_FAILURE; |
} |
- if (eof_record.final_magic_number != kSimpleFinalMagicNumber) { |
+ if (eof_record->final_magic_number != kSimpleFinalMagicNumber) { |
RecordCheckEOFResult(cache_type_, CHECK_EOF_RESULT_MAGIC_NUMBER_MISMATCH); |
DVLOG(1) << "EOF record had bad magic number."; |
return net::ERR_CACHE_CHECKSUM_READ_FAILURE; |
} |
- if (!base::IsValueInRangeForNumericType<int32_t>(eof_record.stream_size)) |
+ if (!base::IsValueInRangeForNumericType<int32_t>(eof_record->stream_size)) |
return net::ERR_FAILED; |
- |
- *out_has_crc32 = (eof_record.flags & SimpleFileEOF::FLAG_HAS_CRC32) == |
- SimpleFileEOF::FLAG_HAS_CRC32; |
- *out_has_key_sha256 = |
- (eof_record.flags & SimpleFileEOF::FLAG_HAS_KEY_SHA256) == |
- SimpleFileEOF::FLAG_HAS_KEY_SHA256; |
- *out_crc32 = eof_record.data_crc32; |
- *out_data_size = eof_record.stream_size; |
- SIMPLE_CACHE_UMA(BOOLEAN, "SyncCheckEOFHasCrc", cache_type_, *out_has_crc32); |
+ SIMPLE_CACHE_UMA(BOOLEAN, "SyncCheckEOFHasCrc", cache_type_, |
+ (eof_record->flags & SimpleFileEOF::FLAG_HAS_CRC32) == |
+ SimpleFileEOF::FLAG_HAS_CRC32); |
return net::OK; |
} |