[Webkit-unassigned] [Bug 27762] [v8] Cache atomic strings in externalized v8 strings

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=27762





--- Comment #11 from Christian Plesner Hansen <christian.plesner.hansen at gmail.com>  2009-07-28 10:49:44 PDT ---
> There are tabs in your ChangeLog.
> 
> m_atomic_impl is not WebKit style.  m_atomicString would be, or m_atomicImpl
> (although we've historically tried to kill Impl, excepting a few places)
> 
> +        if (m_atomic_impl.isNull()) {
> +            m_atomic_impl = AtomicString(m_impl);
> +        }
> 
> is not WK style.
> 
> WK uses 4 space indent:
> +    if (!v8String->MakeExternal(resource)) {
> +      // In case of a failure delete the external resource as it was not used.
> +      delete resource;
> +    }
> 
> I think you should consider running WebKit's copy of cpplint, it's found in
> WebKitTools/Scripts/

Sorry, it's been like two years since I last worked on WK code.  I'll lint and
clean up the code.

> JSC does this by passing UStrings directly to an AtomicString constructor
> (they're implicitly constructed when getElementById is called).
> 
> PassRefPtr<StringImpl> AtomicString::add(const JSC::UString& ustring)
> 
> is where the real magic is.  (Just hashes the string and makes a StringImpl
> wrapper if necessary.)

It's the 'add' call I'm trying to get rid of; currently around 10% of
getElementById is spent just looking up atomic strings and that seems to hold
for most operations that use them.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.