Deploy ASCIILiteral hotness throughout WebCore
Created attachment 161132 [details] Patch
Comment on attachment 161132 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=161132&action=review > Source/WebCore/css/CSSPropertySourceData.cpp:91 > + DEFINE_STATIC_LOCAL(String, emptyValue, (ASCIILiteral("e"))); Maw? > Source/WebCore/dom/MicroDataItemList.cpp:45 > DEFINE_STATIC_LOCAL(String, undefinedItemTypeString, ("")); > + // FIXME: Why not just return emptyString(); ? > return undefinedItemTypeString; Someone hit me with a spoon. > Source/WebCore/editing/MarkupAccumulator.cpp:238 > - DEFINE_STATIC_LOCAL(String, xmlnsWithColon, ("xmlns:")); > + DEFINE_STATIC_LOCAL(String, xmlnsWithColon, (ASCIILiteral("xmlns:"))); This seems odd. > Source/WebCore/inspector/InspectorStyleSheet.cpp:638 > + DEFINE_STATIC_LOCAL(String, defaultPrefix, (ASCIILiteral(" "))); !?! > Source/WebCore/loader/MainResourceLoader.cpp:374 > + DEFINE_STATIC_LOCAL(String, consoleMessage, (ASCIILiteral("Refused to display document because display forbidden by X-Frame-Options.\n"))); Why the \n? > Source/WebCore/page/Page.cpp:395 > DEFINE_STATIC_LOCAL(String, nullString, ()); > + // FIXME: Why not just return String()? Oh please. > Source/WebCore/platform/blackberry/RenderThemeBlackBerry.cpp:126 > - DEFINE_STATIC_LOCAL(String, fontFace, ("Arial")); > + DEFINE_STATIC_LOCAL(String, fontFace, (ASCIILiteral("Arial"))); WTF? > Source/WebCore/xml/parser/XMLTokenizer.cpp:485 > - DEFINE_STATIC_LOCAL(String, xmlString, ("xml")); > + DEFINE_STATIC_LOCAL(String, xmlString, (ASCIILiteral("xml"))); XMLNames bro. > Source/WebCore/xml/parser/XMLTreeBuilder.cpp:341 > + DEFINE_STATIC_LOCAL(String, ampS, (ASCIILiteral("&"))); > + DEFINE_STATIC_LOCAL(String, aposS, (ASCIILiteral("'"))); > + DEFINE_STATIC_LOCAL(String, gtS, (ASCIILiteral(">"))); > + DEFINE_STATIC_LOCAL(String, ltS, (ASCIILiteral("<"))); > + DEFINE_STATIC_LOCAL(String, quotS, (ASCIILiteral("\""))); We have the Entity technology to help you. :)
Comment on attachment 161132 [details] Patch I'll fix some of these in a followup patch. I'm not that inclined to improve the NEWXML at the moment.
Adam, thank you for deploying this widely, it is gonna help me a lot to track our memory. All those static strings make me wonder if that is causing some of the memory behaviors we have. Once the StringImpl is created in a page, we can no longer give that page back to the system. :( Maybe we should have some dense memory area just for things like these. It's something we should measure... > > Source/WebCore/dom/MicroDataItemList.cpp:45 > > DEFINE_STATIC_LOCAL(String, undefinedItemTypeString, ("")); > > + // FIXME: Why not just return emptyString(); ? > > return undefinedItemTypeString; > > Someone hit me with a spoon. Brilliant :-D
> All those static strings make me wonder if that is causing some of the memory behaviors we have. Once the StringImpl is created in a page, we can no longer give that page back to the system. :( > > Maybe we should have some dense memory area just for things like these. It's something we should measure... Yeah, that makes a lot of sense.
Comment on attachment 161132 [details] Patch Clearing flags on attachment: 161132 Committed r126968: <http://trac.webkit.org/changeset/126968>
All reviewed patches have been landed. Closing bug.