RESOLVED INVALID 62527
[EFL][WK2] Change string operation to makeString()
https://bugs.webkit.org/show_bug.cgi?id=62527
Summary [EFL][WK2] Change string operation to makeString()
Eunmi Lee
Reported 2011-06-12 21:12:22 PDT
I've changed string operation codes to use makeString().
Attachments
Patch (1.83 KB, patch)
2011-06-12 21:25 PDT, Eunmi Lee
no flags
Patch (1.94 KB, patch)
2011-06-14 06:40 PDT, Eunmi Lee
no flags
Patch (1.94 KB, patch)
2011-06-14 18:01 PDT, Eunmi Lee
no flags
Eunmi Lee
Comment 1 2011-06-12 21:25:52 PDT
Gyuyoung Kim
Comment 2 2011-06-12 21:39:14 PDT
LGTM.
Raphael Kubo da Costa (:rakuco)
Comment 3 2011-06-13 06:10:42 PDT
Informal r+ from my side.
Kenneth Rohde Christiansen
Comment 4 2011-06-13 07:22:11 PDT
Comment on attachment 96918 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=96918&action=review > Source/WebKit2/ChangeLog:6 > + [EFL][WK2] Change string operation to makeString() > + https://bugs.webkit.org/show_bug.cgi?id=62527 Please add one line explaining why, like why that makes sense, why that is better etc? like 'use makeStrings() instead of creating individual strings and concatenating them'
Eunmi Lee
Comment 5 2011-06-14 06:40:37 PDT
Eunmi Lee
Comment 6 2011-06-14 06:41:37 PDT
(In reply to comment #4) > (From update of attachment 96918 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=96918&action=review > > > Source/WebKit2/ChangeLog:6 > > + [EFL][WK2] Change string operation to makeString() > > + https://bugs.webkit.org/show_bug.cgi?id=62527 > > Please add one line explaining why, like why that makes sense, why that is better etc? > > like 'use makeStrings() instead of creating individual strings and concatenating them' Done. I've added one line for explaining.
Leandro Pereira
Comment 7 2011-06-14 08:49:04 PDT
Comment on attachment 97109 [details] Patch Patch looks OK; changelog could use more work.
Eric Seidel (no email)
Comment 8 2011-06-14 08:50:13 PDT
Comment on attachment 97109 [details] Patch Looks OK to me.
Gyuyoung Kim
Comment 9 2011-06-14 17:09:05 PDT
Comment on attachment 97109 [details] Patch Informal r+
Eunmi Lee
Comment 10 2011-06-14 18:01:49 PDT
Eunmi Lee
Comment 11 2011-06-14 18:02:46 PDT
(In reply to comment #10) > Created an attachment (id=97207) [details] > Patch modify "use" to "Use"
Gyuyoung Kim
Comment 12 2011-06-14 18:09:03 PDT
Comment on attachment 97207 [details] Patch Internal r+
WebKit Review Bot
Comment 13 2011-06-14 23:13:17 PDT
Comment on attachment 97207 [details] Patch Clearing flags on attachment: 97207 Committed r88907: <http://trac.webkit.org/changeset/88907>
WebKit Review Bot
Comment 14 2011-06-14 23:13:22 PDT
All reviewed patches have been landed. Closing bug.
Nikolas Zimmermann
Comment 15 2011-06-15 07:25:39 PDT
This patch is wrong. makeString is deprecated, using String operator+ gives exactly the same performance. This has been changed recently. use "osVersion = name.sysname + " " + name.machine;" You are not required to cast to Strings first. The StringAppend template solution will reduce this String concatting operation to just a single malloc of the final size, no appending is done at all.
Leandro Pereira
Comment 16 2011-06-15 09:20:09 PDT
(In reply to comment #15) > This patch is wrong. makeString is deprecated, using String operator+ gives > exactly the same performance. This has been changed recently. > > use "osVersion = name.sysname + " " + name.machine;" > You are not required to cast to Strings first. > > The StringAppend template solution will reduce this String concatting operation > to just a single malloc of the final size, no appending is done at all. You're right. I've rolled this out as r88944. Changing this bug status to 'invalid'.
Note You need to log in before you can comment on or make changes to this bug.