Use bit fields to reduce sizeof(SecurityOrigin) and sizeof(NetworkLoadMetrics)
Created attachment 431351 [details] Patch
Comment on attachment 431351 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=431351&action=review > Source/WebCore/ChangeLog:3 > + Use bit fields to reduce sizeof(SecurityOrigin) and sizeof(NetworkLoadMetrics) Why? Do we keep a lot of SecurityOrigins in memory? I did not expect us to. I don't think we should use bitfields unless there is a strong argument to reduce memory usage, given that it makes the code less readable/convenient.
<rdar://problem/79573792>
Created attachment 435928 [details] Patch
Created attachment 435959 [details] Patch including soup implementation It also fixes a couple of redundant move in return statement issues.
Created attachment 436357 [details] Patch
Created attachment 436366 [details] Patch
Created attachment 436964 [details] Patch
After r281860 this needs rebasing which will add another use of emptyMetrics, but please review.
The cURL side looks fine from our side.
Comment on attachment 436964 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=436964&action=review r=me with changes > Source/WebCore/platform/network/NetworkLoadMetrics.h:67 > + NetworkLoadMetrics(const NetworkLoadMetrics&) = default; Why do we need these? What prevents the fault ones from being generated? > Source/WebCore/platform/network/NetworkLoadMetrics.h:116 > +struct AdditionalNetworkLoadMetricsForWebInspector : public RefCounted<AdditionalNetworkLoadMetricsForWebInspector> { Given that this subclasses RefCounted, we should make the constructor private and have a public create() function. It is too error-prone otherwise. Alternatively, looking at your patch, I don't think we need to subclass RefCounted at all as it doesn't seem we ever share ownership of such objects? If so, I would just not subclass RefCounted and use makeUnique / unique_ptr.
(In reply to Chris Dumez from comment #11) > Comment on attachment 436964 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=436964&action=review > > r=me with changes > > > Source/WebCore/platform/network/NetworkLoadMetrics.h:67 > > + NetworkLoadMetrics(const NetworkLoadMetrics&) = default; > > Why do we need these? What prevents the fault ones from being generated? I tried to remove the copy constructor in an earlier version of the patch, and I think this change isn't necessary now. Will remove. > > Source/WebCore/platform/network/NetworkLoadMetrics.h:116 > > +struct AdditionalNetworkLoadMetricsForWebInspector : public RefCounted<AdditionalNetworkLoadMetricsForWebInspector> { > > Given that this subclasses RefCounted, we should make the constructor > private and have a public create() function. It is too error-prone otherwise. > > Alternatively, looking at your patch, I don't think we need to subclass > RefCounted at all as it doesn't seem we ever share ownership of such > objects? If so, I would just not subclass RefCounted and use makeUnique / > unique_ptr. The copy constructor copies a reference. It would be an unnecessary copy if it were a unique_ptr.
Created attachment 438882 [details] Patch
Committed r282853 (241984@main): <https://commits.webkit.org/241984@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 438882 [details].