Description
Simon Fraser (smfr)
2018-07-08 22:34:29 PDT
Created attachment 346147 [details]
Patch
Comment on attachment 346147 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=346147&action=review > Source/WebCore/ChangeLog:3 > + ResourceResponseBase wastes a lot of space because of std::optional<> This looks good, but I think Simon's intention was more general than just removing one optional member. So I would attach this patch to a separate bug (maybe a dependency or see also one) so that we keep this bug open. I understand the bug but thought maybe I could get away with it ;) New bug opened at https://bugs.webkit.org/show_bug.cgi?id=188192. Comment on attachment 346147 [details] Patch Move this patch to https://bugs.webkit.org/show_bug.cgi?id=188192. https://github.com/akrzemi1/markable 'markable' is a single header optional-like template library with no space overhead. (In reply to Fujii Hironori from comment #5) > https://github.com/akrzemi1/markable > 'markable' is a single header optional-like template library with no space > overhead. Yeah but it just uses magic values. Created attachment 372680 [details]
Patch
Comment on attachment 372680 [details] Patch Attachment 372680 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/12550088 New failing tests: imported/w3c/web-platform-tests/2dcontext/imagebitmap/createImageBitmap-drawImage.html media/sources-fallback-codecs.html imported/w3c/web-platform-tests/2dcontext/imagebitmap/createImageBitmap-invalid-args.html imported/w3c/web-platform-tests/2dcontext/imagebitmap/createImageBitmap-transfer.html media/audio-data-url.html inspector/console/webcore-logging.html Created attachment 372681 [details]
Archive of layout-test-results from ews101 for mac-highsierra
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101 Port: mac-highsierra Platform: Mac OS X 10.13.6
Comment on attachment 372680 [details] Patch Attachment 372680 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/12550099 New failing tests: imported/w3c/web-platform-tests/2dcontext/imagebitmap/createImageBitmap-drawImage.html media/sources-fallback-codecs.html imported/w3c/web-platform-tests/2dcontext/imagebitmap/createImageBitmap-invalid-args.html imported/w3c/web-platform-tests/2dcontext/imagebitmap/createImageBitmap-transfer.html media/audio-data-url.html inspector/console/webcore-logging.html Created attachment 372682 [details]
Archive of layout-test-results from ews104 for mac-highsierra-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104 Port: mac-highsierra-wk2 Platform: Mac OS X 10.13.6
Comment on attachment 372680 [details] Patch Attachment 372680 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/12550122 New failing tests: imported/w3c/web-platform-tests/2dcontext/imagebitmap/createImageBitmap-drawImage.html media/sources-fallback-codecs.html imported/w3c/web-platform-tests/2dcontext/imagebitmap/createImageBitmap-invalid-args.html imported/w3c/web-platform-tests/2dcontext/imagebitmap/createImageBitmap-transfer.html media/audio-data-url.html inspector/console/webcore-logging.html Created attachment 372683 [details]
Archive of layout-test-results from ews112 for mac-highsierra
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112 Port: mac-highsierra Platform: Mac OS X 10.13.6
Created attachment 372684 [details]
Patch
Comment on attachment 372684 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=372684&action=review > Source/WebCore/platform/network/ParsedContentRange.h:52 > + int64_t m_lastBytePosition { -1 }; Why not use Markable<>? Created attachment 372698 [details]
Patch
Comment on attachment 372698 [details] Patch Attachment 372698 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/12555765 New failing tests: fast/files/apply-blob-url-to-xhr-using-open-panel.html http/tests/appcache/fallback-namespace-outside-manifest-path.html fast/files/apply-blob-url-to-xhr.html http/tests/local/fileapi/send-sliced-dragged-file.html http/tests/appcache/abort-cache-ondownloading-manifest-404.html http/tests/appcache/remove-cache.html http/tests/appcache/404-manifest.html http/tests/appcache/deferred-events.html Created attachment 372702 [details]
Archive of layout-test-results from ews103 for mac-highsierra
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103 Port: mac-highsierra Platform: Mac OS X 10.13.6
Comment on attachment 372698 [details] Patch Attachment 372698 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/12555773 New failing tests: http/tests/appcache/abort-cache-ondownloading-manifest-404.html http/wpt/fetch/inspect-preflight.html http/tests/security/bypassing-cors-checks-for-extension-urls.html http/tests/appcache/fallback-namespace-outside-manifest-path.html http/tests/appcache/remove-cache.html http/tests/appcache/404-manifest.html http/tests/appcache/deferred-events.html http/tests/xmlhttprequest/redirect-cross-origin-sync-double.html Created attachment 372703 [details]
Archive of layout-test-results from ews106 for mac-highsierra-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106 Port: mac-highsierra-wk2 Platform: Mac OS X 10.13.6
Comment on attachment 372698 [details] Patch Attachment 372698 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/12555808 New failing tests: imported/w3c/web-platform-tests/fetch/api/basic/integrity.sub.any.html imported/w3c/web-platform-tests/fetch/api/redirect/redirect-mode.any.html imported/w3c/web-platform-tests/cors/origin.htm imported/w3c/web-platform-tests/resource-timing/resource_TAO_match_origin.htm imported/w3c/web-platform-tests/resource-timing/resource_TAO_match_wildcard.htm imported/w3c/web-platform-tests/cors/allow-headers.htm imported/w3c/web-platform-tests/fetch/api/redirect/redirect-location.any.html imported/w3c/web-platform-tests/resource-timing/resource_TAO_wildcard.htm imported/w3c/web-platform-tests/cors/response-headers.htm imported/w3c/web-platform-tests/fetch/api/cors/cors-basic.any.html http/tests/appcache/abort-cache-ondownloading-manifest-404.html imported/w3c/web-platform-tests/resource-timing/resource_TAO_null.htm http/tests/fetch/caching-with-different-options.html imported/w3c/web-platform-tests/resource-timing/resource_TAO_multi.htm imported/w3c/web-platform-tests/fetch/api/basic/mode-no-cors.sub.any.html imported/w3c/web-platform-tests/resource-timing/resource_TAO_zero.htm imported/w3c/web-platform-tests/resource-timing/resource_TAO_origin.htm http/tests/appcache/deferred-events.html imported/w3c/web-platform-tests/resource-timing/resource_TAO_origin_uppercase.htm imported/w3c/web-platform-tests/resource-timing/resource_TAO_space.htm http/tests/xmlhttprequest/access-control-and-redirects.html http/wpt/fetch/response-opaque-clone.html http/tests/xmlhttprequest/simple-cross-origin-denied-events-sync.html http/tests/appcache/remove-cache.html http/tests/appcache/404-manifest.html Created attachment 372704 [details]
Archive of layout-test-results from ews115 for mac-highsierra
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115 Port: mac-highsierra Platform: Mac OS X 10.13.6
Comment on attachment 372698 [details] Patch Attachment 372698 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/12555876 New failing tests: http/tests/xmlhttprequest/redirect-cross-origin-post-sync.html http/tests/xmlhttprequest/XMLHttpRequestException.html http/tests/xmlhttprequest/xmlhttprequest-unsafe-redirect.html http/tests/xmlhttprequest/connection-error-sync.html http/tests/xmlhttprequest/redirect-cross-origin-tripmine.html http/tests/xmlhttprequest/redirect-cross-origin-sync.html http/tests/xmlhttprequest/access-control-and-redirects.html fast/xmlhttprequest/xmlhttprequest-nonexistent-file.html http/tests/fetch/caching-with-different-options.html http/tests/xmlhttprequest/onloadend-event-after-sync-requests.html Created attachment 372706 [details]
Archive of layout-test-results from ews210 for win-future
The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews210 Port: win-future Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit
Comment on attachment 372698 [details] Patch Attachment 372698 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/12555827 New failing tests: http/tests/xmlhttprequest/cross-site-denied-response-sync-2.html http/tests/xmlhttprequest/cross-site-denied-response-sync.html http/tests/appcache/fallback-namespace-outside-manifest-path.html http/wpt/fetch/inspect-preflight.html http/tests/xmlhttprequest/simple-cross-origin-denied-events-post-sync.html http/tests/xmlhttprequest/simple-cross-origin-denied-events-sync.html http/tests/appcache/abort-cache-ondownloading-manifest-404.html http/tests/xmlhttprequest/redirect-cross-origin-sync.html http/tests/appcache/remove-cache.html http/tests/appcache/deferred-events.html http/tests/xmlhttprequest/origin-whitelisting-removal.html http/tests/xmlhttprequest/redirect-cross-origin-sync-double.html Created attachment 372707 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125 Port: ios-simulator-wk2 Platform: Mac OS X 10.14.5
Created attachment 372708 [details]
Patch
Comment on attachment 372708 [details] Patch Attachment 372708 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/12557113 New failing tests: fast/files/apply-blob-url-to-xhr.html http/tests/appcache/fallback-namespace-outside-manifest-path.html http/tests/local/fileapi/send-sliced-dragged-file.html Created attachment 372712 [details]
Archive of layout-test-results from ews103 for mac-highsierra
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103 Port: mac-highsierra Platform: Mac OS X 10.13.6
Comment on attachment 372708 [details] Patch Attachment 372708 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/12557123 New failing tests: http/tests/appcache/fallback-namespace-outside-manifest-path.html http/tests/xmlhttprequest/XMLHttpRequestException.html Created attachment 372713 [details]
Archive of layout-test-results from ews107 for mac-highsierra-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107 Port: mac-highsierra-wk2 Platform: Mac OS X 10.13.6
Comment on attachment 372708 [details] Patch Attachment 372708 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/12557143 New failing tests: http/wpt/fetch/response-opaque-clone.html imported/w3c/web-platform-tests/fetch/api/basic/integrity.sub.any.html imported/w3c/web-platform-tests/fetch/api/cors/cors-basic.any.html imported/w3c/web-platform-tests/fetch/api/redirect/redirect-mode.any.html http/tests/fetch/caching-with-different-options.html imported/w3c/web-platform-tests/fetch/api/basic/mode-no-cors.sub.any.html imported/w3c/web-platform-tests/fetch/api/redirect/redirect-location.any.html Created attachment 372716 [details]
Archive of layout-test-results from ews115 for mac-highsierra
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115 Port: mac-highsierra Platform: Mac OS X 10.13.6
Created attachment 372738 [details]
Pach
Created attachment 372783 [details]
Patch
Comment on attachment 372783 [details] Patch Attachment 372783 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/12564559 New failing tests: http/tests/appcache/abort-cache-ondownloading-manifest-404.html http/tests/appcache/404-manifest.html http/tests/appcache/remove-cache.html http/tests/appcache/deferred-events.html Created attachment 372788 [details]
Archive of layout-test-results from ews101 for mac-highsierra
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101 Port: mac-highsierra Platform: Mac OS X 10.13.6
Comment on attachment 372783 [details] Patch Attachment 372783 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/12564606 New failing tests: http/wpt/fetch/inspect-preflight.html http/tests/appcache/abort-cache-ondownloading-manifest-404.html http/tests/appcache/404-manifest.html http/tests/appcache/remove-cache.html http/tests/appcache/deferred-events.html Created attachment 372789 [details]
Archive of layout-test-results from ews104 for mac-highsierra-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104 Port: mac-highsierra-wk2 Platform: Mac OS X 10.13.6
Comment on attachment 372783 [details] Patch Attachment 372783 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/12564719 New failing tests: http/tests/appcache/abort-cache-ondownloading-manifest-404.html http/tests/appcache/404-manifest.html http/tests/appcache/remove-cache.html http/tests/appcache/deferred-events.html Created attachment 372799 [details]
Archive of layout-test-results from ews112 for mac-highsierra
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112 Port: mac-highsierra Platform: Mac OS X 10.13.6
Comment on attachment 372783 [details] Patch Attachment 372783 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/12564751 New failing tests: http/wpt/fetch/inspect-preflight.html http/tests/appcache/abort-cache-ondownloading-manifest-404.html http/tests/appcache/remove-cache.html http/tests/appcache/deferred-events.html Created attachment 372802 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126 Port: ios-simulator-wk2 Platform: Mac OS X 10.14.5
Created attachment 373173 [details]
Patch
Comment on attachment 373173 [details] Patch Attachment 373173 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/12613424 New failing tests: http/tests/inspector/network/getSerializedCertificate.html http/tests/inspector/network/resource-security-certificate.html http/tests/inspector/network/resource-sizes-disk-cache.html http/tests/cache/link-prefetch-main-resource.html http/tests/inspector/network/resource-response-source-disk-cache.html Created attachment 373175 [details]
Archive of layout-test-results from ews104 for mac-highsierra-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104 Port: mac-highsierra-wk2 Platform: Mac OS X 10.13.6
Created attachment 373176 [details]
Patch
Created attachment 373194 [details]
Patch
Patches seems absolutely in the wrong direction we want in this project in my opinion. In my opinion the project wants to use enum classes (not enum) and in my opinion we don't want enumerators like SourceNetwork <--concatenating the concept and value. In my opinion I would figure out a way to save the space without undoing everything. Failing this I would demand that I have a damn good reason 😀 for all this terribleness, stats or something concrete and all. Comment on attachment 373194 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=373194&action=review > Source/WebCore/platform/network/ResourceResponseBase.h:234 > +protected: > + int m_httpStatusCode { 0 }; > + > +private: > + mutable bool m_includesCertificateInfo : 1; This is a really shitty way of packing stuff. Move m_httpStatusCode past all these boolean bitfields, and they would pack a lot better. Comment on attachment 373194 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=373194&action=review > Source/WebCore/platform/network/ResourceResponseBase.h:70 > + unsigned type : 3; > + unsigned tainting : 2; > bool isRedirected; Type and Training are each one byte if we use uint8_t width so making them bitfields isn't gonna save anything. We're better off moving httpStatusCode down here so that it's better packed with these enum classes & bool. > Source/WebCore/platform/network/ResourceResponseBase.h:248 > + unsigned m_source : 3; > + unsigned m_type : 3; > + unsigned m_tainting : 2; Again, we should just use enum class with uint8_t width. There are 3 uint8_t enum class types here (3 bytes) then if you move m_httpStatusCode here, that would neatly pack into one word (8 bytes) in 64-bit. (In reply to Daniel Bates from comment #49) > Patches seems absolutely in the wrong direction we want in this project in > my opinion. In my opinion the project wants to use enum classes (not enum) > and in my opinion we don't want enumerators like SourceNetwork > <--concatenating the concept and value. Yeah, it's not like we can't cast unsigned to enum class either. Packing objects better matter in many cases but here, we don't need to resort to bitfields at all since three uint8_t + one uint32_t would pack neatly into a single uint64_t space. I think this is fixed by bug 207618 *** This bug has been marked as a duplicate of bug 207618 *** |