Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

util: Speed up the common case of formatting a single string. #1749

Closed
wants to merge 1 commit into from

Conversation

ChALkeR
Copy link
Member

@ChALkeR ChALkeR commented May 20, 2015

This slightly speeds up a common case of formatting a single string.
Used, for example, by console.log("string").

@mscdex mscdex added the util Issues and PRs related to the built-in util module. label May 20, 2015
@@ -13,6 +13,8 @@ exports.format = function(f) {
return objects.join(' ');
}

if (arguments.length === 1) return f;

var i = 1;
var args = arguments;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would probably also help to not leak the arguments here.

Maybe consider https://gist.github.com/Fishrock123/98c35a0c745cb59d7496

@Fishrock123
Copy link
Contributor

👍

@bnoordhuis
Copy link
Member

LGTM

@ChALkeR
Copy link
Member Author

ChALkeR commented May 20, 2015

for (var i = 0; i < 10000000; i++) {
    util.format('abcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabc1234567890');
}

Before: 4.3 seconds, 2323414 ops/sec.
After: 0.63 seconds, 15927170 ops/sec.

@ChALkeR
Copy link
Member Author

ChALkeR commented May 20, 2015

With cloning arguments — 0.29 seconds, 34269599 ops/sec.

@ChALkeR
Copy link
Member Author

ChALkeR commented May 20, 2015

Should I add the arguments cloning to this PR or make a separate one?
I do not want to add arguments cloning to this PR to keep it simple. Let's make a separate one regarding arguments.

@Fishrock123
Copy link
Contributor

I think this isn't quite right, you still need it to run though https://github.com/nodejs/io.js/blob/master/lib/util.js#L36-L42 afaik.

Nope, I'm wrong. LGTM

@ChALkeR
Copy link
Member Author

ChALkeR commented May 20, 2015

For

for (var i = 0; i < 1000000; i++) {
    console.log('abcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabc1234567890');
}

There is about 35% speedup.

bnoordhuis pushed a commit that referenced this pull request May 20, 2015
PR-URL: #1749
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
@bnoordhuis
Copy link
Member

Landed in 214d020.

@bnoordhuis bnoordhuis closed this May 20, 2015
@ChALkeR ChALkeR deleted the util-format-singlestring branch May 20, 2015 21:39
Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request May 21, 2015
PR-URL: nodejs#1749
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
@rvagg rvagg mentioned this pull request May 23, 2015
rvagg added a commit to rvagg/io.js that referenced this pull request May 23, 2015
PR-URL: nodejs#1532

Notable Changes:

* crypto: Diffie-Hellman key exchange (DHE) parameters ('dhparams') must now be
  1024 bits or longer or an error will be thrown. A warning will also be printed
  to the console if you supply less than 2048 bits. See https://weakdh.org/ for
  further context on this security concern. (Shigeki Ohtsu) nodejs#1739.
* node: A new --trace-sync-io command line flag will print a warning and a stack
  trace whenever a synchronous API is used. This can be used to track down
  synchronous calls that may be slowing down an application.
  (Trevor Norris) nodejs#1707.
* node: To allow for chaining of methods, the setTimeout(), setKeepAlive(),
  setNoDelay(), ref() and unref() methods used in 'net', 'dgram', 'http',
  'https' and 'tls' now return the current instance instead of undefined
  (Roman Reiss) nodejs#1779.
* util: A significant speed-up (in the order of 35%) for the common-case of a
  single string argument to util.format(), used by console.log()
  (Сковорода Никита Андреевич) nodejs#1749.
rvagg added a commit to rvagg/io.js that referenced this pull request May 23, 2015
PR-URL: nodejs#1532

Notable Changes:

* crypto: Diffie-Hellman key exchange (DHE) parameters ('dhparams') must now be
  1024 bits or longer or an error will be thrown. A warning will also be printed
  to the console if you supply less than 2048 bits. See https://weakdh.org/ for
  further context on this security concern. (Shigeki Ohtsu) nodejs#1739.
* node: A new --trace-sync-io command line flag will print a warning and a stack
  trace whenever a synchronous API is used. This can be used to track down
  synchronous calls that may be slowing down an application.
  (Trevor Norris) nodejs#1707.
* node: To allow for chaining of methods, the setTimeout(), setKeepAlive(),
  setNoDelay(), ref() and unref() methods used in 'net', 'dgram', 'http',
  'https' and 'tls' now return the current instance instead of undefined
  (Roman Reiss & Evan Lucas) nodejs#1699 nodejs#1768 nodejs#1779.
* util: A significant speed-up (in the order of 35%) for the common-case of a
  single string argument to util.format(), used by console.log()
  (Сковорода Никита Андреевич) nodejs#1749.
rvagg added a commit to rvagg/io.js that referenced this pull request May 23, 2015
PR-URL: nodejs#1532

Notable Changes:

* crypto: Diffie-Hellman key exchange (DHE) parameters ('dhparams') must now be
  1024 bits or longer or an error will be thrown. A warning will also be printed
  to the console if you supply less than 2048 bits. See https://weakdh.org/ for
  further context on this security concern. (Shigeki Ohtsu) nodejs#1739.
* node: A new --trace-sync-io command line flag will print a warning and a stack
  trace whenever a synchronous API is used. This can be used to track down
  synchronous calls that may be slowing down an application.
  (Trevor Norris) nodejs#1707.
* node: To allow for chaining of methods, the setTimeout(), setKeepAlive(),
  setNoDelay(), ref() and unref() methods used in 'net', 'dgram', 'http',
  'https' and 'tls' now return the current instance instead of undefined
  (Roman Reiss & Evan Lucas) nodejs#1699 nodejs#1768 nodejs#1779.
* util: A significant speed-up (in the order of 35%) for the common-case of a
  single string argument to util.format(), used by console.log()
  (Сковорода Никита Андреевич) nodejs#1749.
rvagg added a commit to rvagg/io.js that referenced this pull request May 24, 2015
PR-URL: nodejs#1532

Notable Changes:

* crypto: Diffie-Hellman key exchange (DHE) parameters ('dhparams') must now be
  1024 bits or longer or an error will be thrown. A warning will also be printed
  to the console if you supply less than 2048 bits. See https://weakdh.org/ for
  further context on this security concern. (Shigeki Ohtsu) nodejs#1739.
* node: A new --trace-sync-io command line flag will print a warning and a stack
  trace whenever a synchronous API is used. This can be used to track down
  synchronous calls that may be slowing down an application.
  (Trevor Norris) nodejs#1707.
* node: To allow for chaining of methods, the setTimeout(), setKeepAlive(),
  setNoDelay(), ref() and unref() methods used in 'net', 'dgram', 'http',
  'https' and 'tls' now return the current instance instead of undefined
  (Roman Reiss & Evan Lucas) nodejs#1699 nodejs#1768 nodejs#1779.
* npm: Upgraded to v2.10.1, release notes can be found in
  https://github.com/npm/npm/releases/tag/v2.10.1 and
  https://github.com/npm/npm/releases/tag/v2.10.0.
* util: A significant speed-up (in the order of 35%) for the common-case of a
  single string argument to util.format(), used by console.log()
  (Сковорода Никита Андреевич) nodejs#1749.
rvagg added a commit that referenced this pull request May 24, 2015
PR-URL: #1777

Notable Changes:

* crypto: Diffie-Hellman key exchange (DHE) parameters ('dhparams') must now be
  1024 bits or longer or an error will be thrown. A warning will also be printed
  to the console if you supply less than 2048 bits. See https://weakdh.org/ for
  further context on this security concern. (Shigeki Ohtsu) #1739.
* node: A new --trace-sync-io command line flag will print a warning and a stack
  trace whenever a synchronous API is used. This can be used to track down
  synchronous calls that may be slowing down an application.
  (Trevor Norris) #1707.
* node: To allow for chaining of methods, the setTimeout(), setKeepAlive(),
  setNoDelay(), ref() and unref() methods used in 'net', 'dgram', 'http',
  'https' and 'tls' now return the current instance instead of undefined
  (Roman Reiss & Evan Lucas) #1699 #1768 #1779.
* npm: Upgraded to v2.10.1, release notes can be found in
  https://github.com/npm/npm/releases/tag/v2.10.1 and
  https://github.com/npm/npm/releases/tag/v2.10.0.
* util: A significant speed-up (in the order of 35%) for the common-case of a
  single string argument to util.format(), used by console.log()
  (Сковорода Никита Андреевич) #1749.
andrewdeandrade pushed a commit to andrewdeandrade/node that referenced this pull request Jun 3, 2015
PR-URL: nodejs/node#1749
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
@ChALkeR ChALkeR added the performance Issues and PRs related to the performance of Node.js. label Feb 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Issues and PRs related to the performance of Node.js. util Issues and PRs related to the built-in util module.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants