SimpleCache: merge the first and second stream in one file

net/disk_cache/simple/simple_util.cc

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/disk_cache/simple/simple_util.h"
 
 #include <limits>
 
 #include "base/file_util.h"
 #include "base/format_macros.h"
@@ skipping 65 matching lines @@
                       key.size(), u.sha_hash);
   return u.key_hash;
 }
 
 std::string GetFilenameFromEntryHashAndIndex(uint64 entry_hash,
                                              int index) {
   return base::StringPrintf("%016" PRIx64 "_%1d", entry_hash, index);
 }
 
 std::string GetFilenameFromKeyAndIndex(const std::string& key, int index) {
   return GetEntryHashKeyAsHexString(key) + base::StringPrintf("_%1d", index);

[pasko, 2013/09/13 19:09:21]

it seems natural to replace "index" to "GetFileIndexFromStreamIndex(index)" in
this line.

Same with GetFilenameFromEntryHashAndIndex()

[clamy, 2013/09/16 15:01:17]

When creating the platform files, we are iterating on file index directly (ie we
only do 2 iterations instead of 3), while calling this function. So we cannot
replace the GetFileIndexFromStreamIndex in the current situation without
duplicating this function in a version that works with the stream index, and one
that works with the file index. Is that desirable?

[pasko, 2013/09/17 11:29:01]

makes sense

just discovered: this function is used only in tests, please move it to
simple_test_util.cc

also, since "index" is more ambiguous now than it used to be, please rename all
arguments named "index" in simple_util to become "file_index" or "stream_index",
there are just 2 remaining.

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

Done.

 }
 
 int32 GetDataSizeFromKeyAndFileSize(const std::string& key, int64 file_size) {
   int64 data_size = file_size - key.size() - sizeof(SimpleFileHeader) -
                     sizeof(SimpleFileEOF);
   DCHECK_GE(implicit_cast<int64>(std::numeric_limits<int32>::max()), data_size);
   return data_size;
 }
 
 int64 GetFileSizeFromKeyAndDataSize(const std::string& key, int32 data_size) {
   return data_size + key.size() + sizeof(SimpleFileHeader) +
       sizeof(SimpleFileEOF);
 }
 
-int64 GetFileOffsetFromKeyAndDataOffset(const std::string& key,
-                                        int data_offset) {
+int GetFileIndexFromStreamIndex(int stream_index) {
+  return (stream_index == 2) ? 1 : 0;

[pasko, 2013/09/13 19:09:21]

I would prefer to leave stream 2 with the "_2" suffix in the file name. This
backwards-compatibl-ish property helps in some situations like "a
simple_cache_dump_cache" tool that understands both old and new formats while it
seemingly does not have any downsides..

[clamy, 2013/09/16 15:01:17]

File index is being used as an offset in an array of 2 entries, therefore it has
to be strictly smaller than two. We can imagine a function that would convert
the file index in memory to a file name number on disk, but I think it is a bit
too much for an hypothetic backward compatibility.

[pasko, 2013/09/17 11:29:01]

Agreed.

+}
+
+int GetMaximumDataOffset(int file_index, const int data_size[]) {

[pasko, 2013/09/13 19:09:21]

Naming is fun. This name suggests (at least to me) that the function should
return the offset of the maximum byte of the data, while it returns the offset
of first non-data byte (well, counting the 0th EOF record as "data").

So how do we call it? How about GetLastEOFRecordOffset()?

[clamy, 2013/09/16 15:01:17]

Done.

+  if (file_index == 0)
+    return data_size[0] + data_size[1] + sizeof(SimpleFileEOF);
+  return data_size[2];
+}
+
+int64 GetFileOffsetFromDataOffset(const std::string& key,
+                                  int data_offset,
+                                  int index,
+                                  int stream_1_size) {
   const int64 headers_size = sizeof(disk_cache::SimpleFileHeader) + key.size();
-  return headers_size + data_offset;
+  const int64 additional_offset =
+      index == 0 ? stream_1_size + sizeof(SimpleFileEOF) : 0;
+  return headers_size + data_offset + additional_offset;
 }
 
 // TODO(clamy, gavinp): this should go in base
 bool GetMTime(const base::FilePath& path, base::Time* out_mtime) {
   DCHECK(out_mtime);
 #if defined(OS_POSIX)
   base::ThreadRestrictions::AssertIOAllowed();
   struct stat file_stat;
   if (stat(path.value().c_str(), &file_stat) != 0)
     return false;
   time_t sec;
   long nsec;
   if (GetNanoSecsFromStat(file_stat, &sec, &nsec)) {
     int64 usec = (nsec / base::Time::kNanosecondsPerMicrosecond);
     *out_mtime = base::Time::FromTimeT(sec)
         + base::TimeDelta::FromMicroseconds(usec);
     return true;
   }
 #endif
   base::PlatformFileInfo file_info;
   if (!file_util::GetFileInfo(path, &file_info))
     return false;
   *out_mtime = file_info.last_modified;
   return true;
 }
 
 }  // namespace simple_backend
 
 }  // namespace disk_cache