SimpleCache: merge the first and second stream in one file

net/disk_cache/simple/simple_synchronous_entry.cc

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 a1ff1ac6adaab1664d6a61656ed1a1af93a0d308..67af92f8b92ecae91fce2980e2e5d03f9c3adc73 100644
--- a/net/disk_cache/simple/simple_synchronous_entry.cc
+++ b/net/disk_cache/simple/simple_synchronous_entry.cc
@@ -146,7 +146,9 @@ using simple_util::GetEntryHashKey;
 using simple_util::GetFilenameFromEntryHashAndIndex;
 using simple_util::GetDataSizeFromKeyAndFileSize;
 using simple_util::GetFileSizeFromKeyAndDataSize;
-using simple_util::GetFileOffsetFromKeyAndDataOffset;
+using simple_util::GetFileIndexFromStreamIndex;
+using simple_util::GetLastEOFRecordOffset;
+using simple_util::GetFileOffsetFromDataOffset;
 
 SimpleEntryStat::SimpleEntryStat() {}
 
@@ -158,6 +160,27 @@ SimpleEntryStat::SimpleEntryStat(base::Time last_used_p,
   memcpy(data_size, data_size_p, sizeof(data_size));
 }
 
+int SimpleEntryStat::GetOffsetInFile(const std::string& key,
+                                     int offset,
+                                     int stream_index) const {
+  return GetFileOffsetFromDataOffset(key, offset, stream_index, data_size[1]);
+}
+
+int SimpleEntryStat::GetEOFOffsetInFile(const std::string& key,
+                                        int stream_index) const {
+  return GetFileOffsetFromDataOffset(
+      key, data_size[stream_index], stream_index, data_size[1]);
+}
+
+int SimpleEntryStat::GetLastEOFOffsetInFile(const std::string& key,
+                                            int stream_index) const {
+  return GetOffsetInFile(
+      key,
+      GetLastEOFRecordOffset(GetFileIndexFromStreamIndex(stream_index),
+                             data_size),
+      stream_index);
+}
+
 SimpleEntryCreationResults::SimpleEntryCreationResults(
     SimpleEntryStat entry_stat)
     : sync_entry(NULL),
@@ -206,11 +229,12 @@ void SimpleSynchronousEntry::OpenEntry(
   SimpleSynchronousEntry* sync_entry =
       new SimpleSynchronousEntry(cache_type, path, "", entry_hash);
   out_results->result = sync_entry->InitializeForOpen(
-      had_index, &out_results->entry_stat);
+      had_index, &out_results->entry_stat, &out_results->stream_0_data);
   if (out_results->result != net::OK) {
     sync_entry->Doom();
     delete sync_entry;
     out_results->sync_entry = NULL;
+    out_results->stream_0_data = NULL;
     return;
   }
   out_results->sync_entry = sync_entry;
@@ -279,17 +303,16 @@ int SimpleSynchronousEntry::DoomEntrySet(
 void SimpleSynchronousEntry::ReadData(const EntryOperationData& in_entry_op,
                                       net::IOBuffer* out_buf,
                                       uint32* out_crc32,
-                                      base::Time* out_last_used,
+                                      SimpleEntryStat* out_entry_stat,
                                       int* out_result) const {
   DCHECK(initialized_);
-  int64 file_offset =
-      GetFileOffsetFromKeyAndDataOffset(key_, in_entry_op.offset);
-  int bytes_read = ReadPlatformFile(files_[in_entry_op.index],
-                                    file_offset,
-                                    out_buf->data(),
-                                    in_entry_op.buf_len);
+  const int64 file_offset = out_entry_stat->GetOffsetInFile(
+      key_, in_entry_op.offset, in_entry_op.index);
+  int file_index = GetFileIndexFromStreamIndex(in_entry_op.index);
+  int bytes_read = ReadPlatformFile(
+      files_[file_index], file_offset, out_buf->data(), in_entry_op.buf_len);
   if (bytes_read > 0) {
-    *out_last_used = Time::Now();
+    out_entry_stat->last_used = Time::Now();
     *out_crc32 = crc32(crc32(0L, Z_NULL, 0),
                        reinterpret_cast<const Bytef*>(out_buf->data()),
                        bytes_read);
@@ -308,26 +331,28 @@ void SimpleSynchronousEntry::WriteData(const EntryOperationData& in_entry_op,
                                        int* out_result) const {
   DCHECK(initialized_);
   int index = in_entry_op.index;
+  int file_index = GetFileIndexFromStreamIndex(index);
   int offset = in_entry_op.offset;
   int buf_len = in_entry_op.buf_len;
   int truncate = in_entry_op.truncate;
-
+  const int64 file_offset = out_entry_stat->GetOffsetInFile(
+      key_, in_entry_op.offset, in_entry_op.index);
   bool extending_by_write = offset + buf_len > out_entry_stat->data_size[index];
   if (extending_by_write) {
-    // We are extending the file, and need to insure the EOF record is zeroed.
-    const int64 file_eof_offset = GetFileOffsetFromKeyAndDataOffset(
-        key_, out_entry_stat->data_size[index]);
-    if (!TruncatePlatformFile(files_[index], file_eof_offset)) {
+    // The EOF record and the eventual stream afterward need to be zeroed out.
+    const int64 file_eof_offset =
+        out_entry_stat->GetEOFOffsetInFile(key_, index);
+    if (!TruncatePlatformFile(files_[file_index], file_eof_offset)) {
       RecordWriteResult(cache_type_, WRITE_RESULT_PRETRUNCATE_FAILURE);
       Doom();
       *out_result = net::ERR_CACHE_WRITE_FAILURE;
       return;
     }
   }
-  const int64 file_offset = GetFileOffsetFromKeyAndDataOffset(key_, offset);
   if (buf_len > 0) {
     if (WritePlatformFile(
-            files_[index], file_offset, in_buf->data(), buf_len) != buf_len) {
+            files_[file_index], file_offset, in_buf->data(), buf_len) !=
+        buf_len) {
       RecordWriteResult(cache_type_, WRITE_RESULT_WRITE_FAILURE);
       Doom();
       *out_result = net::ERR_CACHE_WRITE_FAILURE;
@@ -338,13 +363,14 @@ void SimpleSynchronousEntry::WriteData(const EntryOperationData& in_entry_op,
     out_entry_stat->data_size[index] =
         std::max(out_entry_stat->data_size[index], offset + buf_len);
   } else {
-    if (!TruncatePlatformFile(files_[index], file_offset + buf_len)) {
+    out_entry_stat->data_size[index] = offset + buf_len;
+    int file_eof_offset = out_entry_stat->GetLastEOFOffsetInFile(key_, index);
+    if (!TruncatePlatformFile(files_[file_index], file_eof_offset)) {
       RecordWriteResult(cache_type_, WRITE_RESULT_TRUNCATE_FAILURE);
       Doom();
       *out_result = net::ERR_CACHE_WRITE_FAILURE;
       return;
     }
-    out_entry_stat->data_size[index] = offset + buf_len;
   }
 
   RecordWriteResult(cache_type_, WRITE_RESULT_SUCCESS);
@@ -353,61 +379,63 @@ void SimpleSynchronousEntry::WriteData(const EntryOperationData& in_entry_op,
 }
 
 void SimpleSynchronousEntry::CheckEOFRecord(int index,
-                                            int32 data_size,
+                                            const SimpleEntryStat& entry_stat,
                                             uint32 expected_crc32,
                                             int* out_result) const {
   DCHECK(initialized_);
-
-  SimpleFileEOF eof_record;
-  int64 file_offset = GetFileOffsetFromKeyAndDataOffset(key_, data_size);
-  if (ReadPlatformFile(files_[index],
-                       file_offset,
-                       reinterpret_cast<char*>(&eof_record),
-                       sizeof(eof_record)) != sizeof(eof_record)) {
-    RecordCheckEOFResult(cache_type_, CHECK_EOF_RESULT_READ_FAILURE);
+  uint32 crc32;
+  bool has_crc32;
+  int stream_size;
+  *out_result =
+      GetEOFRecordData(index, entry_stat, &has_crc32, &crc32, &stream_size);
+  if (*out_result != net::OK) {
     Doom();
-    *out_result = net::ERR_CACHE_CHECKSUM_READ_FAILURE;
     return;
   }
-
-  if (eof_record.final_magic_number != kSimpleFinalMagicNumber) {
-    RecordCheckEOFResult(cache_type_, CHECK_EOF_RESULT_MAGIC_NUMBER_MISMATCH);
-    DLOG(INFO) << "eof record had bad magic number.";
-    Doom();
-    *out_result = net::ERR_CACHE_CHECKSUM_READ_FAILURE;
-    return;
-  }
-
-  const bool has_crc = (eof_record.flags & SimpleFileEOF::FLAG_HAS_CRC32) ==
-                       SimpleFileEOF::FLAG_HAS_CRC32;
-  SIMPLE_CACHE_UMA(BOOLEAN, "SyncCheckEOFHasCrc", cache_type_, has_crc);
-  if (has_crc && eof_record.data_crc32 != expected_crc32) {
+  if (has_crc32 && crc32 != expected_crc32) {
+    DLOG(INFO) << "Eof record had bad crc.";
+    *out_result = net::ERR_CACHE_CHECKSUM_MISMATCH;
     RecordCheckEOFResult(cache_type_, CHECK_EOF_RESULT_CRC_MISMATCH);
-    DLOG(INFO) << "eof record had bad crc.";
     Doom();
-    *out_result = net::ERR_CACHE_CHECKSUM_MISMATCH;
     return;
   }
-
   RecordCheckEOFResult(cache_type_, CHECK_EOF_RESULT_SUCCESS);
-  *out_result = net::OK;
 }
 
 void SimpleSynchronousEntry::Close(
     const SimpleEntryStat& entry_stat,
-    scoped_ptr<std::vector<CRCRecord> > crc32s_to_write) {
+    scoped_ptr<std::vector<CRCRecord> > crc32s_to_write,
+    net::GrowableIOBuffer* stream_0_data) {
+  if (!stream_0_data) {

[gavinp, 2013/09/17 13:27:11]

What's happening in this case?

[clamy, 2013/09/17 13:59:42]

If we were supposed to have data and we don't, it's an error. We are not
supposed to though, so I could replace it with a DCHECK.

[gavinp, 2013/09/17 14:25:33]

In practice we always have data in stream 0, but isn't it legal to have nothing?

[clamy, 2013/09/17 14:29:48]

If we get non 0 size in data_size[0], then we expect to have something in stream
0, but not actual data is provided. Wouldn't that be non legal ?

[gavinp, 2013/09/17 15:07:43]

Aha, yup. So you were right the first time, this should be a DCHECK. We
shouldn't check for impossible conditions.

[clamy, 2013/09/18 16:17:14]

With the other modifications that went in, we should always have a stream_0_data
buffer, so I added a DCHECK at the beginning of the function.

+    if (entry_stat.data_size[0])
+      Doom();
+  } else {
+    // Write stream 0 data.
+    int stream_0_offset = entry_stat.GetOffsetInFile(key_, 0, 0);
+    if (WritePlatformFile(files_[0],
+                          stream_0_offset,
+                          stream_0_data->data(),
+                          entry_stat.data_size[0]) != entry_stat.data_size[0]) {
+      RecordCloseResult(cache_type_, CLOSE_RESULT_WRITE_FAILURE);
+      DLOG(INFO) << "Could not write stream 0 data.";
+      Doom();
+    }
+  }
+
   for (std::vector<CRCRecord>::const_iterator it = crc32s_to_write->begin();
        it != crc32s_to_write->end(); ++it) {
     SimpleFileEOF eof_record;
+    int index = it->index;
+    eof_record.stream_size = entry_stat.data_size[index];
     eof_record.final_magic_number = kSimpleFinalMagicNumber;
     eof_record.flags = 0;
     if (it->has_crc32)
       eof_record.flags |= SimpleFileEOF::FLAG_HAS_CRC32;
     eof_record.data_crc32 = it->data_crc32;
-    int64 file_offset = GetFileOffsetFromKeyAndDataOffset(
-        key_, entry_stat.data_size[it->index]);
-    if (WritePlatformFile(files_[it->index],
-                          file_offset,
+    int file_index = GetFileIndexFromStreamIndex(index);
+    int eof_offset = entry_stat.GetEOFOffsetInFile(key_, index);
+    if (WritePlatformFile(files_[file_index],
+                          eof_offset,
                           reinterpret_cast<const char*>(&eof_record),
                           sizeof(eof_record)) != sizeof(eof_record)) {
       RecordCloseResult(cache_type_, CLOSE_RESULT_WRITE_FAILURE);
@@ -415,7 +443,12 @@ void SimpleSynchronousEntry::Close(
       Doom();
       break;
     }
-    const int64 file_size = file_offset + sizeof(eof_record);
+  }
+  for (int i = 0; i < kSimpleEntryFileCount; ++i) {
+    bool did_close_file = ClosePlatformFile(files_[i]);
+    CHECK(did_close_file);
+    const int64 file_size = GetFileSizeFromKeyAndDataSize(
+        key_, GetLastEOFRecordOffset(i, entry_stat.data_size));
     SIMPLE_CACHE_UMA(CUSTOM_COUNTS,
                      "LastClusterSize", cache_type_,
                      file_size % 4096, 0, 4097, 50);
@@ -424,11 +457,6 @@ void SimpleSynchronousEntry::Close(
                      "LastClusterLossPercent", cache_type_,
                      cluster_loss * 100 / (cluster_loss + file_size));
   }
-
-  for (int i = 0; i < kSimpleEntryFileCount; ++i) {
-    bool did_close_file = ClosePlatformFile(files_[i]);
-    CHECK(did_close_file);
-  }
   RecordCloseResult(cache_type_, CLOSE_RESULT_SUCCESS);
   have_open_files_ = false;
   delete this;
@@ -517,7 +545,7 @@ bool SimpleSynchronousEntry::OpenOrCreateFiles(
   have_open_files_ = true;
   if (create) {
     out_entry_stat->last_modified = out_entry_stat->last_used = Time::Now();
-    for (int i = 0; i < kSimpleEntryFileCount; ++i)
+    for (int i = 0; i < kSimpleEntryStreamCount; ++i)
       out_entry_stat->data_size[i] = 0;
   } else {
     base::TimeDelta entry_age = base::Time::Now() - base::Time::UnixEpoch();
@@ -540,9 +568,9 @@ bool SimpleSynchronousEntry::OpenOrCreateFiles(
       if (stream_age < entry_age)
         entry_age = stream_age;
 
-      // Keep the file size in |data size_| briefly until the key is initialized
-      // properly.
-      out_entry_stat->data_size[i] = file_info.size;
+      // Keep the file size in |data size_| temporarily until the data sizes are
+      // initialized correctly.
+      out_entry_stat->data_size[i + 1] = file_info.size;
     }
     SIMPLE_CACHE_UMA(CUSTOM_COUNTS,
                      "SyncOpenEntryAge", cache_type_,
@@ -560,12 +588,13 @@ void SimpleSynchronousEntry::CloseFiles() {
   }
 }
 
-int SimpleSynchronousEntry::InitializeForOpen(bool had_index,
-                                              SimpleEntryStat* out_entry_stat) {
+int SimpleSynchronousEntry::InitializeForOpen(
+    bool had_index,
+    SimpleEntryStat* out_entry_stat,
+    scoped_refptr<net::GrowableIOBuffer>* stream_0_data) {
   DCHECK(!initialized_);
   if (!OpenOrCreateFiles(false, had_index, out_entry_stat))
     return net::ERR_FAILED;
-
   for (int i = 0; i < kSimpleEntryFileCount; ++i) {
     SimpleFileHeader header;
     int header_read_result =
@@ -602,12 +631,19 @@ int SimpleSynchronousEntry::InitializeForOpen(bool had_index,
     }
 
     key_ = std::string(key.get(), header.key_length);
-    out_entry_stat->data_size[i] =
-        GetDataSizeFromKeyAndFileSize(key_, out_entry_stat->data_size[i]);
-    if (out_entry_stat->data_size[i] < 0) {
-      // This entry can't possibly be valid, as it does not have enough space to
-      // store a valid SimpleFileEOF record.
-      return net::ERR_FAILED;
+    if (i == 0) {
+      // File size for stream 0 has been stored temporarily in data_size[1].
+      int total_data_size =
+          GetDataSizeFromKeyAndFileSize(key_, out_entry_stat->data_size[1]);
+      int ret_value_stream_0 = ReadAndValidateStream0(
+          total_data_size, out_entry_stat, stream_0_data);
+      if (ret_value_stream_0 != net::OK)
+        return ret_value_stream_0;
+    } else {
+      out_entry_stat->data_size[2] =
+          GetDataSizeFromKeyAndFileSize(key_, out_entry_stat->data_size[2]);
+      if (out_entry_stat->data_size[2] < 0)
+        return net::ERR_FAILED;
     }
 
     if (base::Hash(key.get(), header.key_length) != header.key_hash) {
@@ -638,16 +674,18 @@ int SimpleSynchronousEntry::InitializeForCreate(
     header.key_length = key_.size();
     header.key_hash = base::Hash(key_);
 
-    if (WritePlatformFile(files_[i], 0, reinterpret_cast<char*>(&header),
-                          sizeof(header)) != sizeof(header)) {
-      DLOG(WARNING) << "Could not write headers to new cache entry.";
+    if (WritePlatformFile(
+            files_[i], 0, reinterpret_cast<char*>(&header), sizeof(header)) !=
+        sizeof(header)) {
+      DLOG(WARNING) << "Could not write cache file header to cache entry.";
       RecordSyncCreateResult(
           cache_type_, CREATE_ENTRY_CANT_WRITE_HEADER, had_index);
       return net::ERR_FAILED;
     }
 
-    if (WritePlatformFile(files_[i], sizeof(header), key_.data(),
-                          key_.size()) != implicit_cast<int>(key_.size())) {
+    if (WritePlatformFile(
+            files_[i], sizeof(SimpleFileHeader), key_.data(), key_.size()) !=
+        implicit_cast<int>(key_.size())) {
       DLOG(WARNING) << "Could not write keys to new cache entry.";
       RecordSyncCreateResult(
           cache_type_, CREATE_ENTRY_CANT_WRITE_KEY, had_index);
@@ -659,6 +697,88 @@ int SimpleSynchronousEntry::InitializeForCreate(
   return net::OK;
 }
 
+int SimpleSynchronousEntry::ReadAndValidateStream0(
+    int total_data_size,
+    SimpleEntryStat* out_entry_stat,
+    scoped_refptr<net::GrowableIOBuffer>* stream_0_data) const {
+  // Temporarily assign all the data size to stream 1 in order to read the
+  // EOF record for stream 0, which contains the size of stream 0.
+  out_entry_stat->data_size[0] = 0;
+  out_entry_stat->data_size[1] = total_data_size - sizeof(SimpleFileEOF);
+
+  bool has_crc32;
+  uint32 read_crc32;
+  int stream_0_size;
+  int ret_value_crc32 = GetEOFRecordData(
+      0, *out_entry_stat, &has_crc32, &read_crc32, &stream_0_size);
+  if (ret_value_crc32 != net::OK)
+    return ret_value_crc32;
+
+  if (stream_0_size > out_entry_stat->data_size[1])
+    return net::ERR_FAILED;
+
+  // These are the real values of data size.
+  out_entry_stat->data_size[0] = stream_0_size;
+  out_entry_stat->data_size[1] -= stream_0_size;
+
+  // Put stream 0 data in memory.
+  *stream_0_data = NULL;
+  if (stream_0_size != 0) {
+    *stream_0_data = new net::GrowableIOBuffer();
+    (*stream_0_data)->SetCapacity(stream_0_size);
+    int file_offset = out_entry_stat->GetOffsetInFile(key_, 0, 0);
+    int bytes_read = ReadPlatformFile(
+        files_[0], file_offset, (*stream_0_data)->data(), stream_0_size);
+    if (bytes_read != stream_0_size)
+      return net::ERR_FAILED;
+  }
+
+  // Check the CRC32.
+  uint32 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) {
+    DLOG(INFO) << "Eof record had bad crc.";
+    RecordCheckEOFResult(cache_type_, CHECK_EOF_RESULT_CRC_MISMATCH);
+    return net::ERR_FAILED;
+  }
+  RecordCheckEOFResult(cache_type_, CHECK_EOF_RESULT_SUCCESS);
+  return net::OK;
+}
+
+int SimpleSynchronousEntry::GetEOFRecordData(int index,
+                                             const SimpleEntryStat& entry_stat,
+                                             bool* out_has_crc32,
+                                             uint32* out_crc32,
+                                             int* out_data_size) const {
+  SimpleFileEOF eof_record;
+  int file_offset = entry_stat.GetEOFOffsetInFile(key_, index);
+  int file_index = GetFileIndexFromStreamIndex(index);
+  if (ReadPlatformFile(files_[file_index],
+                       file_offset,
+                       reinterpret_cast<char*>(&eof_record),
+                       sizeof(eof_record)) != sizeof(eof_record)) {
+    RecordCheckEOFResult(cache_type_, CHECK_EOF_RESULT_READ_FAILURE);
+    return net::ERR_CACHE_CHECKSUM_READ_FAILURE;
+  }
+
+  if (eof_record.final_magic_number != kSimpleFinalMagicNumber) {
+    RecordCheckEOFResult(cache_type_, CHECK_EOF_RESULT_MAGIC_NUMBER_MISMATCH);
+    DLOG(INFO) << "Eof record had bad magic number.";
+    return net::ERR_CACHE_CHECKSUM_READ_FAILURE;
+  }
+
+  *out_has_crc32 = (eof_record.flags & SimpleFileEOF::FLAG_HAS_CRC32) ==
+                   SimpleFileEOF::FLAG_HAS_CRC32;
+  *out_crc32 = eof_record.data_crc32;
+  *out_data_size = eof_record.stream_size;
+  SIMPLE_CACHE_UMA(BOOLEAN, "SyncCheckEOFHasCrc", cache_type_, *out_has_crc32);
+  return net::OK;
+}
+
 void SimpleSynchronousEntry::Doom() const {
   // TODO(gavinp): Consider if we should guard against redundant Doom() calls.
   DeleteFilesForEntryHash(path_, entry_hash_);