Optimize execution time of the memdump tool

tools/android/memdump/memdump.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 <fcntl.h>
 #include <signal.h>
 #include <sys/types.h>
 #include <unistd.h>
 
 #include <algorithm>
@@ skipping 182 matching lines @@
   }
   return true;
 }
 
 // Fills |committed_pages| in with the set of committed pages contained in the
 // provided memory map.
 bool GetPagesForMemoryMap(int pagemap_fd,
                           const MemoryMap& memory_map,
                           std::vector<PageInfo>* committed_pages,
                           BitSet* committed_pages_bits) {
+  off64_t offset = memory_map.start_address / kPageSize;

[Philippe, 2014/02/26 12:05:48]

Nit: const off64_t offset

Reviewers in Chromium would not make this comment usually, but you will notice
here that we use constness extensively :) (mostly for documentation
purpose/extra type safety).

[hyunki, 2014/02/27 01:15:06]

Done.

+  if (lseek64(pagemap_fd, offset * sizeof(PageMapEntry), SEEK_SET) < 0) {
+    PLOG(ERROR) << "lseek";
+    return false;
+  }
   for (uint addr = memory_map.start_address, page_index = 0;
        addr < memory_map.end_address;
        addr += kPageSize, ++page_index) {
     DCHECK_EQ(0, addr % kPageSize);
     PageMapEntry page_map_entry = {};
     COMPILE_ASSERT(sizeof(PageMapEntry) == sizeof(uint64), unexpected_size);
-    const off64_t offset = addr / kPageSize;
-    if (!ReadFromFileAtOffset(pagemap_fd, offset, &page_map_entry))
+    offset++;

[Philippe, 2014/02/26 12:05:48]

Nit: You don't seem to be using |offset| in the loop so you can drop this line
(which also allows |offset| to be const).

[hyunki, 2014/02/27 01:15:06]

Done.

+    ssize_t bytes = read(pagemap_fd, &page_map_entry, sizeof(PageMapEntry));

[Philippe, 2014/02/26 12:05:48]

Nit: sizeof(page_map_entry) would be slightly better. And to add an extra const,
you can rewrite this line like this (if it doesn't fit on a single line):

const ssize_t bytes = read(
    pagemap_fd, &page_map_entry, sizeof(page_map_entry));

[hyunki, 2014/02/27 01:15:06]

Done.

+    if (bytes != sizeof(PageMapEntry) && bytes != 0) {
+      PLOG(ERROR) << "read";
       return false;
+    }
     if (page_map_entry.present) {  // Ignore non-committed pages.
       if (page_map_entry.page_frame_number == 0)
         continue;
       PageInfo page_info = {};
       page_info.page_frame_number = page_map_entry.page_frame_number;
       committed_pages->push_back(page_info);
       committed_pages_bits->set(page_index);
     }
   }
   return true;
@@ skipping 290 matching lines @@
     }
   }
 
   ClassifyPages(&processes_memory);
   if (short_output)
     DumpProcessesMemoryMapsInShortFormat(processes_memory);
   else
     DumpProcessesMemoryMapsInExtendedFormat(processes_memory);
   return EXIT_SUCCESS;
 }