Bug 62527

Summary:

[EFL][WK2] Change string operation to makeString()

Product:

WebKit

Reporter:

Eunmi Lee <enmi.lee>

Component:

WebKit2

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED INVALID

Severity:

Normal

CC:

antognolli+webkit, gyuyoung.kim, kenneth, leandro, lucas.de.marchi, rakuco, ryuan.choi, sangseok.lim, tonikitoo, webkit.review.bot, youngtaeck.song, zimmermann

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

62732

Bug Blocks:

61838

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none

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	Details Formatted Diff Diff
Patch (1.94 KB, patch) 2011-06-14 06:40 PDT, Eunmi Lee	no flags	Details Formatted Diff Diff
Patch (1.94 KB, patch) 2011-06-14 18:01 PDT, Eunmi Lee	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Eunmi Lee

Comment 1 2011-06-12 21:25:52 PDT

Created attachment 96918 [details] Patch

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

Created attachment 97109 [details] Patch

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

Created attachment 97207 [details] Patch

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.