[webkit-reviews] review granted: [Bug 83243] Speed up the conversion from JSValue to String for bulk operations : [Attachment 135748] Patch

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has granted Benjamin Poulain
<benjamin at webkit.org>'s request for review:
Bug 83243: Speed up the conversion from JSValue to String for bulk operations
https://bugs.webkit.org/show_bug.cgi?id=83243

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

------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=135748&action=review


r=me

> Source/JavaScriptCore/ChangeLog:9
> +	   When making operations on primitive types, we loose some time
converting
> +	   values to JSString in order to extract the string.

It looks like JSValue::toString only artificially creates a JSString if the
JSValue is a number. I wonder if NumericStrings should just change to cache
JSStrings instead of UStrings, and how much of this issue that would resolve.

> Source/JavaScriptCore/ChangeLog:11
> +	   This patch speed up some basic Array operations by avoiding the
creation

Grammar: speeds.

> Source/JavaScriptCore/ChangeLog:15
> +	   an inline convertion is used.

Spelling: conversion.

> Source/JavaScriptCore/runtime/JSString.h:472
> +    ALWAYS_INLINE UString inlineJSValueNotStringtoUString(const JSValue&
value, ExecState* exec)

Is there really a measurable advantage to inlining this? How do I know whether
I should call toUString or toUStringInline?