Skip to content

Commit

Permalink
[disk cache] Introduce a class to traverse a directory
Browse files Browse the repository at this point in the history
Split disk_cache::SimpleIndexFile::TraverseCacheDirectory into a
separate class so that we can use it from outside of disk cache easily.

Bug: 1289542
Change-Id: I3070e28efed065cafc077c5af7946a71de90f324
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3569143
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Maks Orlovich <morlovich@chromium.org>
Cr-Commit-Position: refs/heads/main@{#988928}
  • Loading branch information
yutakahirano authored and Chromium LUCI CQ committed Apr 5, 2022
1 parent 56e6876 commit 8921372
Show file tree
Hide file tree
Showing 10 changed files with 239 additions and 122 deletions.
6 changes: 4 additions & 2 deletions net/BUILD.gn
Expand Up @@ -515,6 +515,8 @@ component("net") {
"disk_cache/simple/simple_entry_impl.h",
"disk_cache/simple/simple_entry_operation.cc",
"disk_cache/simple/simple_entry_operation.h",
"disk_cache/simple/simple_file_enumerator.cc",
"disk_cache/simple/simple_file_enumerator.h",
"disk_cache/simple/simple_file_tracker.cc",
"disk_cache/simple/simple_file_tracker.h",
"disk_cache/simple/simple_histogram_macros.h",
Expand Down Expand Up @@ -1277,7 +1279,6 @@ component("net") {
"disk_cache/blockfile/file_win.cc",
"disk_cache/blockfile/mapped_file_win.cc",
"disk_cache/cache_util_win.cc",
"disk_cache/simple/simple_index_file_win.cc",
"disk_cache/simple/simple_util_win.cc",
"http/http_auth_handler_ntlm_win.cc",
"http/http_auth_sspi_win.cc",
Expand Down Expand Up @@ -1322,7 +1323,6 @@ component("net") {
"base/network_interfaces_posix.cc",
"base/network_interfaces_posix.h",
"disk_cache/cache_util_posix.cc",
"disk_cache/simple/simple_index_file_posix.cc",
"disk_cache/simple/simple_util_posix.cc",
"http/url_security_manager_posix.cc",
"socket/socket_posix.cc",
Expand Down Expand Up @@ -4215,6 +4215,7 @@ test("net_unittests") {
"disk_cache/blockfile/storage_block_unittest.cc",
"disk_cache/cache_util_unittest.cc",
"disk_cache/entry_unittest.cc",
"disk_cache/simple/simple_file_enumerator_unittest.cc",
"disk_cache/simple/simple_file_tracker_unittest.cc",
"disk_cache/simple/simple_index_file_unittest.cc",
"disk_cache/simple/simple_index_unittest.cc",
Expand Down Expand Up @@ -4711,6 +4712,7 @@ test("net_unittests") {
"disk_cache/blockfile/block_files_unittest.cc",

# Need to read input data files.
"disk_cache/simple/simple_file_enumerator_unittest.cc",
"socket/ssl_server_socket_unittest.cc",
"spdy/fuzzing/hpack_fuzz_util_test.cc",

Expand Down
@@ -0,0 +1 @@
hello
1 change: 1 addition & 0 deletions net/data/cache_tests/simple_file_enumerator/test.txt
@@ -0,0 +1 @@
hello, world
97 changes: 97 additions & 0 deletions net/disk_cache/simple/simple_file_enumerator.cc
@@ -0,0 +1,97 @@
// Copyright 2022 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_file_enumerator.h"

#include "base/files/file.h"
#include "base/files/file_util.h"
#include "base/logging.h"

using Entry = disk_cache::SimpleFileEnumerator::Entry;

// We have an optimized implementation for POSIX, and a fallback
// implementation for other platforms.

namespace disk_cache {

#if BUILDFLAG(IS_POSIX) || BUILDFLAG(IS_FUCHSIA)

SimpleFileEnumerator::SimpleFileEnumerator(const base::FilePath& path)
: path_(path), dir_(opendir(path.value().c_str())), has_error_(!dir_) {
if (has_error_) {
PLOG(ERROR) << "opendir " << path;
}
}
SimpleFileEnumerator::~SimpleFileEnumerator() = default;

bool SimpleFileEnumerator::HasError() const {
return has_error_;
}

absl::optional<Entry> SimpleFileEnumerator::Next() {
if (!dir_) {
return absl::nullopt;
}
while (true) {
// errno must be set to 0 before every readdir() call to detect errors.
errno = 0;
dirent* entry = readdir(dir_.get());
if (!entry) {
// Some implementations of readdir() (particularly older versions of
// Android Bionic) may leave errno set to EINTR even after they handle
// this case internally. It's safe to ignore EINTR in that case.
if (errno && errno != EINTR) {
PLOG(ERROR) << "readdir " << path_;
has_error_ = true;
dir_ = nullptr;
return absl::nullopt;
}
break;
}

const std::string filename(entry->d_name);
if (filename == "." || filename == "..") {
continue;
}
base::FilePath path = path_.Append(base::FilePath(filename));
base::File::Info file_info;
if (!base::GetFileInfo(path, &file_info)) {
LOG(ERROR) << "Could not get file info for " << path;
continue;
}
if (file_info.is_directory) {
continue;
}
return absl::make_optional<Entry>(std::move(path), file_info.size,
file_info.last_accessed,
file_info.last_modified);
}
dir_ = nullptr;
return absl::nullopt;
}

#else
SimpleFileEnumerator::SimpleFileEnumerator(const base::FilePath& path)
: enumerator_(path,
/*recursive=*/false,
base::FileEnumerator::FILES) {}
SimpleFileEnumerator::~SimpleFileEnumerator() = default;

bool SimpleFileEnumerator::HasError() const {
return enumerator_.GetError() != base::File::FILE_OK;
}

absl::optional<Entry> SimpleFileEnumerator::Next() {
base::FilePath path = enumerator_.Next();
if (path.empty()) {
return absl::nullopt;
}
base::FileEnumerator::FileInfo info = enumerator_.GetInfo();
return absl::make_optional<Entry>(std::move(path), info.GetSize(),
/*last_accessed=*/base::Time(),
info.GetLastModifiedTime());
}
#endif // BUILDFLAG(IS_POSIX) || BUILDFLAG(IS_FUCHSIA)

} // namespace disk_cache
71 changes: 71 additions & 0 deletions net/disk_cache/simple/simple_file_enumerator.h
@@ -0,0 +1,71 @@
// Copyright 2022 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 NET_DISK_CACHE_SIMPLE_SIMPLE_FILE_ENUMERATOR_H_
#define NET_DISK_CACHE_SIMPLE_SIMPLE_FILE_ENUMERATOR_H_

#include <memory>

#include "base/files/file_path.h"
#include "base/time/time.h"
#include "build/build_config.h"
#include "net/base/net_export.h"
#include "third_party/abseil-cpp/absl/types/optional.h"

#if BUILDFLAG(IS_POSIX) || BUILDFLAG(IS_FUCHSIA)
#include <dirent.h>
#include <sys/types.h>
#else
#include "base/files/file_enumerator.h"
#endif

namespace disk_cache {

// This is similar to base::SimpleFileEnumerator, but the implementation is
// optimized for the big directory use-case on POSIX. See
// https://crbug.com/270762 and https://codereview.chromium.org/22927018.
class NET_EXPORT SimpleFileEnumerator final {
public:
explicit SimpleFileEnumerator(const base::FilePath& root_path);
~SimpleFileEnumerator();

struct Entry {
Entry(base::FilePath path,
int64_t size,
base::Time last_accessed,
base::Time last_modified)
: path(std::move(path)),
size(size),
last_accessed(last_accessed),
last_modified(last_modified) {}

base::FilePath path;
int64_t size;
base::Time last_accessed;
base::Time last_modified;
};

// Returns true if we've found an error during enumeration.
bool HasError() const;

// Returns the next item, or nullopt if there are no more results (including
// the error case).
absl::optional<Entry> Next();

private:
#if BUILDFLAG(IS_POSIX) || BUILDFLAG(IS_FUCHSIA)
struct DirCloser {
void operator()(DIR* dir) { closedir(dir); }
};
const base::FilePath path_;
std::unique_ptr<DIR, DirCloser> dir_;
bool has_error_ = false;
#else
base::FileEnumerator enumerator_;
#endif
};

} // namespace disk_cache

#endif // NET_DISK_CACHE_SIMPLE_SIMPLE_FILE_ENUMERATOR_H_
57 changes: 57 additions & 0 deletions net/disk_cache/simple/simple_file_enumerator_unittest.cc
@@ -0,0 +1,57 @@
// Copyright 2022 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_file_enumerator.h"

#include "base/path_service.h"
#include "net/test/gtest_util.h"
#include "net/test/test_with_task_environment.h"
#include "testing/gtest/include/gtest/gtest.h"

namespace disk_cache {
namespace {

using Entry = SimpleFileEnumerator::Entry;

base::FilePath GetRoot() {
base::FilePath root;
base::PathService::Get(base::DIR_SOURCE_ROOT, &root);
return root.AppendASCII("net")
.AppendASCII("data")
.AppendASCII("cache_tests")
.AppendASCII("simple_file_enumerator");
}

TEST(SimpleFileEnumeratorTest, Root) {
const base::FilePath kRoot = GetRoot();
SimpleFileEnumerator enumerator(kRoot);

auto entry = enumerator.Next();
ASSERT_TRUE(entry.has_value());
EXPECT_EQ(entry->path, kRoot.AppendASCII("test.txt"));
EXPECT_EQ(entry->size, 13);
EXPECT_FALSE(enumerator.HasError());

// No directories should be listed, no indirect descendants should be listed.
EXPECT_EQ(absl::nullopt, enumerator.Next());
EXPECT_FALSE(enumerator.HasError());

// We can call enumerator.Next() after the iteration is done.
EXPECT_EQ(absl::nullopt, enumerator.Next());
EXPECT_FALSE(enumerator.HasError());
}

TEST(SimpleFileEnumeratorTest, NotFound) {
const base::FilePath kRoot = GetRoot().AppendASCII("not-found");
SimpleFileEnumerator enumerator(kRoot);

auto entry = enumerator.Next();
EXPECT_EQ(absl::nullopt, enumerator.Next());
#if BUILDFLAG(IS_POSIX) || BUILDFLAG(IS_FUCHSIA)
EXPECT_TRUE(enumerator.HasError());
#endif
}

} // namespace
} // namespace disk_cache
12 changes: 8 additions & 4 deletions net/disk_cache/simple/simple_index_file.cc
Expand Up @@ -20,6 +20,7 @@
#include "build/build_config.h"
#include "net/disk_cache/simple/simple_backend_impl.h"
#include "net/disk_cache/simple/simple_entry_format.h"
#include "net/disk_cache/simple/simple_file_enumerator.h"
#include "net/disk_cache/simple/simple_histogram_macros.h"
#include "net/disk_cache/simple/simple_index.h"
#include "net/disk_cache/simple/simple_synchronous_entry.h"
Expand Down Expand Up @@ -571,10 +572,13 @@ void SimpleIndexFile::SyncRestoreFromDisk(net::CacheType cache_type,
out_result->Reset();
SimpleIndex::EntrySet* entries = &out_result->entries;

const bool did_succeed = TraverseCacheDirectory(
cache_directory,
base::BindRepeating(&ProcessEntryFile, cache_type, entries));
if (!did_succeed) {
SimpleFileEnumerator enumerator(cache_directory);
using Entry = SimpleFileEnumerator::Entry;
while (absl::optional<Entry> entry = enumerator.Next()) {
ProcessEntryFile(cache_type, entries, entry->path, entry->last_accessed,
entry->last_modified, entry->size);
}
if (enumerator.HasError()) {
LOG(ERROR) << "Could not reconstruct index from disk";
return;
}
Expand Down
9 changes: 0 additions & 9 deletions net/disk_cache/simple/simple_index_file.h
Expand Up @@ -161,15 +161,6 @@ class NET_EXPORT_PRIVATE SimpleIndexFile {
base::Time* out_cache_last_modified,
SimpleIndexLoadResult* out_result);

// Implemented either in simple_index_file_posix.cc or
// simple_index_file_win.cc. base::FileEnumerator turned out to be very
// expensive in terms of memory usage therefore it's used only on non-POSIX
// environments for convenience (for now). Returns whether the traversal
// succeeded.
static bool TraverseCacheDirectory(
const base::FilePath& cache_path,
const EntryFileCallback& entry_file_callback);

// Writes the index file to disk atomically.
static void SyncWriteToDisk(net::CacheType cache_type,
const base::FilePath& cache_directory,
Expand Down
72 changes: 0 additions & 72 deletions net/disk_cache/simple/simple_index_file_posix.cc

This file was deleted.

0 comments on commit 8921372

Please sign in to comment.