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

5-10% inc #12

Merged
merged 1 commit into from
Mar 16, 2016
Merged

5-10% inc #12

merged 1 commit into from
Mar 16, 2016

Conversation

davidmarkclements
Copy link
Member

.apply cant be inlined - we're doing format.apply all the time

the thing is, we actually only ever need it if the first param is attempting interpolation,
however using regexp to test whether a string contains '%' is just as expensive as just doing the format.apply.

But - we can check if args length > 1 (instead of greater than 0), and then prioritise the 0 case by simply setting message to params[0]. This gives a 5-10% perf increase, bringing 10000 ops down to 260-265ms

benchmarks:

  function benchPino (cb) {
    for (var i = 0; i < max; i++) {
      plog.info('hello world')
    }
    setImmediate(cb)
  },
  function benchPino2 (cb) {
    for (var i = 0; i < max; i++) {
      plog2.info('hello world')
    }
    setImmediate(cb)
  },
  function benchPinoMultiArg (cb) {
    for (var i = 0; i < max; i++) {
      plog.info('hello', 'world')
    }
    setImmediate(cb)
  },
  function benchPino2MultiArg (cb) {
    for (var i = 0; i < max; i++) {
      plog2.info('hello', 'world')
    }
    setImmediate(cb)
  },

results at 10000 ops:

benchPino_10000: 319.005ms
benchPino2_10000: 268.181ms
benchPinoMultiArg_10000: 297.216ms
benchPino2MultiArg_10000: 283.411ms
benchPino_10000: 290.877ms
benchPino2_10000: 259.626ms
benchPinoMultiArg_10000: 295.353ms
benchPino2MultiArg_10000: 282.366ms

results at 50000 ops

benchPino_50000: 1334.496ms
benchPino2_50000: 1269.305ms
benchPinoMultiArg_50000: 1342.086ms
benchPino2MultiArg_50000: 1345.085ms
benchPino_50000: 1312.044ms
benchPino2_50000: 1269.279ms
benchPinoMultiArg_50000: 1343.235ms
benchPino2MultiArg_50000: 1354.008ms

full benchmark results:

benchBunyan_10000: 1075.649ms
benchWinston_10000: 1840.185ms
benchBole_10000: 1635.768ms
benchPino_10000: 265.838ms
benchBunyanObj_10000: 1250.343ms
benchWinstonObj_10000: 1828.912ms
benchPinoObj_10000: 357.569ms
benchBoleObj_10000: 1594.823ms
benchBunyan_10000: 1082.447ms
benchWinston_10000: 1717.277ms
benchBole_10000: 1591.113ms
benchPino_10000: 260.141ms
benchBunyanObj_10000: 1231.469ms
benchWinstonObj_10000: 1807.529ms
benchPinoObj_10000: 385.072ms
benchBoleObj_10000: 1542.180ms

@mcollina
Copy link
Member

Whoaaaaa :D. Magic. The weird thing is that such an if is not causing a perf issue on itself - the overhead of apply is massive then.

mcollina added a commit that referenced this pull request Mar 16, 2016
@mcollina mcollina merged commit c128d9b into master Mar 16, 2016
@mcollina mcollina deleted the avoid-format-if-poss branch March 16, 2016 23:16
@davidmarkclements
Copy link
Member Author

it might be worth creating our own formatter that takes an array

@mcollina
Copy link
Member

I agree
Il giorno gio 17 mar 2016 alle 00:28 David Mark Clements <
notifications@github.com> ha scritto:

it might be worth creating our own formatter that takes an array


You are receiving this because you modified the open/close state.

Reply to this email directly or view it on GitHub
#12 (comment)

@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants