Bug 131923 – Speed up jsStringWithCache() through WeakGCMap inlining.

• Status: RESOLVED
• Resolution: FIXED
• Component: Bindings

[Andreas Kling]

Speed up jsStringWithCache() through WeakGCMap inlining.

[Andreas Kling]

Created attachment 229780 [details]
Patch

[Darin Adler]

Comment on attachment 229780 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=229780&action=review

> Source/WebCore/bindings/js/JSDOMBinding.cpp:66
> +JSC::JSValue jsStringWithCache(JSC::ExecState* exec, const String& s)

rename s to string?

> Source/WebCore/bindings/js/JSDOMBinding.cpp:81
> +    JSStringCache& stringCache = currentWorld(exec).m_stringCache;
> +    JSStringCache::AddResult addResult = stringCache.add(stringImpl, nullptr);

I suggest using auto. I also suggest getting rid of the stringCache local variable.

> Source/WebCore/bindings/js/JSDOMBinding.cpp:83
> +        addResult.iterator->value = JSC::jsString(exec, String(stringImpl));

Seems a shame to do String(stringImpl) here instead of just using "s"

[Andreas Kling]

(In reply to comment #2)
> (From update of attachment 229780 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=229780&action=review
> 
> > Source/WebCore/bindings/js/JSDOMBinding.cpp:66
> > +JSC::JSValue jsStringWithCache(JSC::ExecState* exec, const String& s)
> 
> rename s to string?
> 
> > Source/WebCore/bindings/js/JSDOMBinding.cpp:81
> > +    JSStringCache& stringCache = currentWorld(exec).m_stringCache;
> > +    JSStringCache::AddResult addResult = stringCache.add(stringImpl, nullptr);
> 
> I suggest using auto. I also suggest getting rid of the stringCache local variable.
> 
> > Source/WebCore/bindings/js/JSDOMBinding.cpp:83
> > +        addResult.iterator->value = JSC::jsString(exec, String(stringImpl));
> 
> Seems a shame to do String(stringImpl) here instead of just using "s"

I'll get rid of the ref count churn here shortly. Will pretty up the names then too.

[WebKit Commit Bot]

Comment on attachment 229780 [details]
Patch

Clearing flags on attachment: 229780

Committed r167577: <http://trac.webkit.org/changeset/167577>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.