[Setup]: Use memory mapped files in LzmaUtilImpl.

This CL removes usage of LzmaFileAllocator and tries to write unpack
result directly in the output file.
When uncompressing a folder that maps directly to a file, this is
possible and a memory mapping is used. In practice, this is always
the case. Support for folder containing multiple files is kept as
fallback and uses a pre-allocated file buffer, which is then copied in
individual files.

For running full setup install from chrome.packed.7z (Kb)
Before:
Setup.Install.PeakPagefileUsage 19748
Setup.Install.PeakWorkingSetSize 250528
GetCumulativeDiskUsageInBytes 761134
After:
Setup.Install.PeakPagefileUsage 19720
Setup.Install.PeakWorkingSetSize 250512
GetCumulativeDiskUsageInBytes 297818

The difference in memory is not significant, but improvement
are expected mostly for disk usage.

Bug: 933975
Change-Id: I26c1b118fc512d2caaa8112d3da37e6db460486b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1845956
Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
Reviewed-by: Greg Thompson <grt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#709940}

chrome/installer/setup/setup_util_unittest.cc

@@ -263,22 +263,19 @@ TEST(SetupUtilTest, RecordUnPackMetricsTest) {
       std::string(installer::kUnPackNTSTATUSMetricsName) + "_SetupExePatch";
   histogram_tester.ExpectTotalCount(unpack_status_metrics_name, 0);
 
-  RecordUnPackMetrics(UnPackStatus::UNPACK_NO_ERROR, 0, ERROR_SUCCESS,
+  RecordUnPackMetrics(UnPackStatus::UNPACK_NO_ERROR, base::nullopt,
+                      base::nullopt,
                       installer::UnPackConsumer::SETUP_EXE_PATCH);
   histogram_tester.ExpectTotalCount(unpack_status_metrics_name, 1);
   histogram_tester.ExpectBucketCount(unpack_status_metrics_name, 0, 1);
-  histogram_tester.ExpectTotalCount(unpack_result_metrics_name, 1);
-  histogram_tester.ExpectBucketCount(unpack_result_metrics_name, 0, 1);
-  histogram_tester.ExpectTotalCount(ntstatus_metrics_name, 1);
-  histogram_tester.ExpectBucketCount(ntstatus_metrics_name, 0, 1);
 
-  RecordUnPackMetrics(UnPackStatus::UNPACK_CLOSE_FILE_ERROR, 1, 2,
+  RecordUnPackMetrics(UnPackStatus::UNPACK_EXTRACT_ERROR, 1, 2,
                       installer::UnPackConsumer::SETUP_EXE_PATCH);
   histogram_tester.ExpectTotalCount(unpack_status_metrics_name, 2);
-  histogram_tester.ExpectBucketCount(unpack_status_metrics_name, 10, 1);
-  histogram_tester.ExpectTotalCount(unpack_result_metrics_name, 2);
+  histogram_tester.ExpectBucketCount(unpack_status_metrics_name, 4, 1);
+  histogram_tester.ExpectTotalCount(unpack_result_metrics_name, 1);
   histogram_tester.ExpectBucketCount(unpack_result_metrics_name, 2, 1);
-  histogram_tester.ExpectTotalCount(ntstatus_metrics_name, 2);
+  histogram_tester.ExpectTotalCount(ntstatus_metrics_name, 1);
   histogram_tester.ExpectBucketCount(ntstatus_metrics_name, 1, 1);
 }
 

chrome/installer/util/BUILD.gn

@@ -68,8 +68,6 @@ static_library("with_no_strings") {
       "html_dialog_impl.cc",
       "logging_installer.cc",
       "logging_installer.h",
-      "lzma_file_allocator.cc",
-      "lzma_file_allocator.h",
       "lzma_util.cc",
       "lzma_util.h",
       "master_preferences.cc",
@@ -275,7 +273,6 @@ if (is_win) {
       "install_util_unittest.cc",
       "l10n_string_util_unittest.cc",
       "logging_installer_unittest.cc",
-      "lzma_file_allocator_unittest.cc",
       "lzma_util_unittest.cc",
       "master_preferences_unittest.cc",
       "move_tree_work_item_unittest.cc",

chrome/installer/util/lzma_file_allocator.cc

-// Copyright 2015 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 "chrome/installer/util/lzma_file_allocator.h"
-
-#include "base/files/file_util.h"
-#include "base/logging.h"
-
-#include <windows.h>
-
-extern "C" {
-#include "third_party/lzma_sdk/7zAlloc.h"
-}
-
-LzmaFileAllocator::LzmaFileAllocator(const base::FilePath& temp_directory) {
-  DCHECK(base::DirectoryExists(temp_directory));
-
-  // Direct the ISzAlloc functions to the LZMA SDK's default allocator.
-  Alloc = SzAlloc;
-  Free = SzFree;
-
-  if (!base::CreateTemporaryFileInDir(temp_directory, &mapped_file_path_))
-    return;
-
-  mapped_file_.Initialize(mapped_file_path_,
-                          base::File::FLAG_CREATE_ALWAYS |
-                              base::File::FLAG_READ | base::File::FLAG_WRITE |
-                              base::File::FLAG_TEMPORARY |
-                              base::File::FLAG_DELETE_ON_CLOSE);
-  if (!mapped_file_.IsValid()) {
-    DPLOG(ERROR) << "Failed to initialize mapped file";
-    return;
-  }
-
-  // Direct the ISzAlloc functions to this class's allocator.
-  Alloc = SzFileAlloc;
-  Free = SzFileFree;
-}
-
-LzmaFileAllocator::~LzmaFileAllocator() {
-  DCHECK(!file_mapping_handle_.IsValid());
-  Alloc = nullptr;
-  Free = nullptr;
-}
-
-bool LzmaFileAllocator::IsAddressMapped(uintptr_t address) const {
-  return (address >= mapped_start_address_) &&
-         (address < mapped_start_address_ + mapped_size_);
-}
-
-void* LzmaFileAllocator::DoAllocate(size_t size) {
-  DCHECK(!file_mapping_handle_.IsValid());
-
-  if (size == 0)
-    return nullptr;
-
-  if (!mapped_file_.IsValid())
-    return SzAlloc(this, size);
-
-  if (!mapped_file_.SetLength(size)) {
-    DPLOG(ERROR) << "Failed to set length of mapped file";
-    return SzAlloc(this, size);
-  }
-
-  file_mapping_handle_.Set(::CreateFileMapping(
-      mapped_file_.GetPlatformFile(), nullptr, PAGE_READWRITE, 0, 0, nullptr));
-  if (!file_mapping_handle_.IsValid()) {
-    DPLOG(ERROR) << "Failed to create file mapping handle";
-    return SzAlloc(this, size);
-  }
-
-  void* ret =
-      ::MapViewOfFile(file_mapping_handle_.Get(), FILE_MAP_WRITE, 0, 0, 0);
-  if (!ret) {
-    DPLOG(ERROR) << "Failed to map view of file";
-    file_mapping_handle_.Close();
-    return SzAlloc(this, size);
-  }
-  mapped_start_address_ = reinterpret_cast<uintptr_t>(ret);
-  mapped_size_ = size;
-  return ret;
-}
-
-void LzmaFileAllocator::DoFree(void* address) {
-  if (address == nullptr)
-    return;
-  if (!file_mapping_handle_.IsValid()) {
-    SzFree(this, address);
-    return;
-  }
-  int ret = ::UnmapViewOfFile(address);
-  DPCHECK(ret != 0) << "Failed to unmap view of file";
-  mapped_start_address_ = 0;
-  mapped_size_ = 0;
-  file_mapping_handle_.Close();
-}
-
-void* LzmaFileAllocator::SzFileAlloc(void* p, size_t size) {
-  return static_cast<LzmaFileAllocator*>(p)->DoAllocate(size);
-}
-
-void LzmaFileAllocator::SzFileFree(void* p, void* address) {
-  static_cast<LzmaFileAllocator*>(p)->DoFree(address);
-}

chrome/installer/util/lzma_file_allocator.h

-// Copyright 2015 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.
-
-#ifndef CHROME_INSTALLER_UTIL_LZMA_FILE_ALLOCATOR_H_
-#define CHROME_INSTALLER_UTIL_LZMA_FILE_ALLOCATOR_H_
-
-#include <stddef.h>
-
-#include "base/files/file.h"
-#include "base/files/file_path.h"
-#include "base/gtest_prod_util.h"
-#include "base/macros.h"
-#include "base/win/scoped_handle.h"
-#include "third_party/lzma_sdk/7zTypes.h"
-
-// File mapping memory management class which supports multiple allocations in
-// series, but not in parallel. It creates a unique temporary file within
-// |temp_directory| to back subsequent allocations and falls back to normal
-// management in case of error.
-// Example:
-//   LzmaFileAllocator allocator(mmp_file_path);
-//   char* s = reinterpret_cast<char*>(IAlloc_Alloc(&allocator, size));
-//   IAlloc_Free(&allocator, s);
-class LzmaFileAllocator : public ISzAlloc {
- public:
-  explicit LzmaFileAllocator(const base::FilePath& temp_directory);
-  ~LzmaFileAllocator();
-  bool IsAddressMapped(uintptr_t address) const;
-
- private:
-  FRIEND_TEST_ALL_PREFIXES(LzmaFileAllocatorTest, ErrorAndFallbackTest);
-  FRIEND_TEST_ALL_PREFIXES(LzmaFileAllocatorTest, DeleteAfterCloseTest);
-
-  // Allocates |size| bytes backed by the instance's temporary file. The heap
-  // is used if file allocation fails for any reason.
-  void* DoAllocate(size_t size);
-
-  // Frees the memory from disk or heap depending on the type of allocation.
-  void DoFree(void* address);
-
-  // ISzAlloc hook functions
-  static void* SzFileAlloc(void* p, size_t size);
-  static void SzFileFree(void* p, void* address);
-
-  base::FilePath mapped_file_path_;
-  base::File mapped_file_;
-  base::win::ScopedHandle file_mapping_handle_;
-
-  uintptr_t mapped_start_address_ = 0;
-  size_t mapped_size_ = 0;
-
-  DISALLOW_COPY_AND_ASSIGN(LzmaFileAllocator);
-};
-
-#endif  // CHROME_INSTALLER_UTIL_LZMA_FILE_ALLOCATOR_H_

chrome/installer/util/lzma_file_allocator_unittest.cc

-// Copyright 2015 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 "chrome/installer/util/lzma_file_allocator.h"
-
-#include <stddef.h>
-
-#include <memory>
-#include <string>
-
-#include "base/files/file_util.h"
-#include "base/files/scoped_temp_dir.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-#include <windows.h>
-
-class LzmaFileAllocatorTest : public testing::Test {
- protected:
-  void SetUp() override { ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); }
-  // Returns the type of the memory page identified by |address|; one of
-  // MEM_IMAGE, MEM_MAPPED or MEM_PRIVATE.
-  static DWORD GetMemoryType(void* address);
-
-  base::ScopedTempDir temp_dir_;
-};
-
-DWORD LzmaFileAllocatorTest::GetMemoryType(void* address) {
-  MEMORY_BASIC_INFORMATION memory_info = {};
-  EXPECT_NE(0U, ::VirtualQuery(address, &memory_info,
-                               sizeof(memory_info)));
-  return memory_info.Type;
-}
-
-TEST_F(LzmaFileAllocatorTest, ReadAndWriteWithMultipleSizeTest) {
-  static const char kSampleExpectedCharacter = 'a';
-  SYSTEM_INFO sysinfo;
-  ::GetSystemInfo(&sysinfo);
-  EXPECT_GT(sysinfo.dwPageSize, 0U);
-
-  size_t size_list[] = {1, 10, sysinfo.dwPageSize - 1, sysinfo.dwPageSize,
-                        sysinfo.dwPageSize + 1};
-
-  for (size_t size : size_list) {
-    LzmaFileAllocator allocator(temp_dir_.GetPath());
-    char* s = reinterpret_cast<char*>(IAlloc_Alloc(&allocator, size));
-    std::fill_n(s, size, kSampleExpectedCharacter);
-    char* ret = std::find_if(s, s + size, [](char c) {
-      return c != kSampleExpectedCharacter;
-    });
-    EXPECT_EQ(s + size, ret);
-    EXPECT_EQ(static_cast<DWORD>(MEM_MAPPED), GetMemoryType(s));
-
-    IAlloc_Free(&allocator, s);
-  }
-}
-
-TEST_F(LzmaFileAllocatorTest, SizeIsZeroTest) {
-  LzmaFileAllocator allocator(temp_dir_.GetPath());
-  char* s = reinterpret_cast<char*>(IAlloc_Alloc(&allocator, 0));
-  EXPECT_EQ(s, nullptr);
-
-  IAlloc_Free(&allocator, s);
-}
-
-TEST_F(LzmaFileAllocatorTest, DeleteAfterCloseTest) {
-  std::unique_ptr<LzmaFileAllocator> allocator =
-      std::make_unique<LzmaFileAllocator>(temp_dir_.GetPath());
-  base::FilePath file_path = allocator->mapped_file_path_;
-  ASSERT_TRUE(base::PathExists(file_path));
-  allocator.reset();
-  ASSERT_FALSE(base::PathExists(file_path));
-}
-
-TEST_F(LzmaFileAllocatorTest, ErrorAndFallbackTest) {
-  LzmaFileAllocator allocator(temp_dir_.GetPath());
-  allocator.mapped_file_.Close();
-  char* s = reinterpret_cast<char*>(IAlloc_Alloc(&allocator, 10));
-  EXPECT_NE(nullptr, s);
-  ASSERT_FALSE(allocator.file_mapping_handle_.IsValid());
-  EXPECT_EQ(static_cast<DWORD>(MEM_PRIVATE), GetMemoryType(s));
-
-  IAlloc_Free(&allocator, s);
-}
-
-TEST_F(LzmaFileAllocatorTest, IsAddressMappedTest) {
-  LzmaFileAllocator allocator(temp_dir_.GetPath());
-  size_t size = 10;
-  uintptr_t address =
-      reinterpret_cast<uintptr_t>(IAlloc_Alloc(&allocator, size));
-  ASSERT_TRUE(allocator.IsAddressMapped(address));
-  ASSERT_TRUE(allocator.IsAddressMapped(address + 1));
-  ASSERT_TRUE(allocator.IsAddressMapped(address + 9));
-  ASSERT_FALSE(allocator.IsAddressMapped(address + 10));
-  ASSERT_FALSE(allocator.IsAddressMapped(address + 11));
-  ASSERT_FALSE(allocator.IsAddressMapped(address - 1));
-  IAlloc_Free(&allocator, reinterpret_cast<void*>(address));
-}

chrome/installer/util/lzma_util.h

@@ -26,7 +26,9 @@ enum UnPackStatus {
   UNPACK_CREATE_FILE_ERROR = 7,
   UNPACK_WRITE_FILE_ERROR = 8,
   UNPACK_SET_FILE_TIME_ERROR = 9,
-  UNPACK_CLOSE_FILE_ERROR = 10,
+  // UNPACK_CLOSE_FILE_ERROR = 10, Deprecated.
+  UNPACK_ALLOCATE_ERROR = 11,
+  UNPACK_CRC_ERROR = 12,
   UNPACK_STATUS_COUNT,
 };
 

tools/metrics/histograms/enums.xml

@@ -62046,7 +62046,9 @@ Full version information for the fingerprint enum values:
   <int value="7" label="Can not create extracted file on disk"/>
   <int value="8" label="Can not write the extracted file on disk"/>
   <int value="9" label="Can not set the extract time to file"/>
-  <int value="10" label="Can not close extracted file"/>
+  <int value="10" label="Can not close extracted file (Deprecated)"/>
+  <int value="11" label="Can not create file memory mapping"/>
+  <int value="12" label="File CRC mismatch"/>
 </enum>
 
 <enum name="UnsupportedContainers">