Skip to content

Commit

Permalink
SkSL precompile (flutter#12412)
Browse files Browse the repository at this point in the history
For flutter/flutter#40686

Unit tests added:
- CacheSkSLWorks
- VisitFilesCanBeCalledTwice
- CanListFilesRecursively
  • Loading branch information
liyuqian authored Oct 8, 2019
1 parent 25e2f03 commit df0e911
Show file tree
Hide file tree
Showing 21 changed files with 563 additions and 43 deletions.
1 change: 1 addition & 0 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,7 @@ FILE: ../../../flutter/shell/common/isolate_configuration.cc
FILE: ../../../flutter/shell/common/isolate_configuration.h
FILE: ../../../flutter/shell/common/persistent_cache.cc
FILE: ../../../flutter/shell/common/persistent_cache.h
FILE: ../../../flutter/shell/common/persistent_cache_unittests.cc
FILE: ../../../flutter/shell/common/pipeline.cc
FILE: ../../../flutter/shell/common/pipeline.h
FILE: ../../../flutter/shell/common/pipeline_unittests.cc
Expand Down
1 change: 1 addition & 0 deletions common/settings.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ std::string Settings::ToString() const {
stream << "trace_systrace: " << trace_systrace << std::endl;
stream << "dump_skp_on_shader_compilation: " << dump_skp_on_shader_compilation
<< std::endl;
stream << "cache_sksl: " << cache_sksl << std::endl;
stream << "endless_trace_buffer: " << endless_trace_buffer << std::endl;
stream << "enable_dart_profiling: " << enable_dart_profiling << std::endl;
stream << "disable_dart_asserts: " << disable_dart_asserts << std::endl;
Expand Down
1 change: 1 addition & 0 deletions common/settings.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ struct Settings {
bool trace_startup = false;
bool trace_systrace = false;
bool dump_skp_on_shader_compilation = false;
bool cache_sksl = false;
bool endless_trace_buffer = false;
bool enable_dart_profiling = false;
bool disable_dart_asserts = false;
Expand Down
34 changes: 34 additions & 0 deletions fml/file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "flutter/fml/file.h"

#include "flutter/fml/logging.h"
#include "flutter/fml/unique_fd.h"

namespace fml {

Expand Down Expand Up @@ -51,11 +52,44 @@ ScopedTemporaryDirectory::ScopedTemporaryDirectory() {
}

ScopedTemporaryDirectory::~ScopedTemporaryDirectory() {
// Windows has to close UniqueFD first before UnlinkDirectory
dir_fd_.reset();
if (path_ != "") {
if (!UnlinkDirectory(path_.c_str())) {
FML_LOG(ERROR) << "Could not remove directory: " << path_;
}
}
}

bool VisitFilesRecursively(const fml::UniqueFD& directory,
FileVisitor visitor) {
FileVisitor recursive_visitor = [&recursive_visitor, &visitor](
const UniqueFD& directory,
const std::string& filename) {
if (!visitor(directory, filename)) {
return false;
}
if (IsDirectory(directory, filename.c_str())) {
UniqueFD sub_dir = OpenDirectoryReadOnly(directory, filename.c_str());
if (!sub_dir.is_valid()) {
FML_LOG(ERROR) << "Can't open sub-directory: " << filename;
return true;
}
return VisitFiles(sub_dir, recursive_visitor);
}
return true;
};
return VisitFiles(directory, recursive_visitor);
}

fml::UniqueFD OpenFileReadOnly(const fml::UniqueFD& base_directory,
const char* path) {
return OpenFile(base_directory, path, false, FilePermission::kRead);
}

fml::UniqueFD OpenDirectoryReadOnly(const fml::UniqueFD& base_directory,
const char* path) {
return OpenDirectory(base_directory, path, false, FilePermission::kRead);
}

} // namespace fml
49 changes: 49 additions & 0 deletions fml/file.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#ifndef FLUTTER_FML_FILE_H_
#define FLUTTER_FML_FILE_H_

#include <functional>
#include <initializer_list>
#include <string>
#include <vector>
Expand All @@ -28,15 +29,24 @@ enum class FilePermission {

std::string CreateTemporaryDirectory();

/// This can open a directory on POSIX, but not on Windows.
fml::UniqueFD OpenFile(const char* path,
bool create_if_necessary,
FilePermission permission);

/// This can open a directory on POSIX, but not on Windows.
fml::UniqueFD OpenFile(const fml::UniqueFD& base_directory,
const char* path,
bool create_if_necessary,
FilePermission permission);

/// Helper method that calls `OpenFile` with create_if_necessary = false
/// and permission = kRead.
///
/// This can open a directory on POSIX, but not on Windows.
fml::UniqueFD OpenFileReadOnly(const fml::UniqueFD& base_directory,
const char* path);

fml::UniqueFD OpenDirectory(const char* path,
bool create_if_necessary,
FilePermission permission);
Expand All @@ -46,10 +56,17 @@ fml::UniqueFD OpenDirectory(const fml::UniqueFD& base_directory,
bool create_if_necessary,
FilePermission permission);

/// Helper method that calls `OpenDirectory` with create_if_necessary = false
/// and permission = kRead.
fml::UniqueFD OpenDirectoryReadOnly(const fml::UniqueFD& base_directory,
const char* path);

fml::UniqueFD Duplicate(fml::UniqueFD::element_type descriptor);

bool IsDirectory(const fml::UniqueFD& directory);

bool IsDirectory(const fml::UniqueFD& base_directory, const char* path);

// Returns whether the given path is a file.
bool IsFile(const std::string& path);

Expand All @@ -73,12 +90,44 @@ bool WriteAtomically(const fml::UniqueFD& base_directory,
const char* file_name,
const Mapping& mapping);

/// Signature of a callback on a file in `directory` with `filename` (relative
/// to `directory`). The returned bool should be false if and only if further
/// traversal should be stopped. For example, a file-search visitor may return
/// false when the file is found so no more visiting is needed.
using FileVisitor = std::function<bool(const fml::UniqueFD& directory,
const std::string& filename)>;

/// Call `visitor` on all files inside the `directory` non-recursively. The
/// trivial file "." and ".." will not be visited.
///
/// Return false if and only if the visitor returns false during the
/// traversal.
///
/// If recursive visiting is needed, call `VisitFiles` inside the `visitor`, or
/// use our helper method `VisitFilesRecursively`.
///
/// @see `VisitFilesRecursively`.
bool VisitFiles(const fml::UniqueFD& directory, FileVisitor visitor);

/// Recursively call `visitor` on all files inside the `directory`. Return false
/// if and only if the visitor returns false during the traversal.
///
/// This is a helper method that wraps the general `VisitFiles` method. The
/// `VisitFiles` is strictly more powerful as it has the access of the recursion
/// stack to the file. For example, `VisitFiles` may be able to maintain a
/// vector of directory names that lead to a file. That could be useful to
/// compute the relative path between the root directory and the visited file.
///
/// @see `VisitFiles`.
bool VisitFilesRecursively(const fml::UniqueFD& directory, FileVisitor visitor);

class ScopedTemporaryDirectory {
public:
ScopedTemporaryDirectory();

~ScopedTemporaryDirectory();

const std::string& path() const { return path_; }
const UniqueFD& fd() { return dir_fd_; }

private:
Expand Down
110 changes: 104 additions & 6 deletions fml/file_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "flutter/fml/build_config.h"
#include "flutter/fml/file.h"
#include "flutter/fml/mapping.h"
#include "flutter/fml/unique_fd.h"

static bool WriteStringToFile(const fml::UniqueFD& fd,
const std::string& contents) {
Expand Down Expand Up @@ -131,6 +132,101 @@ TEST(FileTest, CreateDirectoryStructure) {
ASSERT_TRUE(fml::UnlinkDirectory(dir.fd(), "a"));
}

TEST(FileTest, VisitFilesCanBeCalledTwice) {
fml::ScopedTemporaryDirectory dir;

{
auto file = fml::OpenFile(dir.fd(), "my_contents", true,
fml::FilePermission::kReadWrite);
ASSERT_TRUE(file.is_valid());
}

int count;
fml::FileVisitor count_visitor = [&count](const fml::UniqueFD& directory,
const std::string& filename) {
count += 1;
return true;
};
count = 0;
fml::VisitFiles(dir.fd(), count_visitor);
ASSERT_EQ(count, 1);

// Without `rewinddir` in `VisitFiles`, the following check would fail.
count = 0;
fml::VisitFiles(dir.fd(), count_visitor);
ASSERT_EQ(count, 1);

ASSERT_TRUE(fml::UnlinkFile(dir.fd(), "my_contents"));
}

TEST(FileTest, CanListFilesRecursively) {
fml::ScopedTemporaryDirectory dir;

{
auto c = fml::CreateDirectory(dir.fd(), {"a", "b", "c"},
fml::FilePermission::kReadWrite);
ASSERT_TRUE(c.is_valid());
auto file1 =
fml::OpenFile(c, "file1", true, fml::FilePermission::kReadWrite);
auto file2 =
fml::OpenFile(c, "file2", true, fml::FilePermission::kReadWrite);
auto d = fml::CreateDirectory(c, {"d"}, fml::FilePermission::kReadWrite);
ASSERT_TRUE(d.is_valid());
auto file3 =
fml::OpenFile(d, "file3", true, fml::FilePermission::kReadWrite);
ASSERT_TRUE(file1.is_valid());
ASSERT_TRUE(file2.is_valid());
ASSERT_TRUE(file3.is_valid());
}

std::set<std::string> names;
fml::FileVisitor visitor = [&names](const fml::UniqueFD& directory,
const std::string& filename) {
names.insert(filename);
return true;
};

fml::VisitFilesRecursively(dir.fd(), visitor);
ASSERT_EQ(names, std::set<std::string>(
{"a", "b", "c", "d", "file1", "file2", "file3"}));

// Cleanup.
ASSERT_TRUE(fml::UnlinkFile(dir.fd(), "a/b/c/d/file3"));
ASSERT_TRUE(fml::UnlinkFile(dir.fd(), "a/b/c/file1"));
ASSERT_TRUE(fml::UnlinkFile(dir.fd(), "a/b/c/file2"));
ASSERT_TRUE(fml::UnlinkDirectory(dir.fd(), "a/b/c/d"));
ASSERT_TRUE(fml::UnlinkDirectory(dir.fd(), "a/b/c"));
ASSERT_TRUE(fml::UnlinkDirectory(dir.fd(), "a/b"));
ASSERT_TRUE(fml::UnlinkDirectory(dir.fd(), "a"));
}

TEST(FileTest, CanStopVisitEarly) {
fml::ScopedTemporaryDirectory dir;

{
auto d = fml::CreateDirectory(dir.fd(), {"a", "b", "c", "d"},
fml::FilePermission::kReadWrite);
ASSERT_TRUE(d.is_valid());
}

std::set<std::string> names;
fml::FileVisitor visitor = [&names](const fml::UniqueFD& directory,
const std::string& filename) {
names.insert(filename);
return filename == "c" ? false : true; // stop if c is found
};

// Check the d is not visited as we stop at c.
ASSERT_FALSE(fml::VisitFilesRecursively(dir.fd(), visitor));
ASSERT_EQ(names, std::set<std::string>({"a", "b", "c"}));

// Cleanup.
ASSERT_TRUE(fml::UnlinkDirectory(dir.fd(), "a/b/c/d"));
ASSERT_TRUE(fml::UnlinkDirectory(dir.fd(), "a/b/c"));
ASSERT_TRUE(fml::UnlinkDirectory(dir.fd(), "a/b"));
ASSERT_TRUE(fml::UnlinkDirectory(dir.fd(), "a"));
}

#if OS_WIN
#define AtomicWriteTest DISABLED_AtomicWriteTest
#else
Expand Down Expand Up @@ -159,13 +255,15 @@ TEST(FileTest, AtomicWriteTest) {
TEST(FileTest, EmptyMappingTest) {
fml::ScopedTemporaryDirectory dir;

auto file = fml::OpenFile(dir.fd(), "my_contents", true,
fml::FilePermission::kReadWrite);
{
auto file = fml::OpenFile(dir.fd(), "my_contents", true,
fml::FilePermission::kReadWrite);

fml::FileMapping mapping(file);
ASSERT_TRUE(mapping.IsValid());
ASSERT_EQ(mapping.GetSize(), 0ul);
ASSERT_EQ(mapping.GetMapping(), nullptr);
fml::FileMapping mapping(file);
ASSERT_TRUE(mapping.IsValid());
ASSERT_EQ(mapping.GetSize(), 0ul);
ASSERT_EQ(mapping.GetMapping(), nullptr);
}

ASSERT_TRUE(fml::UnlinkFile(dir.fd(), "my_contents"));
}
39 changes: 38 additions & 1 deletion fml/platform/posix/file_posix.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "flutter/fml/file.h"

#include <dirent.h>
#include <fcntl.h>
#include <sys/mman.h>
#include <sys/stat.h>
Expand All @@ -13,7 +14,9 @@
#include <sstream>

#include "flutter/fml/eintr_wrapper.h"
#include "flutter/fml/logging.h"
#include "flutter/fml/mapping.h"
#include "flutter/fml/unique_fd.h"

namespace fml {

Expand Down Expand Up @@ -132,6 +135,11 @@ bool IsDirectory(const fml::UniqueFD& directory) {
return S_ISDIR(stat_result.st_mode);
}

bool IsDirectory(const fml::UniqueFD& base_directory, const char* path) {
UniqueFD file = OpenFileReadOnly(base_directory, path);
return (file.is_valid() && IsDirectory(file));
}

bool IsFile(const std::string& path) {
struct stat buf;
if (stat(path.c_str(), &buf) != 0) {
Expand Down Expand Up @@ -162,7 +170,11 @@ bool UnlinkFile(const char* path) {
}

bool UnlinkFile(const fml::UniqueFD& base_directory, const char* path) {
return ::unlinkat(base_directory.get(), path, 0) == 0;
int code = ::unlinkat(base_directory.get(), path, 0);
if (code != 0) {
FML_DLOG(ERROR) << strerror(errno);
}
return code == 0;
}

bool FileExists(const fml::UniqueFD& base_directory, const char* path) {
Expand Down Expand Up @@ -210,4 +222,29 @@ bool WriteAtomically(const fml::UniqueFD& base_directory,
base_directory.get(), file_name) == 0;
}

bool VisitFiles(const fml::UniqueFD& directory, FileVisitor visitor) {
// We cannot call closedir(dir) because it will also close the corresponding
// UniqueFD, and later reference to that UniqueFD will fail. Also, we don't
// have to call closedir because UniqueFD will call close on its destructor.
DIR* dir = ::fdopendir(directory.get());
if (dir == nullptr) {
FML_DLOG(ERROR) << "Can't open the directory. Error: " << strerror(errno);
return true; // continue to visit other files
}

// Without `rewinddir`, `readir` will directly return NULL (end of dir is
// reached) after a previuos `VisitFiles` call for the same `const
// fml::UniqueFd& directory`.
rewinddir(dir);
while (dirent* ent = readdir(dir)) {
std::string filename = ent->d_name;
if (filename != "." && filename != "..") {
if (!visitor(directory, filename)) {
return false;
}
}
}
return true;
}

} // namespace fml
Loading

0 comments on commit df0e911

Please sign in to comment.