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

Write to stdout ASAP in server mode #10814

Merged
merged 1 commit into from Jul 18, 2022
Merged

Conversation

ccutrer
Copy link
Contributor

@ccutrer ccutrer commented Jul 13, 2022

Don't wait for a full 4KB of output, or a timeout, just write it as
we get it. This makes the experience feel much more like you're running
rubocop directly.

@bbatsov
Copy link
Collaborator

bbatsov commented Jul 14, 2022

I'll defer to @koic here, as I'm not sure why he went for the output buffering in the first place.

@@ -0,0 +1 @@
* Avoid buffering stdout when running in server mode. ([@ccutrer][])
Copy link
Member

Choose a reason for hiding this comment

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

Can you add this PR address?

Suggested change
* Avoid buffering stdout when running in server mode. ([@ccutrer][])
* [#10814](https://github.com/rubocop/rubocop/pull/10814): Avoid buffering stdout when running in server mode. ([@ccutrer][])

@koic
Copy link
Member

koic commented Jul 14, 2022

Yeah, sokect.read(4096) is still the logic imported from rubocop-daemon. Let's take this change.

@koic
Copy link
Member

koic commented Jul 14, 2022

@ccutrer If possible, can you provide a benchmark comparison before (socket.read(4096)) and after (socket.readpartial(4096))?

Don't wait for a full 4KB of output, or a timeout, just write it as
we get it. This makes the experience feel much more like you're running
rubocop directly
@ccutrer
Copy link
Contributor Author

ccutrer commented Jul 14, 2022

@ccutrer If possible, can you provide a benchmark comparison before (socket.read(4096)) and after (socket.readpartial(4096))?

Environment: ruby 3.0.2p107 (2021-07-07 revision 0db68f0233) [x86_64-linux-gnu], Intel Core i5 8259U @ 2.3 GHz (quad core, hyperthreaded), 16GB RAM, NVMe SSD (it's an Intel NUC). This is running Rubocop against itself.

Command Socket Min Max Avg
bundle exec rubocop --no-server N/A 1.196s 1.470s 1.227s
bundle exec rubocop --server socket.read(4096) 1.046s 1.154s 1.092s
bundle exec rubocop --server socket.readpartial(4096) 0.995s 1.160s 1.083s
bundle exec rubocop --server -C false socket.read(4096) 1m32.900s 1m35.282s 1m33.769s
bundle exec rubocop --server -C false socket.readpartial(4096) 1m32.784s 1m35.236 1m34.223s

In other words, no discernible time difference.

When running --server -C false with socket.read(4096), the output is blank for quite a while, then it outputs a large chunk at once, silent for a while, etc. in 4 distinct batches. When running --server -C false with socket.readpartial(4096), the Inspecting 1401 files comes immediately, and then per-file dots are discrete events, continuously coming.

I've updated ed the changelog entry.

@koic koic merged commit 992019e into rubocop:master Jul 18, 2022
@koic
Copy link
Member

koic commented Jul 18, 2022

Thanks!

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.

None yet

3 participants