Bug 222658

Summary: Limit HashTable entry size to 500 bytes
Product: WebKit Reporter: Alex Christensen <achristensen>
Component: New BugsAssignee: Alex Christensen <achristensen>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, cdumez, cgarcia, cmarcelo, ews-watchlist, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=222696
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Alex Christensen
Reported 2021-03-03 09:01:21 PST
Limit HashTable entry size to 500 bytes
Attachments
Patch (10.75 KB, patch)
2021-03-03 09:05 PST, Alex Christensen 		no flags
DetailsFormatted DiffDiff
Patch (11.22 KB, patch)
2021-03-03 10:56 PST, Alex Christensen 		no flags
DetailsFormatted DiffDiff
Patch (11.13 KB, patch)
2021-03-03 19:54 PST, Alex Christensen 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Alex Christensen
Comment 1 2021-03-03 09:05:04 PST
Created attachment 422097 [details] Patch
Alex Christensen
Comment 2 2021-03-03 10:56:35 PST
Created attachment 422121 [details] Patch
Yusuke Suzuki
Comment 3 2021-03-03 11:40:31 PST
Comment on attachment 422121 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=422121&action=review r=me > Source/WebCore/ChangeLog:14 > + Moving large structures inside HashTables is slower than just moving a pointer. > + There is a point at which it becomes more efficient to add a separate allocation > + rather than have gigantic HashTables that use all that memory for each empty slot > + and use all those read and write operations to move when rehashing. I'm not sure > + exactly where that point is, but I'm pretty sure it's less than 500 bytes. This > + introduces a limit and removes the two largest HashTables in WebKit, replacing their > + values with std::unique_ptr to save memory and time. Totally right. Furthermore, HashTable allocates large spaces to keep O(1) access. If entry size is large, we waste much more memory than using a pointer.
EWS
Comment 4 2021-03-03 12:54:32 PST
Committed r273832: <https://commits.webkit.org/r273832> All reviewed patches have been landed. Closing bug and clearing flags on attachment 422121 [details].
Radar WebKit Bug Importer
Comment 5 2021-03-03 12:55:35 PST
<rdar://problem/74999075>
Ryan Haddad
Comment 6 2021-03-03 16:49:41 PST
Reverted r273832 for reason: 26 inspector tests failing a new assert added with this change Committed r273859 (234825@main): <https://commits.webkit.org/234825@main>
Ryan Haddad
Comment 7 2021-03-03 16:51:23 PST
(In reply to Ryan Haddad from comment #6) > Reverted r273832 for reason: > > 26 inspector tests failing a new assert added with this change Details in https://bugs.webkit.org/show_bug.cgi?id=222696 SHOULD NEVER BE REACHED /Volumes/Data/worker/bigsur-debug/build/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.h(145) : void WebKit::NetworkConnectionToWebProcess::takeNetworkLoadInformationMetrics(WebKit::ResourceLoadIdentifier, CompletionHandler<void (const WebCore::NetworkLoadMetrics &)> &&)
Alex Christensen
Comment 8 2021-03-03 19:54:27 PST
Created attachment 422177 [details] Patch
Yusuke Suzuki
Comment 9 2021-03-03 20:27:36 PST
Comment on attachment 422177 [details] Patch r=me.
EWS
Comment 10 2021-03-03 21:00:15 PST
Committed r273872: <https://commits.webkit.org/r273872> All reviewed patches have been landed. Closing bug and clearing flags on attachment 422177 [details].
Note You need to log in before you can comment on or make changes to this bug.