Bug 21203 – Optimize appending a number to a string

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Maciej Stachowiak]

Optimize appending a number to a string. It's pretty common in real-world code (and on some of the v8 benchmarks) to do this.

[Maciej Stachowiak]

Created attachment 23900 [details]
patch to do it. Some sad code duplication here.

[Maciej Stachowiak]

I sadly duplicated the logic for UString::from(int) and UString::from(double)
though fortunately not the actual append logic. Perhaps I should try harder to
refactor to make it ot duplicat, though it was not 100% clear how to do this
except maybe using a template. UString::from(int) is not much code but the
double case is nontrivial.

[Darin Adler]

Comment on attachment 23900 [details]
patch to do it. Some sad code duplication here.

+    if ((leftIsString = v1->isString()) && v2->isString()) {

Why not just put the leftIsString boolean initialization separate in front of the if statement in a more conventional way?

+        RefPtr<UString::Rep> value;
+        if (JSImmediate::isImmediate(v2))
+            value = concatenate(static_cast<JSString*>(v1)->value().rep(), JSImmediate::getTruncatedInt32(v2));
+        else
+            value = concatenate(static_cast<JSString*>(v1)->value().rep(), right);

I believe would be more efficient if done with the ternary operator instead of an if statement. I believe it would save a null check and a branch over deref code on the assignment to the value local.

+inline size_t expandedSize(size_t size, size_t otherSize)

If this isn't going to be a static member function, then I suggest internal linkage -- add the static keyword.

+inline bool expandCapacity(UString::Rep* rep, int requiredLength)

This should also be marked static to get internal linkage.

r=me

[Maciej Stachowiak]

Landed as r37089