215012 2020-07-30 22:40:58 -0700 Optimize WebsiteDataStoreConfiguration::copy 2020-07-31 10:27:44 -0700 1 1 1 Unclassified WebKit New Bugs WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 achristensen achristensen beidson ggaren webkit-bug-importer oldest_to_newest 1676728 0 achristensen 2020-07-30 22:40:58 -0700 Optimize WebsiteDataStoreConfiguration::copy 1676731 1 achristensen 2020-07-30 22:41:21 -0700 *** Bug 215011 has been marked as a duplicate of this bug. *** 1676733 2 405670 achristensen 2020-07-30 22:43:38 -0700 Created attachment 405670 Patch 1676734 3 achristensen 2020-07-30 22:43:41 -0700 <rdar://problem/64263406> 1676743 4 ews-feeder 2020-07-30 23:16:38 -0700 Committed r265134: <https://trac.webkit.org/changeset/265134> All reviewed patches have been landed. Closing bug and clearing flags on attachment 405670. 1676757 5 beidson 2020-07-31 00:17:28 -0700 This actually wasn't quite enough! Need to apply the flag to the WebsiteDataStore constructor as well: WebsiteDataStore::WebsiteDataStore(Ref<WebsiteDataStoreConfiguration>&& configuration, PAL::SessionID sessionID) : m_sessionID(sessionID) , m_resolvedConfiguration(WTFMove(configuration)) , m_configuration((m_resolvedConfiguration)->copy()) <-------Here I'll followup in the morning 1676825 6 achristensen 2020-07-31 08:24:37 -0700 That calls WebsiteDataStoreConfiguration::copy() which benefits from this optimization. 1676836 7 405670 ggaren 2020-07-31 09:02:33 -0700 Comment on attachment 405670 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=405670&action=review > Source/WebKit/UIProcess/WebsiteData/WebsiteDataStoreConfiguration.h:36 > +enum class WillCopyPathsFromExistingConfiguration : bool { No, Yes }; What's important here is not necessarily that the paths came from some other configuration, but rather that the paths are already fully resolved -- right? If so, perhaps a better enum would be PathsAreFullyResolved { No, Yes }. Global knowledge of the code and logical deduction can tell us that a path from an existing configuration is a fully resolved path, but it is best if reading code doesn't require global knowledge or logical deduction. 1676838 8 beidson 2020-07-31 09:08:08 -0700 (In reply to Alex Christensen from comment #6) > That calls WebsiteDataStoreConfiguration::copy() which benefits from this > optimization. Yup I totally misread it late at night, we're good. 1676863 9 achristensen 2020-07-31 10:27:44 -0700 What's really important here is that we don't do disk read and write operations during WebsiteDataStoreConfiguration::copy. This was one solution, but I think a better one would be do std::call_once in each of the WebsiteDataStore::default*Directory in WebsiteDataStoreCocoa.mm and remove this strange looking code from WebsiteDataStoreConfiguration. That's not urgent, though, because this bug is fixed and we don't touch WebsiteDataStoreConfiguration much. 405670 2020-07-30 22:43:38 -0700 2020-07-30 23:16:39 -0700 Patch bug-215012-20200730224337.patch text/plain 4069 achristensen SW5kZXg6IFNvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJL aXQvQ2hhbmdlTG9nCShyZXZpc2lvbiAyNjUxMzEpCisrKyBTb3VyY2UvV2ViS2l0L0NoYW5nZUxv Zwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDIxIEBACisyMDIwLTA3LTMwICBBbGV4IENocmlz dGVuc2VuICA8YWNocmlzdGVuc2VuQHdlYmtpdC5vcmc+CisKKyAgICAgICAgT3B0aW1pemUgV2Vi c2l0ZURhdGFTdG9yZUNvbmZpZ3VyYXRpb246OmNvcHkKKyAgICAgICAgaHR0cHM6Ly9idWdzLndl YmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTIxNTAxMgorICAgICAgICA8cmRhcjovL3Byb2JsZW0v NjQyNjM0MDY+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAg ICAgVGhlIFdlYnNpdGVEYXRhU3RvcmVDb25maWd1cmF0aW9uIGNvbnN0cnVjdG9yIGRvZXMgc29t ZSByZWFkL3dyaXRlIGhlYXZ5IHRoaW5ncy4KKyAgICAgICAgV2Ugb25seSB3YW50IHRvIGRvIHRo ZXNlIHRoaW5ncyBpZiB3ZSBhcmUgbm90IGp1c3QgZ29pbmcgdG8gb3ZlcndyaXRlIHRoZSBzdHJp bmdzIHdpdGggc3RyaW5ncyBmcm9tIGFub3RoZXIgV2Vic2l0ZURhdGFTdG9yZUNvbmZpZ3VyYXRp b24uCisgICAgICAgIE5vIGNoYW5nZSBpbiBiZWhhdmlvciwganVzdCBtYWtpbmcgdGhpbmdzIGZh c3Rlci4KKworICAgICAgICAqIFVJUHJvY2Vzcy9XZWJzaXRlRGF0YS9XZWJzaXRlRGF0YVN0b3Jl Q29uZmlndXJhdGlvbi5jcHA6CisgICAgICAgIChXZWJLaXQ6OldlYnNpdGVEYXRhU3RvcmVDb25m aWd1cmF0aW9uOjpXZWJzaXRlRGF0YVN0b3JlQ29uZmlndXJhdGlvbik6CisgICAgICAgIChXZWJL aXQ6OldlYnNpdGVEYXRhU3RvcmVDb25maWd1cmF0aW9uOjpjb3B5IGNvbnN0KToKKyAgICAgICAg KiBVSVByb2Nlc3MvV2Vic2l0ZURhdGEvV2Vic2l0ZURhdGFTdG9yZUNvbmZpZ3VyYXRpb24uaDoK KyAgICAgICAgKFdlYktpdDo6V2Vic2l0ZURhdGFTdG9yZUNvbmZpZ3VyYXRpb246OmNyZWF0ZSk6 CisKIDIwMjAtMDctMzAgIFJ5YW4gSGFkZGFkICA8cnlhbmhhZGRhZEBhcHBsZS5jb20+CiAKICAg ICAgICAgVW5yZXZpZXdlZCBzZWNvbmQgYXR0ZW1wdCB0byBmaXggQ2F0YWx5c3QgYnVpbGQgYWZ0 ZXIgcjI2NTA4NC4KSW5kZXg6IFNvdXJjZS9XZWJLaXQvVUlQcm9jZXNzL1dlYnNpdGVEYXRhL1dl YnNpdGVEYXRhU3RvcmVDb25maWd1cmF0aW9uLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi S2l0L1VJUHJvY2Vzcy9XZWJzaXRlRGF0YS9XZWJzaXRlRGF0YVN0b3JlQ29uZmlndXJhdGlvbi5j cHAJKHJldmlzaW9uIDI2NTA4MSkKKysrIFNvdXJjZS9XZWJLaXQvVUlQcm9jZXNzL1dlYnNpdGVE YXRhL1dlYnNpdGVEYXRhU3RvcmVDb25maWd1cmF0aW9uLmNwcAkod29ya2luZyBjb3B5KQpAQCAt MzAsMTAgKzMwLDEwIEBACiAKIG5hbWVzcGFjZSBXZWJLaXQgewogCi1XZWJzaXRlRGF0YVN0b3Jl Q29uZmlndXJhdGlvbjo6V2Vic2l0ZURhdGFTdG9yZUNvbmZpZ3VyYXRpb24oSXNQZXJzaXN0ZW50 IGlzUGVyc2lzdGVudCkKK1dlYnNpdGVEYXRhU3RvcmVDb25maWd1cmF0aW9uOjpXZWJzaXRlRGF0 YVN0b3JlQ29uZmlndXJhdGlvbihJc1BlcnNpc3RlbnQgaXNQZXJzaXN0ZW50LCBXaWxsQ29weVBh dGhzRnJvbUV4aXN0aW5nQ29uZmlndXJhdGlvbiB3aWxsQ29weVBhdGhzKQogICAgIDogbV9pc1Bl cnNpc3RlbnQoaXNQZXJzaXN0ZW50KQogewotICAgIGlmIChpc1BlcnNpc3RlbnQgPT0gSXNQZXJz aXN0ZW50OjpZZXMpIHsKKyAgICBpZiAoaXNQZXJzaXN0ZW50ID09IElzUGVyc2lzdGVudDo6WWVz ICYmIHdpbGxDb3B5UGF0aHMgPT0gV2lsbENvcHlQYXRoc0Zyb21FeGlzdGluZ0NvbmZpZ3VyYXRp b246Ok5vKSB7CiAgICAgICAgIHNldEFwcGxpY2F0aW9uQ2FjaGVEaXJlY3RvcnkoV2Vic2l0ZURh dGFTdG9yZTo6ZGVmYXVsdEFwcGxpY2F0aW9uQ2FjaGVEaXJlY3RvcnkoKSk7CiAgICAgICAgIHNl dENhY2hlU3RvcmFnZURpcmVjdG9yeShXZWJzaXRlRGF0YVN0b3JlOjpkZWZhdWx0Q2FjaGVTdG9y YWdlRGlyZWN0b3J5KCkpOwogICAgICAgICBzZXROZXR3b3JrQ2FjaGVEaXJlY3RvcnkoV2Vic2l0 ZURhdGFTdG9yZTo6ZGVmYXVsdE5ldHdvcmtDYWNoZURpcmVjdG9yeSgpKTsKQEAgLTUyLDcgKzUy LDcgQEAgV2Vic2l0ZURhdGFTdG9yZUNvbmZpZ3VyYXRpb246OldlYnNpdGVEYQogCiBSZWY8V2Vi c2l0ZURhdGFTdG9yZUNvbmZpZ3VyYXRpb24+IFdlYnNpdGVEYXRhU3RvcmVDb25maWd1cmF0aW9u Ojpjb3B5KCkgY29uc3QKIHsKLSAgICBhdXRvIGNvcHkgPSBXZWJzaXRlRGF0YVN0b3JlQ29uZmln dXJhdGlvbjo6Y3JlYXRlKG1faXNQZXJzaXN0ZW50KTsKKyAgICBhdXRvIGNvcHkgPSBXZWJzaXRl RGF0YVN0b3JlQ29uZmlndXJhdGlvbjo6Y3JlYXRlKG1faXNQZXJzaXN0ZW50LCBXaWxsQ29weVBh dGhzRnJvbUV4aXN0aW5nQ29uZmlndXJhdGlvbjo6WWVzKTsKIAogICAgIGNvcHktPm1fc2Vydmlj ZVdvcmtlclByb2Nlc3NUZXJtaW5hdGlvbkRlbGF5RW5hYmxlZCA9IHRoaXMtPm1fc2VydmljZVdv cmtlclByb2Nlc3NUZXJtaW5hdGlvbkRlbGF5RW5hYmxlZDsKICAgICBjb3B5LT5tX2Zhc3RTZXJ2 ZXJUcnVzdEV2YWx1YXRpb25FbmFibGVkID0gdGhpcy0+bV9mYXN0U2VydmVyVHJ1c3RFdmFsdWF0 aW9uRW5hYmxlZDsKSW5kZXg6IFNvdXJjZS9XZWJLaXQvVUlQcm9jZXNzL1dlYnNpdGVEYXRhL1dl YnNpdGVEYXRhU3RvcmVDb25maWd1cmF0aW9uLmgKPT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYktp dC9VSVByb2Nlc3MvV2Vic2l0ZURhdGEvV2Vic2l0ZURhdGFTdG9yZUNvbmZpZ3VyYXRpb24uaAko cmV2aXNpb24gMjY1MDgxKQorKysgU291cmNlL1dlYktpdC9VSVByb2Nlc3MvV2Vic2l0ZURhdGEv V2Vic2l0ZURhdGFTdG9yZUNvbmZpZ3VyYXRpb24uaAkod29ya2luZyBjb3B5KQpAQCAtMzMsMTEg KzMzLDEyIEBACiBuYW1lc3BhY2UgV2ViS2l0IHsKIAogZW51bSBjbGFzcyBJc1BlcnNpc3RlbnQg OiBib29sIHsgTm8sIFllcyB9OworZW51bSBjbGFzcyBXaWxsQ29weVBhdGhzRnJvbUV4aXN0aW5n Q29uZmlndXJhdGlvbiA6IGJvb2wgeyBObywgWWVzIH07CiAKIGNsYXNzIFdlYnNpdGVEYXRhU3Rv cmVDb25maWd1cmF0aW9uIDogcHVibGljIEFQSTo6T2JqZWN0SW1wbDxBUEk6Ok9iamVjdDo6VHlw ZTo6V2Vic2l0ZURhdGFTdG9yZUNvbmZpZ3VyYXRpb24+IHsKIHB1YmxpYzoKLSAgICBzdGF0aWMg UmVmPFdlYnNpdGVEYXRhU3RvcmVDb25maWd1cmF0aW9uPiBjcmVhdGUoSXNQZXJzaXN0ZW50IGlz UGVyc2lzdGVudCkgeyByZXR1cm4gYWRvcHRSZWYoKm5ldyBXZWJzaXRlRGF0YVN0b3JlQ29uZmln dXJhdGlvbihpc1BlcnNpc3RlbnQpKTsgfQotICAgIFdlYnNpdGVEYXRhU3RvcmVDb25maWd1cmF0 aW9uKElzUGVyc2lzdGVudCk7CisgICAgc3RhdGljIFJlZjxXZWJzaXRlRGF0YVN0b3JlQ29uZmln dXJhdGlvbj4gY3JlYXRlKElzUGVyc2lzdGVudCBpc1BlcnNpc3RlbnQsIFdpbGxDb3B5UGF0aHNG cm9tRXhpc3RpbmdDb25maWd1cmF0aW9uIHdpbGxDb3B5UGF0aHMgPSBXaWxsQ29weVBhdGhzRnJv bUV4aXN0aW5nQ29uZmlndXJhdGlvbjo6Tm8pIHsgcmV0dXJuIGFkb3B0UmVmKCpuZXcgV2Vic2l0 ZURhdGFTdG9yZUNvbmZpZ3VyYXRpb24oaXNQZXJzaXN0ZW50LCB3aWxsQ29weVBhdGhzKSk7IH0K KyAgICBXZWJzaXRlRGF0YVN0b3JlQ29uZmlndXJhdGlvbihJc1BlcnNpc3RlbnQsIFdpbGxDb3B5 UGF0aHNGcm9tRXhpc3RpbmdDb25maWd1cmF0aW9uID0gV2lsbENvcHlQYXRoc0Zyb21FeGlzdGlu Z0NvbmZpZ3VyYXRpb246Ok5vKTsKIAogICAgIFJlZjxXZWJzaXRlRGF0YVN0b3JlQ29uZmlndXJh dGlvbj4gY29weSgpIGNvbnN0OwogCg==