[webkit-reviews] review granted: [Bug 45555] Speed up deserialisation of strings : [Attachment 67220] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Oliver Hunt <oliver at apple.com>'s
request for review:
Bug 45555: Speed up deserialisation of strings
https://bugs.webkit.org/show_bug.cgi?id=45555

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

------- Additional Comments from Darin Adler <darin at apple.com>
> +	   UString m_ident;
> +	   JSValue m_string;

These are confusing names. The string is named "ident" (unnecessary
abbreviation), and the JSValue is named string! How about naming one thing
m_string and the other m_stringAsJSValue or m_stringWrappedInJSValue or
something like that.

Also, I think the function name jsString is not so great, but I don’t have a
better suggestion. Maybe something with the word "wrap" in it.

> -    bool readStringData(Identifier& ident)
> +    bool readStringData(CachedString*& ident)

You should name this string, not ident.

> -    bool readStringData(Identifier& ident, bool& wasTerminator)
> +    bool readStringData(CachedString*& ident, bool& wasTerminator)

And this.

> -	       Identifier ident;
> +	       CachedString* ident;

And this.