Bug 222555

Summary: Use PriorityQueue in NetworkCache::Storage
Product: WebKit Reporter: Antti Koivisto <koivisto>
Component: Page LoadingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: beidson, cdumez, cgarcia, ews-watchlist, ggaren, keith_miller, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch
ews-feeder: commit-queue-
patch none

Antti Koivisto
Reported 2021-03-01 08:44:45 PST
Instead of Vector<Deque<>>.
Attachments
patch (6.60 KB, patch)
2021-03-01 09:05 PST, Antti Koivisto 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
patch (6.63 KB, patch)
2021-03-01 09:53 PST, Antti Koivisto 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Antti Koivisto
Comment 1 2021-03-01 09:05:27 PST
Created attachment 421832 [details] patch
Keith Miller
Comment 2 2021-03-01 09:29:55 PST
Comment on attachment 421832 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=421832&action=review Overall, looks good to me. Glad someone else is using the PriorityQueue! > Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.cpp:797 > + size_t pendingCount = m_pendingReadOperations.size(); I think you need an UNUSED_PARAM here.
Antti Koivisto
Comment 3 2021-03-01 09:53:49 PST
Created attachment 421840 [details] patch
Chris Dumez
Comment 4 2021-03-01 10:00:57 PST
Comment on attachment 421840 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=421840&action=review r=me > Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.cpp:61 > + static uint64_t ordinal; I had to look up ordinal in a dictionary but it holds up :) > Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.h:187 > + PriorityQueue<std::unique_ptr<ReadOperation>, &isHigherPriority> m_pendingReadOperations; We may be able to use UniqueRef instead of unique_ptr?
Antti Koivisto
Comment 5 2021-03-01 10:16:19 PST
> We may be able to use UniqueRef instead of unique_ptr? Probably better done in a separate cleanup that fixes other cases here too.
EWS
Comment 6 2021-03-01 11:50:29 PST
Committed r273670: <https://commits.webkit.org/r273670> All reviewed patches have been landed. Closing bug and clearing flags on attachment 421840 [details].
Radar WebKit Bug Importer
Comment 7 2021-03-01 11:51:15 PST
<rdar://problem/74882108>
Alexey Proskuryakov
Comment 8 2021-03-01 13:41:18 PST
Comment on attachment 421840 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=421840&action=review >> Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.cpp:61 >> + static uint64_t ordinal; > > I had to look up ordinal in a dictionary but it holds up :) Maybe ASSERT(isMainThread()) or whatever thread this is supposed to be on here? Seems like no reason not to.
Antti Koivisto
Comment 9 2021-03-02 07:33:43 PST
> Maybe ASSERT(isMainThread()) or whatever thread this is supposed to be on > here? Seems like no reason not to. Good idea!
Note You need to log in before you can comment on or make changes to this bug.