Changeset 182124 in webkit

Timestamp:

Mar 29, 2015 4:43:32 PM (9 years ago)

Author:

Antti Koivisto

Message:

Use st_mtime instead of st_atime to track file access time
​https://bugs.webkit.org/show_bug.cgi?id=143200

Reviewed by Darin Adler.

On OS X atime updates automatically on read so calling Storage::traverse() would always ends up updating access times
for all cache entries to the current time. This would make entry worth computation produce unexpected results.
We update mtime manually on successful cache retrieve only so switching to it fixes the problem.

• NetworkProcess/cache/NetworkCacheFileSystemPosix.h:

(WebKit::NetworkCache::fileTimes):
(WebKit::NetworkCache::updateFileModificationTimeIfNeeded):
(WebKit::NetworkCache::updateFileAccessTimeIfNeeded): Deleted.

• NetworkProcess/cache/NetworkCacheStorage.cpp:

(WebKit::NetworkCache::Storage::updateFileModificationTime):
(WebKit::NetworkCache::Storage::dispatchReadOperation):
(WebKit::NetworkCache::deletionProbability):
(WebKit::NetworkCache::Storage::updateFileAccessTime): Deleted.

• NetworkProcess/cache/NetworkCacheStorage.h:

Location:

trunk/Source/WebKit2

Files:

• ChangeLog (modified) (1 diff)
• NetworkProcess/cache/NetworkCacheFileSystemPosix.h (modified) (2 diffs)
• NetworkProcess/cache/NetworkCacheStorage.cpp (modified) (3 diffs)
• NetworkProcess/cache/NetworkCacheStorage.h (modified) (1 diff)

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2015-03-29  Antti Koivisto  <antti@apple.com>
+
+        Use st_mtime instead of st_atime to track file access time
+        https://bugs.webkit.org/show_bug.cgi?id=143200
+
+        Reviewed by Darin Adler.
+
+        On OS X atime updates automatically on read so calling Storage::traverse() would always ends up updating access times
+        for all cache entries to the current time. This would make entry worth computation produce unexpected results.
+        We update mtime manually on successful cache retrieve only so switching to it fixes the problem.
+
+        * NetworkProcess/cache/NetworkCacheFileSystemPosix.h:
+        (WebKit::NetworkCache::fileTimes):
+        (WebKit::NetworkCache::updateFileModificationTimeIfNeeded):
+        (WebKit::NetworkCache::updateFileAccessTimeIfNeeded): Deleted.
+        * NetworkProcess/cache/NetworkCacheStorage.cpp:
+        (WebKit::NetworkCache::Storage::updateFileModificationTime):
+        (WebKit::NetworkCache::Storage::dispatchReadOperation):
+        (WebKit::NetworkCache::deletionProbability):
+        (WebKit::NetworkCache::Storage::updateFileAccessTime): Deleted.
+        * NetworkProcess/cache/NetworkCacheStorage.h:
+
 2015-03-27  Gwang Yoon Hwang  <yoon@igalia.com>
 

trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheFileSystemPosix.h

@@ -72 +72 @@
 struct FileTimes {
     std::chrono::system_clock::time_point creation;
-    std::chrono::system_clock::time_point access;
+    std::chrono::system_clock::time_point modification;
 };
 
@@ -80 +80 @@
     if (stat(WebCore::fileSystemRepresentation(path).data(), &fileInfo))
         return { };
-    return { std::chrono::system_clock::from_time_t(fileInfo.st_birthtime), std::chrono::system_clock::from_time_t(fileInfo.st_atime) };
+    return { std::chrono::system_clock::from_time_t(fileInfo.st_birthtime), std::chrono::system_clock::from_time_t(fileInfo.st_mtime) };
 }
 
-inline void updateFileAccessTimeIfNeeded(const String& path)
+inline void updateFileModificationTimeIfNeeded(const String& path)
 {
     auto times = fileTimes(path);
-    if (times.creation != times.access) {
-        // Don't update more than once per hour;
-        if (std::chrono::system_clock::now() - times.access < std::chrono::hours(1))
+    if (times.creation != times.modification) {
+        // Don't update more than once per hour.
+        if (std::chrono::system_clock::now() - times.modification < std::chrono::hours(1))
             return;
     }
-    // This really updates both access time and modification time.
+    // This really updates both the access time and the modification time.
     utimes(WebCore::fileSystemRepresentation(path).data(), 0);
 }

trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheStorage.cpp

@@ -293 +293 @@
 }
 
-void Storage::updateFileAccessTime(IOChannel& channel)
+void Storage::updateFileModificationTime(IOChannel& channel)
 {
     StringCapture filePathCapture(channel.path());
     serialBackgroundIOQueue().dispatch([filePathCapture] {
-        updateFileAccessTimeIfNeeded(filePathCapture.string());
+        updateFileModificationTimeIfNeeded(filePathCapture.string());
     });
 }
@@ -317 +317 @@
                 bool success = read.completionHandler(WTF::move(record));
                 if (success)
-                    updateFileAccessTime(*channel);
+                    updateFileModificationTime(*channel);
                 else
                     remove(read.key);
@@ -590 +590 @@
     using namespace std::chrono;
     auto age = system_clock::now() - times.creation;
-    auto accessAge = times.access - times.creation;
+    // File modification time is updated manually on cache read. We don't use access time since OS may update it automatically.
+    auto accessAge = times.modification - times.creation;
 
     // For sanity.

trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheStorage.h

@@ -98 +98 @@
     void dispatchPendingWriteOperations();
 
-    void updateFileAccessTime(IOChannel&);
+    void updateFileModificationTime(IOChannel&);
 
     WorkQueue& ioQueue() { return m_ioQueue.get(); }