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

logger performance warning #243

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

logger performance warning #243

wants to merge 2 commits into from

Conversation

missinglink
Copy link
Member

Usage of the info or debug level logging when stdout is a TTY results in a ~25% performance hit.
This PR simply adds a warning so developers are aware.

stdout is a TTY
    iteration_duration.........: avg=71.83ms min=15.43ms med=70.58ms max=135.37ms p(90)=86.29ms p(95)=92.87ms
    iterations.................: 10000  1388.112538/s

stdout is /dev/null
    iteration_duration.........: avg=55.19ms min=23.44ms med=54.81ms max=81.28ms p(90)=62.97ms p(95)=65.49ms
    iterations.................: 10000  1806.935143/s

const street = require('../api/street');
const near = require('../api/near');
const pretty = require('../lib/pretty');
const analyze = require('../lib/analyze');
Copy link
Member Author

Choose a reason for hiding this comment

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

I just cleaned these up:

  • make them const
  • remove unused require()

@orangejulius
Copy link
Member

Since this is true everwhere, should we consider putting the warning somewhere in our general documentation?

I'd rather we didn't add code to print this warning, especially in one of the (currently) least performance sensitive Pelias services.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants