That should save some initialization time and some memory.
Created attachment 156550 [details] Patch
Comment on attachment 156550 [details] Patch Attachment 156550 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/13436356
Comment on attachment 156550 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=156550&action=review r- for build failure. Needs an export, I think: 4>JavaScriptCore.exp : error LNK2001: unresolved external symbol "public: static class WTF::PassRefPtr<class WTF::StringImpl> __cdecl JSC::Identifier::add(class JSC::ExecState *,char const *)" (?add@Identifier@JSC@@SA?AV?$PassRefPtr@VStringImpl@WTF@@@WTF@@PAVExecState@2@PBD@Z) Is this measurably faster on some benchmark? > Source/JavaScriptCore/runtime/Identifier.cpp:153 > + const LiteralIdentifierTable::iterator& iter = literalIdentifierTable.find(c); > + if (iter != literalIdentifierTable.end()) > + return iter->second; > + > + CharBuffer<LChar> charBuffer = { reinterpret_cast<const LChar*>(c), length }; > + HashSet<StringImpl*>::AddResult addResult = identifierTable.add<CharBuffer<LChar>, IdentifierASCIICharBufferTranslator>(charBuffer); I wonder if there's a way to avoid computing the string hash twice here.
> r- for build failure. Needs an export, I think: Yep, damn Windows :( > Is this measurably faster on some benchmark? The change is mostly for memory usage, I have only measured it does not regress. If the QualifiedName change is any indication, this is also likely faster at initialization. I doubt that shows up in any benchmark, this likely only affects startup time. I'll see if I can DTrace on ARM to get some numbers for CPU time. > I wonder if there's a way to avoid computing the string hash twice here. Actually, they are not the same hash. The literalIdentifierTable hashes the pointer to the string, the identifierTable hashes the string itself.
Created attachment 156844 [details] Patch
> Is this measurably faster on some benchmark? It is actually also measurable in time. On http://pmav.eu/stuff/javascript-webworkers/ , if you put enough workers, you get a small improvement (~0.9% on the smallest workset). I gave up on the version that hardcodes the string length. The problem is that was growing the binary since the size is hardcoded in the text segment. Growing the binary size to improve launch time is about as bad as it can get, so I settled for this simpler version.
Comment on attachment 156844 [details] Patch r=me
Comment on attachment 156844 [details] Patch Clearing flags on attachment: 156844 Committed r124922: <http://trac.webkit.org/changeset/124922>
All reviewed patches have been landed. Closing bug.
Comment on attachment 156844 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=156844&action=review > Source/JavaScriptCore/runtime/CommonIdentifiers.cpp:30 > + : nullIdentifier() Maybe we can remove this initializer too.