WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
96030
Add StringBuilder::appendNumber() and use it
https://bugs.webkit.org/show_bug.cgi?id=96030
Summary
Add StringBuilder::appendNumber() and use it
Patrick R. Gansterer
Reported
2012-09-06 15:00:10 PDT
Add StringBuilder::appendNumber() and use it
Attachments
Patch
(20.96 KB, patch)
2012-09-06 15:07 PDT
,
Patrick R. Gansterer
no flags
Details
Formatted Diff
Diff
Patch
(23.70 KB, patch)
2012-09-09 15:06 PDT
,
Patrick R. Gansterer
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Patrick R. Gansterer
Comment 1
2012-09-06 15:07:19 PDT
Created
attachment 162597
[details]
Patch
Benjamin Poulain
Comment 2
2012-09-06 15:13:46 PDT
Nice!
Eric Seidel (no email)
Comment 3
2012-09-09 14:32:17 PDT
Comment on
attachment 162597
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=162597&action=review
Faster, or just cleaner?
> Source/JavaScriptCore/API/JSContextRef.cpp:210 > + builder.append(':');
We really need to rename this appendChar or overload appendLiteral(char) instead so it's easier to tell that it's the "fast" version.
Build Bot
Comment 4
2012-09-09 14:32:49 PDT
Comment on
attachment 162597
[details]
Patch
Attachment 162597
[details]
did not pass win-ews (win): Output:
http://queues.webkit.org/results/13796642
Patrick R. Gansterer
Comment 5
2012-09-09 14:40:54 PDT
(In reply to
comment #3
)
> (From update of
attachment 162597
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=162597&action=review
> > Faster, or just cleaner?
Since it does not create a StringImpl for every number and appends the content of a character buffer directly it is faster too.
> > Source/JavaScriptCore/API/JSContextRef.cpp:210 > > + builder.append(':'); > > We really need to rename this appendChar or overload appendLiteral(char) instead so it's easier to tell that it's the "fast" version.
I can do this in a follow up patch.
Patrick R. Gansterer
Comment 6
2012-09-09 15:06:57 PDT
Created
attachment 163017
[details]
Patch
Eric Seidel (no email)
Comment 7
2012-09-09 15:11:30 PDT
(In reply to
comment #5
)
> (In reply to
comment #3
) > > > Source/JavaScriptCore/API/JSContextRef.cpp:210 > > > + builder.append(':'); > > > > We really need to rename this appendChar or overload appendLiteral(char) instead so it's easier to tell that it's the "fast" version. > > I can do this in a follow up patch.
yes. :) I certainly wan't expecting such in this patch. THanks!
Eric Seidel (no email)
Comment 8
2012-09-09 15:11:47 PDT
Comment on
attachment 163017
[details]
Patch LGTM if the EWSes think so.
WebKit Review Bot
Comment 9
2012-09-09 21:19:12 PDT
Comment on
attachment 163017
[details]
Patch Clearing flags on attachment: 163017 Committed
r128014
: <
http://trac.webkit.org/changeset/128014
>
WebKit Review Bot
Comment 10
2012-09-09 21:19:16 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug