timestamp opt (20-25% speed increase) #5
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
oh - also added CLI usage |
|
👍 but we should make this configurable via a constructor option. |
|
My contention is we should provide the most optimal approach to logging We're positioning pino as the fastest logger, so do we really want to add an option that slows it down? I'm thinking we should encourage post processing |
|
ok after discussion we decided to go with an option with an ominous name "slowtime", a warning in the readme, and the ability to transform json logs with the pino command line tool using the |
davidmarkclements
force-pushed
the
timestamp
branch
from
6074426
to
f6038fd
Compare
mcollina
added a commit
that referenced
this pull request
Mar 9, 2016
timestamp opt (20-25% speed increase)
|
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. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When we profile an isolated benchmark with http://npm.im/0x (again) we can see that toISOString is hot (10.8% on stack 8.72% on top of stack)
If we alter out timestamps from ISO to epoch, we get the following flamegraph showing that
Date.now()is far less expensive thatDate.prototype.toISOStringRemoving the bottleneck gives us a 20-25% perf increase:
benchPino_10000: 369.998ms
benchPino2_10000: 278.406ms
benchPinoObj_10000: 431.747ms
benchPinoObj2_10000: 347.675ms
benchPino_10000: 344.995ms
benchPino2_10000: 272.239ms
benchPinoObj_10000: 420.123ms
benchPinoObj2_10000: 340.240ms
We can keep consistent log output formatting via the
pinocommand, by applying the toISOString call at the point of transformation.