[webkit-reviews] review denied: [Bug 93193] Use the initialization from literal for JSC's Identifiers : [Attachment 156550] Patch

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has denied Benjamin Poulain
<benjamin at webkit.org>'s request for review:
Bug 93193: Use the initialization from literal for JSC's Identifiers
https://bugs.webkit.org/show_bug.cgi?id=93193

Attachment 156550: Patch
https://bugs.webkit.org/attachment.cgi?id=156550&action=review

------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
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 at Identifier@JSC@@SA?AV?$PassRefPtr at VStringImpl@WTF@@@WTF@@PAVExecState at 2@P
BD at 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.