Bug 204027 - Add size file for CacheStorage
Summary: Add size file for CacheStorage
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sihui Liu
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-11-08 15:34 PST by Sihui Liu
Modified: 2019-11-12 17:24 PST (History)
7 users (show)

See Also:


Attachments
Patch (8.04 KB, patch)
2019-11-08 15:52 PST, Sihui Liu
no flags Details | Formatted Diff | Diff
Patch (7.71 KB, patch)
2019-11-11 16:19 PST, Sihui Liu
no flags Details | Formatted Diff | Diff
Patch for landing (7.67 KB, patch)
2019-11-11 17:05 PST, Sihui Liu
no flags Details | Formatted Diff | Diff
Patch (9.70 KB, patch)
2019-11-12 13:39 PST, Sihui Liu
no flags Details | Formatted Diff | Diff
Patch for landing (9.49 KB, patch)
2019-11-12 15:58 PST, Sihui Liu
no flags Details | Formatted Diff | Diff
Patch for landing (9.26 KB, patch)
2019-11-12 16:03 PST, Sihui Liu
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sihui Liu 2019-11-08 15:34:18 PST
... so we can look at the size file and get the size of record without waiting for Caches to initialize.
Comment 1 Sihui Liu 2019-11-08 15:52:39 PST
Created attachment 383174 [details]
Patch
Comment 2 youenn fablet 2019-11-08 17:08:36 PST
Comment on attachment 383174 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=383174&action=review

> Source/WebKit/ChangeLog:8
> +        No behavior change as the file is not in use now.

I do not think we should do that in NetworkCacheStorage.
Instead, we could do it in CacheStorage::Caches::writeRecord/removeRecord/writeCachesToDisk.
Comment 3 Sihui Liu 2019-11-11 16:19:08 PST
Created attachment 383313 [details]
Patch
Comment 4 youenn fablet 2019-11-11 16:55:10 PST
Comment on attachment 383313 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=383313&action=review

> Source/WebKit/NetworkProcess/cache/CacheStorageEngine.cpp:470
> +    m_ioQueue->dispatch([path = path.isolatedCopy(), size]() mutable {

mutable?

> Source/WebKit/NetworkProcess/cache/CacheStorageEngine.cpp:481
> +        auto sizeString = String::number(size).utf8();

Could be a oneliner.

> Source/WebKit/NetworkProcess/cache/CacheStorageEngine.cpp:512
> +    return String::fromUTF8(buffer.data()).toUIntStrict();

toUInt64Strict

> Source/WebKit/NetworkProcess/cache/CacheStorageEngine.h:88
> +    static Optional<uint64_t> readSizeFile(const String& path);

'path' not really useful.
Comment 5 Sihui Liu 2019-11-11 17:05:21 PST
Created attachment 383319 [details]
Patch for landing
Comment 6 WebKit Commit Bot 2019-11-11 17:49:33 PST
Comment on attachment 383319 [details]
Patch for landing

Clearing flags on attachment: 383319

Committed r252351: <https://trac.webkit.org/changeset/252351>
Comment 7 WebKit Commit Bot 2019-11-11 17:49:35 PST
All reviewed patches have been landed.  Closing bug.
Comment 8 Radar WebKit Bug Importer 2019-11-11 17:50:16 PST
<rdar://problem/57100861>
Comment 9 Truitt Savell 2019-11-12 11:05:51 PST
It looks like the added assertion in https://trac.webkit.org/changeset/252351/webkit

is causing testing to exit early on Mac and iOS wk2 debug

Build:
https://build.webkit.org/builders/Apple%20Mojave%20Debug%20WK2%20%28Tests%29/builds/5776

crash:
Thread 6 Crashed:: Dispatch queue: com.apple.WebKit.CacheStorageEngine.serialBackground
0   com.apple.JavaScriptCore      	0x000000012d434ad0 WTFCrash + 16 (Assertions.cpp:305)
1   com.apple.JavaScriptCore      	0x000000012e94dbfb WTFCrashWithInfo(int, char const*, char const*, int) + 27
2   com.apple.JavaScriptCore      	0x000000012d472958 WTF::FileSystemImpl::unlockAndCloseFile(int) + 104 (FileSystem.cpp:365)
3   com.apple.WebKit              	0x000000010de4fc6a WebKit::CacheStorage::Engine::writeSizeFile(WTF::String const&, unsigned long long)::$_48::operator()() const::'lambda'()::operator()() const + 26 (CacheStorageEngine.cpp:474)
Comment 10 Truitt Savell 2019-11-12 11:07:45 PST
Reverted r252351 for reason:

casued 50+ crashes on Mac and iOS wk2 debug

Committed r252369: <https://trac.webkit.org/changeset/252369>
Comment 11 Sihui Liu 2019-11-12 13:39:11 PST
Created attachment 383375 [details]
Patch
Comment 12 Sihui Liu 2019-11-12 15:58:00 PST
Created attachment 383396 [details]
Patch for landing
Comment 13 Sihui Liu 2019-11-12 16:03:00 PST
Created attachment 383397 [details]
Patch for landing
Comment 14 WebKit Commit Bot 2019-11-12 17:24:34 PST
Comment on attachment 383397 [details]
Patch for landing

Clearing flags on attachment: 383397

Committed r252381: <https://trac.webkit.org/changeset/252381>
Comment 15 WebKit Commit Bot 2019-11-12 17:24:36 PST
All reviewed patches have been landed.  Closing bug.