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

Slow because data is fetched one byte at a time #11

Open
tarui opened this issue May 17, 2022 · 7 comments
Open

Slow because data is fetched one byte at a time #11

tarui opened this issue May 17, 2022 · 7 comments

Comments

@tarui
Copy link

tarui commented May 17, 2022

The following section, reading data one byte at a time, is very slow when trying to handle MB-order packets.

unless recv_data = @socket.getc
sleep 1
next
end

Simply replace with the following to speed up the process. I didn't make a pull request because I wasn't sure what to specify as a buffer size, but I hope you will consider replacing it.

recv_data = @socket.readpartial(4096*16)
@unasuke
Copy link
Collaborator

unasuke commented May 18, 2022

@tarui Sorry, late. That's makes sense. Could you provide script to send-and-read MB order packets data with this gem?

@tarui
Copy link
Author

tarui commented May 18, 2022

@tarui Sorry, late. That's makes sense. Could you provide script to send-and-read MB order packets data with this gem?

Not late at all!
I use to talk to Chrome with the Chrome DevTools Protocol.
Sorry for toy script( and you need run chrome with --remote-debugging-port=9222 option)
Can you try it?
https://gist.github.com/tarui/e0ce48a87f884e7642cef836e5fac395

@unasuke
Copy link
Collaborator

unasuke commented May 18, 2022

I tried it and confirmed. Increasing buffer size is reasonable. Thank you report!

@unasuke
Copy link
Collaborator

unasuke commented May 23, 2022

I investigated it. I think using Socket#readpartial instead of Socket#getc is bring incompatible because getc is text-reading method but readpartial is binary-reading method.

@tarui
Copy link
Author

tarui commented May 24, 2022

Thank you for your researching.

So that means we need to handle the encoding correctly. And that seems a bit cumbersome considering the buffer runs out in the middle of a character.

@tarui
Copy link
Author

tarui commented May 24, 2022

I checked RFC6455.

Certainly there could be discrepancies at that stage, but in the first place, since it is a binary-packed packet at this layer, isn't this String treated as binary at the latter stage?
Text being UTF-8 is a specification( https://datatracker.ietf.org/doc/html/rfc6455#section-5.6),
so I don't think the Encoding here has any impact.
(or websocket gem has a bug related to encoding?)

@unasuke
Copy link
Collaborator

unasuke commented May 25, 2022

I see. It changes the behavior, but using readpartial may not cause any major problems.

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 a pull request may close this issue.

2 participants