Use st_mtime instead of st_atime to track file access time
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 29 Mar 2015 23:43:32 +0000 (23:43 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 29 Mar 2015 23:43:32 +0000 (23:43 +0000)
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:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@182124 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit2/ChangeLog
Source/WebKit2/NetworkProcess/cache/NetworkCacheFileSystemPosix.h
Source/WebKit2/NetworkProcess/cache/NetworkCacheStorage.cpp
Source/WebKit2/NetworkProcess/cache/NetworkCacheStorage.h

index fa22d48..e62b4a8 100644 (file)
@@ -1,3 +1,25 @@
+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>
 
         [TexMap] Seperate BitmapTexture related classes implementations from TextureMapper
index ba56e3c..185c2be 100644 (file)
@@ -71,7 +71,7 @@ inline void traverseCacheFiles(const String& cachePath, const Function& function
 
 struct FileTimes {
     std::chrono::system_clock::time_point creation;
-    std::chrono::system_clock::time_point access;
+    std::chrono::system_clock::time_point modification;
 };
 
 inline FileTimes fileTimes(const String& path)
@@ -79,18 +79,18 @@ inline FileTimes fileTimes(const String& path)
     struct stat fileInfo;
     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);
 }
 
index a51ac85..3c5233f 100644 (file)
@@ -292,11 +292,11 @@ void Storage::remove(const Key& key)
     });
 }
 
-void Storage::updateFileAccessTime(IOChannel& channel)
+void Storage::updateFileModificationTime(IOChannel& channel)
 {
     StringCapture filePathCapture(channel.path());
     serialBackgroundIOQueue().dispatch([filePathCapture] {
-        updateFileAccessTimeIfNeeded(filePathCapture.string());
+        updateFileModificationTimeIfNeeded(filePathCapture.string());
     });
 }
 
@@ -316,7 +316,7 @@ void Storage::dispatchReadOperation(const ReadOperation& read)
                 auto record = decodeRecord(fileData, channel->fileDescriptor(), read.key);
                 bool success = read.completionHandler(WTF::move(record));
                 if (success)
-                    updateFileAccessTime(*channel);
+                    updateFileModificationTime(*channel);
                 else
                     remove(read.key);
             }
@@ -589,7 +589,8 @@ static double deletionProbability(FileTimes times)
 
     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.
     if (age <= seconds::zero() || accessAge < seconds::zero() || accessAge > age)
index 8636002..216c6ce 100644 (file)
@@ -97,7 +97,7 @@ private:
     void dispatchHeaderWriteOperation(const WriteOperation&);
     void dispatchPendingWriteOperations();
 
-    void updateFileAccessTime(IOChannel&);
+    void updateFileModificationTime(IOChannel&);
 
     WorkQueue& ioQueue() { return m_ioQueue.get(); }
     WorkQueue& backgroundIOQueue() { return m_backgroundIOQueue.get(); }