Disk cache: Introduce a BlockHeader class.

net/disk_cache/block_files.cc

Index: net/disk_cache/block_files.cc
===================================================================
--- net/disk_cache/block_files.cc	(revision 205976)
+++ net/disk_cache/block_files.cc	(working copy)
@@ -32,15 +32,29 @@
   return s_types[value];
 }
 
-void FixAllocationCounters(disk_cache::BlockFileHeader* header);
+}  // namespace
 
-// Creates a new entry on the allocation map, updating the apropriate counters.
-// target is the type of block to use (number of empty blocks), and size is the
-// actual number of blocks to use.
-bool CreateMapBlock(int target, int size, disk_cache::BlockFileHeader* header,
-                    int* index) {
-  if (target <= 0 || target > disk_cache::kMaxNumBlocks ||
-      size <= 0 || size > disk_cache::kMaxNumBlocks) {
+namespace disk_cache {
+
+BlockHeader::BlockHeader() : header_(NULL) {
+}
+
+BlockHeader::BlockHeader(BlockFileHeader* header) : header_(header) {
+}
+
+BlockHeader::BlockHeader(MappedFile* file)
+    : header_(reinterpret_cast<BlockFileHeader*>(file->buffer())) {

[gavinp, 2013/06/18 14:17:45]

Shouldn't this be a static_cast?

[rvargas (doing something else), 2013/06/18 18:12:23]

This is a cast between "unrelated" pointer types, so IMO logically should be a
reinterpret.

+}
+
+BlockHeader::BlockHeader(const BlockHeader& other) : header_(other.header_) {
+}
+
+BlockHeader::~BlockHeader() {
+}
+
+bool BlockHeader::CreateMapBlock(int target, int size, int* index) {
+  if (target <= 0 || target > kMaxNumBlocks ||
+      size <= 0 || size > kMaxNumBlocks) {
     NOTREACHED();
     return false;
   }
@@ -48,35 +62,35 @@
   TimeTicks start = TimeTicks::Now();
   // We are going to process the map on 32-block chunks (32 bits), and on every
   // chunk, iterate through the 8 nibbles where the new block can be located.
-  int current = header->hints[target - 1];
-  for (int i = 0; i < header->max_entries / 32; i++, current++) {
-    if (current == header->max_entries / 32)
+  int current = header_->hints[target - 1];
+  for (int i = 0; i < header_->max_entries / 32; i++, current++) {
+    if (current == header_->max_entries / 32)
       current = 0;
-    uint32 map_block = header->allocation_map[current];
+    uint32 map_block = header_->allocation_map[current];
 
     for (int j = 0; j < 8; j++, map_block >>= 4) {
       if (GetMapBlockType(map_block) != target)
         continue;
 
-      disk_cache::FileLock lock(header);
+      disk_cache::FileLock lock(header_);
       int index_offset = j * 4 + 4 - target;
       *index = current * 32 + index_offset;
       DCHECK_EQ(*index / 4, (*index + size - 1) / 4);
       uint32 to_add = ((1 << size) - 1) << index_offset;
-      header->num_entries++;
+      header_->num_entries++;
 
       // Note that there is no race in the normal sense here, but if we enforce
       // the order of memory accesses between num_entries and allocation_map, we
       // can assert that even if we crash here, num_entries will never be less
       // than the actual number of used blocks.
       base::subtle::MemoryBarrier();
-      header->allocation_map[current] |= to_add;
+      header_->allocation_map[current] |= to_add;
 
-      header->hints[target - 1] = current;
-      header->empty[target - 1]--;
-      DCHECK_GE(header->empty[target - 1], 0);
+      header_->hints[target - 1] = current;
+      header_->empty[target - 1]--;
+      DCHECK_GE(header_->empty[target - 1], 0);
       if (target != size) {
-        header->empty[target - size - 1]++;
+        header_->empty[target - size - 1]++;
       }
       HISTOGRAM_TIMES("DiskCache.CreateBlock", TimeTicks::Now() - start);
       return true;
@@ -86,33 +100,31 @@
   // It is possible to have an undetected corruption (for example when the OS
   // crashes), fix it here.
   LOG(ERROR) << "Failing CreateMapBlock";
-  FixAllocationCounters(header);
+  FixAllocationCounters();
   return false;
 }
 
-// Deletes the block pointed by index from allocation_map, and updates the
-// relevant counters on the header.
-void DeleteMapBlock(int index, int size, disk_cache::BlockFileHeader* header) {
-  if (size < 0 || size > disk_cache::kMaxNumBlocks) {
+void BlockHeader::DeleteMapBlock(int index, int size) {
+  if (size < 0 || size > kMaxNumBlocks) {
     NOTREACHED();
     return;
   }
   TimeTicks start = TimeTicks::Now();
   int byte_index = index / 8;
-  uint8* byte_map = reinterpret_cast<uint8*>(header->allocation_map);
+  uint8* byte_map = reinterpret_cast<uint8*>(header_->allocation_map);
   uint8 map_block = byte_map[byte_index];
 
   if (index % 8 >= 4)
     map_block >>= 4;
 
-  // See what type of block will be availabe after we delete this one.
+  // See what type of block will be available after we delete this one.
   int bits_at_end = 4 - size - index % 4;
   uint8 end_mask = (0xf << (4 - bits_at_end)) & 0xf;
   bool update_counters = (map_block & end_mask) == 0;
   uint8 new_value = map_block & ~(((1 << size) - 1) << (index % 4));
   int new_type = GetMapBlockType(new_value);
 
-  disk_cache::FileLock lock(header);
+  disk_cache::FileLock lock(header_);
   DCHECK((((1 << size) - 1) << (index % 8)) < 0x100);
   uint8  to_clear = ((1 << size) - 1) << (index % 8);
   DCHECK((byte_map[byte_index] & to_clear) == to_clear);
@@ -120,26 +132,24 @@
 
   if (update_counters) {
     if (bits_at_end)
-      header->empty[bits_at_end - 1]--;
-    header->empty[new_type - 1]++;
-    DCHECK_GE(header->empty[bits_at_end - 1], 0);
+      header_->empty[bits_at_end - 1]--;
+    header_->empty[new_type - 1]++;
+    DCHECK_GE(header_->empty[bits_at_end - 1], 0);
   }
   base::subtle::MemoryBarrier();
-  header->num_entries--;
-  DCHECK_GE(header->num_entries, 0);
+  header_->num_entries--;
+  DCHECK_GE(header_->num_entries, 0);
   HISTOGRAM_TIMES("DiskCache.DeleteBlock", TimeTicks::Now() - start);
 }
 
-#ifndef NDEBUG
-// Returns true if the specified block is used. Note that this is a simplified
-// version of DeleteMapBlock().
-bool UsedMapBlock(int index, int size, disk_cache::BlockFileHeader* header) {
-  if (size < 0 || size > disk_cache::kMaxNumBlocks) {
+// Note that this is a simplified version of DeleteMapBlock().
+bool BlockHeader::UsedMapBlock(int index, int size) {
+  if (size < 0 || size > kMaxNumBlocks) {
     NOTREACHED();
     return false;
   }
   int byte_index = index / 8;
-  uint8* byte_map = reinterpret_cast<uint8*>(header->allocation_map);
+  uint8* byte_map = reinterpret_cast<uint8*>(header_->allocation_map);
   uint8 map_block = byte_map[byte_index];
 
   if (index % 8 >= 4)
@@ -149,39 +159,34 @@
   uint8  to_clear = ((1 << size) - 1) << (index % 8);
   return ((byte_map[byte_index] & to_clear) == to_clear);
 }
-#endif  // NDEBUG
 
-// Restores the "empty counters" and allocation hints.
-void FixAllocationCounters(disk_cache::BlockFileHeader* header) {
-  for (int i = 0; i < disk_cache::kMaxNumBlocks; i++) {
-    header->hints[i] = 0;
-    header->empty[i] = 0;
+void BlockHeader::FixAllocationCounters() {
+  for (int i = 0; i < kMaxNumBlocks; i++) {
+    header_->hints[i] = 0;
+    header_->empty[i] = 0;
   }
 
-  for (int i = 0; i < header->max_entries / 32; i++) {
-    uint32 map_block = header->allocation_map[i];
+  for (int i = 0; i < header_->max_entries / 32; i++) {
+    uint32 map_block = header_->allocation_map[i];
 
     for (int j = 0; j < 8; j++, map_block >>= 4) {
       int type = GetMapBlockType(map_block);
       if (type)
-        header->empty[type -1]++;
+        header_->empty[type -1]++;
     }
   }
 }
 
-// Returns true if the current block file should not be used as-is to store more
-// records. |block_count| is the number of blocks to allocate.
-bool NeedToGrowBlockFile(const disk_cache::BlockFileHeader* header,
-                         int block_count) {
+bool BlockHeader::NeedToGrowBlockFile(int block_count) {
   bool have_space = false;
   int empty_blocks = 0;
-  for (int i = 0; i < disk_cache::kMaxNumBlocks; i++) {
-    empty_blocks += header->empty[i] * (i + 1);
-    if (i >= block_count - 1 && header->empty[i])
+  for (int i = 0; i < kMaxNumBlocks; i++) {
+    empty_blocks += header_->empty[i] * (i + 1);
+    if (i >= block_count - 1 && header_->empty[i])
       have_space = true;
   }
 
-  if (header->next_file && (empty_blocks < disk_cache::kMaxBlocks / 10)) {
+  if (header_->next_file && (empty_blocks < kMaxBlocks / 10)) {
     // This file is almost full but we already created another one, don't use
     // this file yet so that it is easier to find empty blocks when we start
     // using this file again.
@@ -190,33 +195,33 @@
   return !have_space;
 }
 
-// Returns the number of empty blocks for this file.
-int EmptyBlocks(const disk_cache::BlockFileHeader* header) {
+int BlockHeader::EmptyBlocks() const {
   int empty_blocks = 0;
   for (int i = 0; i < disk_cache::kMaxNumBlocks; i++) {
-    empty_blocks += header->empty[i] * (i + 1);
-    if (header->empty[i] < 0)
-      return false;
+    empty_blocks += header_->empty[i] * (i + 1);
+    if (header_->empty[i] < 0)
+      return 0;
   }
   return empty_blocks;
 }
 
-// Returns true if the counters look OK.
-bool ValidateCounters(const disk_cache::BlockFileHeader* header) {
-  if (header->max_entries < 0 || header->max_entries > disk_cache::kMaxBlocks ||
-      header->num_entries < 0)
+bool BlockHeader::ValidateCounters() const {
+  if (header_->max_entries < 0 || header_->max_entries > kMaxBlocks ||
+      header_->num_entries < 0)
     return false;
 
-  int empty_blocks = EmptyBlocks(header);
-  if (empty_blocks + header->num_entries > header->max_entries)
+  int empty_blocks = EmptyBlocks();
+  if (empty_blocks + header_->num_entries > header_->max_entries)
     return false;
 
   return true;
 }
 
-}  // namespace
+int BlockHeader::Size() const {

[gavinp, 2013/06/18 18:14:03]

This is a good candidate for an inline function, especially since it's compile
time evaluable.

[rvargas (doing something else), 2013/06/19 01:32:32]

This method will be removed when the code to open v3 block files land.

+  return static_cast<int>(sizeof(*header_));
+}
 
-namespace disk_cache {
+// ------------------------------------------------------------------------
 
 BlockFiles::BlockFiles(const base::FilePath& path)
     : init_(false), zero_buffer_(NULL), path_(path) {
@@ -285,7 +290,7 @@
     return false;
 
   ScopedFlush flush(file);
-  BlockFileHeader* header = reinterpret_cast<BlockFileHeader*>(file->buffer());
+  BlockHeader header(file);
 
   int target_size = 0;
   for (int i = block_count; i <= 4; i++) {
@@ -297,7 +302,7 @@
 
   DCHECK(target_size);
   int index;
-  if (!CreateMapBlock(target_size, block_count, header, &index))
+  if (!header.CreateMapBlock(target_size, block_count, &index))
     return false;
 
   Addr address(block_type, block_count, header->this_file, index);
@@ -327,8 +332,8 @@
   if (deep)
     file->Write(zero_buffer_, size, offset);
 
-  BlockFileHeader* header = reinterpret_cast<BlockFileHeader*>(file->buffer());
-  DeleteMapBlock(address.start_block(), address.num_blocks(), header);
+  BlockHeader header(file);
+  header.DeleteMapBlock(address.start_block(), address.num_blocks());
   file->Flush();
 
   if (!header->num_entries) {
@@ -383,8 +388,8 @@
   if (!file)
     return false;
 
-  BlockFileHeader* header = reinterpret_cast<BlockFileHeader*>(file->buffer());
-  bool rv = UsedMapBlock(address.start_block(), address.num_blocks(), header);
+  BlockHeader header(file);
+  bool rv = header.UsedMapBlock(address.start_block(), address.num_blocks());
   DCHECK(rv);
 
   static bool read_contents = false;
@@ -445,13 +450,13 @@
     return false;
   }
 
-  BlockFileHeader* header = reinterpret_cast<BlockFileHeader*>(file->buffer());
+  BlockHeader header(file);
   if (kBlockMagic != header->magic || kBlockVersion2 != header->version) {
     LOG(ERROR) << "Invalid file version or magic " << name.value();
     return false;
   }
 
-  if (header->updating || !ValidateCounters(header)) {
+  if (header->updating || !header.ValidateCounters()) {
     // Last instance was not properly shutdown, or counters are out of sync.
     if (!FixBlockFileHeader(file.get())) {
       LOG(ERROR) << "Unable to fix block file " << name.value();
@@ -511,19 +516,19 @@
 MappedFile* BlockFiles::FileForNewBlock(FileType block_type, int block_count) {
   COMPILE_ASSERT(RANKINGS == 1, invalid_file_type);
   MappedFile* file = block_files_[block_type - 1];
-  BlockFileHeader* header = reinterpret_cast<BlockFileHeader*>(file->buffer());
+  BlockHeader header(file);
 
   TimeTicks start = TimeTicks::Now();
-  while (NeedToGrowBlockFile(header, block_count)) {
+  while (header.NeedToGrowBlockFile(block_count)) {
     if (kMaxBlocks == header->max_entries) {
       file = NextFile(file);
       if (!file)
         return NULL;
-      header = reinterpret_cast<BlockFileHeader*>(file->buffer());
+      header = BlockHeader(file);
       continue;
     }
 
-    if (!GrowBlockFile(file, header))
+    if (!GrowBlockFile(file, header.Get()))
       return NULL;
     break;
   }
@@ -611,9 +616,9 @@
 // DCHECK on header->updating because we may be fixing a crash.
 bool BlockFiles::FixBlockFileHeader(MappedFile* file) {
   ScopedFlush flush(file);
-  BlockFileHeader* header = reinterpret_cast<BlockFileHeader*>(file->buffer());
+  BlockHeader header(file);
   int file_size = static_cast<int>(file->GetLength());
-  if (file_size < static_cast<int>(sizeof(*header)))
+  if (file_size < header.Size())
     return false;  // file_size > 2GB is also an error.
 
   const int kMinBlockSize = 36;
@@ -624,25 +629,25 @@
 
   // Make sure that we survive crashes.
   header->updating = 1;
-  int expected = header->entry_size * header->max_entries + sizeof(*header);
+  int expected = header->entry_size * header->max_entries + header.Size();
   if (file_size != expected) {
-    int max_expected = header->entry_size * kMaxBlocks + sizeof(*header);
+    int max_expected = header->entry_size * kMaxBlocks + header.Size();
     if (file_size < expected || header->empty[3] || file_size > max_expected) {
       NOTREACHED();
       LOG(ERROR) << "Unexpected file size";
       return false;
     }
     // We were in the middle of growing the file.
-    int num_entries = (file_size - sizeof(*header)) / header->entry_size;
+    int num_entries = (file_size - header.Size()) / header->entry_size;
     header->max_entries = num_entries;
   }
 
-  FixAllocationCounters(header);
-  int empty_blocks = EmptyBlocks(header);
+  header.FixAllocationCounters();
+  int empty_blocks = header.EmptyBlocks();
   if (empty_blocks + header->num_entries > header->max_entries)
     header->num_entries = header->max_entries - empty_blocks;
 
-  if (!ValidateCounters(header))
+  if (!header.ValidateCounters())
     return false;
 
   header->updating = 0;