Bug 225063

Summary: ResourceLoadStatisticsDatabaseStore::aggregatedThirdPartyData() hangs for a long time
Product: WebKit Reporter: Kate Cheney <katherine_cheney>
Component: New BugsAssignee: Kate Cheney <katherine_cheney>
Status: RESOLVED FIXED    
Severity: Normal CC: beidson, bfulgham, sihui_liu, webkit-bug-importer, wilander
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Kate Cheney 2021-04-26 11:31:10 PDT
ResourceLoadStatisticsDatabaseStore::aggregatedThirdPartyData() hangs for a long time
Comment 1 Kate Cheney 2021-04-26 11:31:38 PDT
<rdar://problem/76248557>
Comment 2 Kate Cheney 2021-04-26 12:19:05 PDT
Created attachment 427076 [details]
Patch
Comment 3 Brent Fulgham 2021-04-26 13:26:05 PDT
Comment on attachment 427076 [details]
Patch

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

Do we need to check in all three of these files? I thought that the *.db-shm and *.db-wal were dynamic byproducts of loading the sqlite database?

> Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsDatabaseStore.cpp:305
> +static const MemoryCompactLookupOnlyRobinHoodHashMap<String, std::pair<String, Optional<String>>>& createTableAndIndexQueries()

Some of this would be easier to read with a type declaration:

typedef std::pair<String, Optional<String>> TableAndIndexPair

.. or something similar.

> Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsDatabaseStore.cpp:437
> +std::pair<String, Optional<String>> ResourceLoadStatisticsDatabaseStore::tableSchemaAndIndex(const String& tableName)

I wonder if this should be called "tableAndIndexSchema"?
Comment 4 Brent Fulgham 2021-04-26 13:27:03 PDT
Comment on attachment 427076 [details]
Patch

r=me, but please confirm we need to check in all three files. I also think that creating a typedef for the table and index schema would make the code easier to read and understand.
Comment 5 Kate Cheney 2021-04-26 13:33:46 PDT
Comment on attachment 427076 [details]
Patch

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

Hmm, good point about the 3 files. Let me run the test without them to be sure, but seems likely they could be removed. Thanks for the comments!

>> Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsDatabaseStore.cpp:305
>> +static const MemoryCompactLookupOnlyRobinHoodHashMap<String, std::pair<String, Optional<String>>>& createTableAndIndexQueries()
> 
> Some of this would be easier to read with a type declaration:
> 
> typedef std::pair<String, Optional<String>> TableAndIndexPair
> 
> .. or something similar.

Good call, will change.

>> Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsDatabaseStore.cpp:437
>> +std::pair<String, Optional<String>> ResourceLoadStatisticsDatabaseStore::tableSchemaAndIndex(const String& tableName)
> 
> I wonder if this should be called "tableAndIndexSchema"?

Yes, the name is a bit awkward. Maybe even "tableAndIndexQueries" would be better, because that's really what the data is. We are using the queries to imply the schema.
Comment 6 Kate Cheney 2021-04-26 15:35:17 PDT
Created attachment 427098 [details]
Patch
Comment 7 EWS 2021-04-26 16:43:03 PDT
Committed r276620 (237049@main): <https://commits.webkit.org/237049@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 427098 [details].
Comment 8 Radar WebKit Bug Importer 2021-04-26 16:44:29 PDT
<rdar://problem/77182578>