8000 Use process.stdout.write instead of console.log and increase batch size by gdiggs · Pull Request #58 · codeclimate/codeclimate-eslint · GitHub
[go: up one dir, main page]

Skip to content

Use process.stdout.write instead of console.log and increase batch size #58

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

Merged
merged 1 commit into from
Jan 12, 2016

Conversation

gdiggs
Copy link
Contributor
@gdiggs gdiggs commented Jan 12, 2016

console.log does some extra work to construct the output, and because
it's already a string, we can save some memory by using
process.stdout.write. See this issue for more information:
nodejs/node#1741 (comment)

This is an attempt to remedy the OOM issues when rendering all the
issues. Increasing the batch size will also help by reducing the number
of GCs.

@codeclimate/review

`console.log` does some extra work to construct the output, and because
it's already a string, we can save some memory by using
`process.stdout.write`. See this issue for more information:
nodejs/node#1741 (comment)

This is an attempt to remedy the OOM issues when rendering all the
issues. Increasing the batch size will also help by reducing the number
of GCs.
@wfleming
Copy link
Contributor

I think it's hilarious our batchSize was 1 for a while. I think it used to be rather larger than 10, actually, with good results? Probably something to look at separately.

LGTM. Do you know if write also flushes the buffer or are you deciding not to force that for now?

@gdiggs
Copy link
Contributor Author
gdiggs commented Jan 12, 2016

@wfleming it used to be 100, which caused issues (I forget exactly what they were though).

@gdiggs
Copy link
Contributor Author
gdiggs commented Jan 12, 2016

In terms of flushing the buffer, I was wrong about what was going on. See the linked github issue for what is actually happening.

gdiggs added a commit that referenced this pull request Jan 12, 2016
Use process.stdout.write instead of console.log and increase batch size
@gdiggs gdiggs merged commit 219b907 into master Jan 12, 2016
@gdiggs gdiggs deleted the gd-memory branch January 12, 2016 17:10
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
0