Clean up Blob code
Created attachment 270725 [details] Patch
Created attachment 270736 [details] Patch
Created attachment 270737 [details] Patch
Comment on attachment 270737 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=270737&action=review > Source/WebCore/fileapi/Blob.h:79 > +#ifndef NDEBUG If this is for assertions only, then it should be using ASSERT_DISABLED macro (name from memory). > Source/WebCore/platform/network/BlobRegistry.h:69 > + virtual ~BlobRegistry() { }; This is not an improvement - constructors and destructors of polymorphic classes should not be inline, because of code size bloat. > Source/WebCore/platform/network/BlobRegistryImpl.cpp:78 > + return nullptr; Does return value optimization work in functions with multiple exit points? > Source/WebCore/platform/network/BlobRegistryImpl.h:70 > + HashMap<URL, RefPtr<BlobData>> m_blobs; Why is this better? URL is a large object. Do we waste time reparsing them later?
Comment on attachment 270737 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=270737&action=review >> Source/WebCore/platform/network/BlobRegistryImpl.cpp:78 >> + return nullptr; > > Does return value optimization work in functions with multiple exit points? Yes. > Source/WebCore/platform/network/BlobRegistryImpl.cpp:149 > - if (!m_blobs.contains(part.url().string())) > + if (!m_blobs.contains(part.url())) > return; > - for (const BlobDataItem& item : m_blobs.get(part.url().string())->items()) > + for (const BlobDataItem& item : m_blobs.get(part.url())->items()) I am going to make this only lookup the url once in the HashMap, though. >> Source/WebCore/platform/network/BlobRegistryImpl.h:70 >> + HashMap<URL, RefPtr<BlobData>> m_blobs; > > Why is this better? URL is a large object. Do we waste time reparsing them later? I thought it was more clear, and we don't reparse them, but you're right: this is a memory regression.
http://trac.webkit.org/changeset/196174