Title: [278641] trunk/Source/WTF
- Revision
- 278641
- Author
- [email protected]
- Date
- 2021-06-08 19:26:34 -0700 (Tue, 08 Jun 2021)
Log Message
Use access instead of stat in some filesystem functions
https://bugs.webkit.org/show_bug.cgi?id=226667
Reviewed by Chris Dumez.
We are spending a bit more time in WTF::FileSystemImpl functions after the move to
std::filesystem (https://bugs.webkit.org/show_bug.cgi?id=225255). In particular, several
std::filesystem functions (like the ones called by fileExists and makeAllDirectories) prefer
to use stat() over access() for file existence checks. Since our sandbox has a fast path for
access(path, F_OK) but not for stat, we ended up spending more time in sandbox evaluation in
the kernel after the move to std::filesystem.
Note that the two checks don't do exactly the same thing. access(path, F_OK) only checks for
path existence, while stat(path) additionally fetches metadata, which requires checking the
file-read-metadata permission. But in practice our code was written to be fine with just
checking for existence.
To work around this, I've re-introduced some of the old WTF::FileSystemImpl functions from
FileSystemPosix.cpp. They are the ones that are called by NetworkCache, which seems to be
the biggest consumer of these functions. The rest of the functions are still implemented
using std::filesystem.
* wtf/FileSystem.cpp:
(WTF::FileSystemImpl::fileExists):
(WTF::FileSystemImpl::deleteFile):
(WTF::FileSystemImpl::makeAllDirectories):
(WTF::FileSystemImpl::pathByAppendingComponent):
(WTF::FileSystemImpl::pathByAppendingComponents):
* wtf/PlatformEnableCocoa.h:
* wtf/posix/FileSystemPOSIX.cpp:
(WTF::FileSystemImpl::fileExists):
(WTF::FileSystemImpl::deleteFile):
(WTF::FileSystemImpl::makeAllDirectories):
(WTF::FileSystemImpl::pathByAppendingComponent):
(WTF::FileSystemImpl::pathByAppendingComponents):
Modified Paths
Diff
Modified: trunk/Source/WTF/ChangeLog (278640 => 278641)
--- trunk/Source/WTF/ChangeLog 2021-06-09 02:23:59 UTC (rev 278640)
+++ trunk/Source/WTF/ChangeLog 2021-06-09 02:26:34 UTC (rev 278641)
@@ -1,3 +1,41 @@
+2021-06-08 Ben Nham <[email protected]>
+
+ Use access instead of stat in some filesystem functions
+ https://bugs.webkit.org/show_bug.cgi?id=226667
+
+ Reviewed by Chris Dumez.
+
+ We are spending a bit more time in WTF::FileSystemImpl functions after the move to
+ std::filesystem (https://bugs.webkit.org/show_bug.cgi?id=225255). In particular, several
+ std::filesystem functions (like the ones called by fileExists and makeAllDirectories) prefer
+ to use stat() over access() for file existence checks. Since our sandbox has a fast path for
+ access(path, F_OK) but not for stat, we ended up spending more time in sandbox evaluation in
+ the kernel after the move to std::filesystem.
+
+ Note that the two checks don't do exactly the same thing. access(path, F_OK) only checks for
+ path existence, while stat(path) additionally fetches metadata, which requires checking the
+ file-read-metadata permission. But in practice our code was written to be fine with just
+ checking for existence.
+
+ To work around this, I've re-introduced some of the old WTF::FileSystemImpl functions from
+ FileSystemPosix.cpp. They are the ones that are called by NetworkCache, which seems to be
+ the biggest consumer of these functions. The rest of the functions are still implemented
+ using std::filesystem.
+
+ * wtf/FileSystem.cpp:
+ (WTF::FileSystemImpl::fileExists):
+ (WTF::FileSystemImpl::deleteFile):
+ (WTF::FileSystemImpl::makeAllDirectories):
+ (WTF::FileSystemImpl::pathByAppendingComponent):
+ (WTF::FileSystemImpl::pathByAppendingComponents):
+ * wtf/PlatformEnableCocoa.h:
+ * wtf/posix/FileSystemPOSIX.cpp:
+ (WTF::FileSystemImpl::fileExists):
+ (WTF::FileSystemImpl::deleteFile):
+ (WTF::FileSystemImpl::makeAllDirectories):
+ (WTF::FileSystemImpl::pathByAppendingComponent):
+ (WTF::FileSystemImpl::pathByAppendingComponents):
+
2021-06-08 Devin Rousso <[email protected]>
[Payment Request] upstream new features
Modified: trunk/Source/WTF/wtf/FileSystem.cpp (278640 => 278641)
--- trunk/Source/WTF/wtf/FileSystem.cpp 2021-06-09 02:23:59 UTC (rev 278640)
+++ trunk/Source/WTF/wtf/FileSystem.cpp 2021-06-09 02:26:34 UTC (rev 278641)
@@ -522,26 +522,6 @@
#if HAVE(STD_FILESYSTEM) || HAVE(STD_EXPERIMENTAL_FILESYSTEM)
-bool fileExists(const String& path)
-{
- std::error_code ec;
- // exists() returns false on error so no need to check ec.
- return std::filesystem::exists(toStdFileSystemPath(path), ec);
-}
-
-bool deleteFile(const String& path)
-{
- std::error_code ec;
- auto fsPath = toStdFileSystemPath(path);
-
- auto fileStatus = std::filesystem::symlink_status(fsPath, ec);
- if (ec || fileStatus.type() == std::filesystem::file_type::directory)
- return false;
-
- // remove() returns false on error so no need to check ec.
- return std::filesystem::remove(fsPath, ec);
-}
-
bool deleteEmptyDirectory(const String& path)
{
std::error_code ec;
@@ -597,13 +577,6 @@
return size;
}
-bool makeAllDirectories(const String& path)
-{
- std::error_code ec;
- std::filesystem::create_directories(toStdFileSystemPath(path), ec);
- return !ec;
-}
-
std::optional<uint64_t> volumeFreeSpace(const String& path)
{
std::error_code ec;
@@ -728,6 +701,48 @@
return ec ? path : fromStdFileSystemPath(canonicalPath);
}
+Vector<String> listDirectory(const String& path)
+{
+ Vector<String> fileNames;
+ std::error_code ec;
+ auto entries = std::filesystem::directory_iterator(toStdFileSystemPath(path), ec);
+ for (auto it = std::filesystem::begin(entries), end = std::filesystem::end(entries); !ec && it != end; it.increment(ec)) {
+ auto fileName = fromStdFileSystemPath(it->path().filename());
+ if (!fileName.isNull())
+ fileNames.append(WTFMove(fileName));
+ }
+ return fileNames;
+}
+
+#if !ENABLE(FILESYSTEM_POSIX_FAST_PATH)
+
+bool fileExists(const String& path)
+{
+ std::error_code ec;
+ // exists() returns false on error so no need to check ec.
+ return std::filesystem::exists(toStdFileSystemPath(path), ec);
+}
+
+bool deleteFile(const String& path)
+{
+ std::error_code ec;
+ auto fsPath = toStdFileSystemPath(path);
+
+ auto fileStatus = std::filesystem::symlink_status(fsPath, ec);
+ if (ec || fileStatus.type() == std::filesystem::file_type::directory)
+ return false;
+
+ // remove() returns false on error so no need to check ec.
+ return std::filesystem::remove(fsPath, ec);
+}
+
+bool makeAllDirectories(const String& path)
+{
+ std::error_code ec;
+ std::filesystem::create_directories(toStdFileSystemPath(path), ec);
+ return !ec;
+}
+
String pathByAppendingComponent(const String& path, const String& component)
{
return fromStdFileSystemPath(toStdFileSystemPath(path) / toStdFileSystemPath(component));
@@ -741,18 +756,7 @@
return fromStdFileSystemPath(fsPath);
}
-Vector<String> listDirectory(const String& path)
-{
- Vector<String> fileNames;
- std::error_code ec;
- auto entries = std::filesystem::directory_iterator(toStdFileSystemPath(path), ec);
- for (auto it = std::filesystem::begin(entries), end = std::filesystem::end(entries); !ec && it != end; it.increment(ec)) {
- auto fileName = fromStdFileSystemPath(it->path().filename());
- if (!fileName.isNull())
- fileNames.append(WTFMove(fileName));
- }
- return fileNames;
-}
+#endif
#endif // HAVE(STD_FILESYSTEM) || HAVE(STD_EXPERIMENTAL_FILESYSTEM)
Modified: trunk/Source/WTF/wtf/PlatformEnableCocoa.h (278640 => 278641)
--- trunk/Source/WTF/wtf/PlatformEnableCocoa.h 2021-06-09 02:23:59 UTC (rev 278640)
+++ trunk/Source/WTF/wtf/PlatformEnableCocoa.h 2021-06-09 02:26:34 UTC (rev 278641)
@@ -220,6 +220,10 @@
#define ENABLE_FILE_REPLACEMENT 1
#endif
+#if !defined(ENABLE_FILESYSTEM_POSIX_FAST_PATH)
+#define ENABLE_FILESYSTEM_POSIX_FAST_PATH 1
+#endif
+
#if !defined(ENABLE_FILTERS_LEVEL_2)
#define ENABLE_FILTERS_LEVEL_2 1
#endif
Modified: trunk/Source/WTF/wtf/posix/FileSystemPOSIX.cpp (278640 => 278641)
--- trunk/Source/WTF/wtf/posix/FileSystemPOSIX.cpp 2021-06-09 02:23:59 UTC (rev 278640)
+++ trunk/Source/WTF/wtf/posix/FileSystemPOSIX.cpp 2021-06-09 02:26:34 UTC (rev 278641)
@@ -235,5 +235,77 @@
return fileStat.st_dev;
}
+#if ENABLE(FILESYSTEM_POSIX_FAST_PATH)
+
+bool fileExists(const String& path)
+{
+ return access(fileSystemRepresentation(path).data(), F_OK) != -1;
+}
+
+bool deleteFile(const String& path)
+{
+ // unlink(...) returns 0 on successful deletion of the path and non-zero in any other case (including invalid permissions or non-existent file)
+ bool unlinked = !unlink(fileSystemRepresentation(path).data());
+ if (!unlinked && errno != ENOENT)
+ LOG_ERROR("File failed to delete. Error message: %s", strerror(errno));
+
+ return unlinked;
+}
+
+bool makeAllDirectories(const String& path)
+{
+ auto fullPath = fileSystemRepresentation(path);
+ if (!access(fullPath.data(), F_OK))
+ return true;
+
+ char* p = fullPath.mutableData() + 1;
+ int length = fullPath.length();
+ if (p[length - 1] == '/')
+ p[length - 1] = '\0';
+ for (; *p; ++p) {
+ if (*p == '/') {
+ *p = '\0';
+ if (access(fullPath.data(), F_OK)) {
+ if (mkdir(fullPath.data(), S_IRWXU))
+ return false;
+ }
+ *p = '/';
+ }
+ }
+ if (access(fullPath.data(), F_OK)) {
+ if (mkdir(fullPath.data(), S_IRWXU))
+ return false;
+ }
+
+ return true;
+}
+
+String pathByAppendingComponent(const String& path, const String& component)
+{
+ if (path.endsWith('/'))
+ return path + component;
+ return path + "/" + component;
+}
+
+String pathByAppendingComponents(StringView path, const Vector<StringView>& components)
+{
+ StringBuilder builder;
+ builder.append(path);
+ bool isFirstComponent = true;
+ for (auto& component : components) {
+ if (isFirstComponent) {
+ isFirstComponent = false;
+ if (path.endsWith('/')) {
+ builder.append(component);
+ continue;
+ }
+ }
+ builder.append('/', component);
+ }
+ return builder.toString();
+}
+
+#endif
+
} // namespace FileSystemImpl
} // namespace WTF
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
