[webkit-reviews] review granted: [Bug 15837] AddNode causes excessive mallocs due to add() using toString(exec) : [Attachment 17062] another fix (0.8% speed speedup according to SunSpider)

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Eric Seidel <eric at webkit.org>'s
request for review:
Bug 15837: AddNode causes excessive mallocs due to add() using toString(exec)
http://bugs.webkit.org/show_bug.cgi?id=15837

Attachment 17062: another fix (0.8% speed speedup according to SunSpider)
http://bugs.webkit.org/attachment.cgi?id=17062&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
+static inline JSValue* throwOutOfMemoryError(ExecState* exec)

I don't think this should be inline.

     if (p1->isString() || p2->isString()) {
	 UString value = p1->toString(exec) + p2->toString(exec);

Can't those use value() too? I know this is the slow case, but it would be nice
to code it tighter.

+	 if (value.isNull())
+	     return throwOutOfMemoryError(exec);
+	 else
	     return jsString(value);

A nice opportunity to get rid of an else after return.

+    if (t1 == NumberType && t2 == NumberType)
+	 return jsNumber(v1->toNumber(exec) + v2->toNumber(exec));
+    else if (t1 == StringType && t2 == StringType) {

No else after return.

+    } else if (t1 == StringType) { // common js idium "" + object

Misspelling -- it's idiom.

review+, assuming you'll make throwOutOfMemoryError not be inline.