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

check timeout immediately to avoid unnecessary wait until next timer … #1

Closed
wants to merge 1 commit into from

Conversation

traderbagel
Copy link

check timeout immediately after the increments of idle time of the connection, then it could avoid unnecessary wait, which had already timeout, until next timer tick

@boazsegev
Copy link
Owner

boazsegev commented Apr 19, 2016

Thank you @chuchuhao for your commit and support.

However, I have concerns about this optimization.

I am concerned about fractions of a second and raising a timeout event too early. If a connection is accepted in the middle of a second, that fraction of a second is ignored, which could (potentially) cause a timeout of 1 second to be raised after a few milliseconds.

Here is a practical example:

  1. Connection is accepted at 00:00:01.75 with timeout set to 1 second - this is possible, as connections can always be accepted.
  2. Timeout is reviewed at 00:00:01.80 - this is possible because the timeout review is sporadic (no more then once a second, but depending on the server's stress and polling cycle, it could be any time within that second).
  3. The review sets the timeout to 1.

If there review is optimized, the connection is disconnected due to the timeout review after only 5 milliseconds.

If the review isn't optimized, it will take two more timeout review cycles before the timeout is processed, so it will take (at least) 1.2 seconds before the timeout is processed.

This is why I use the > instead of the >= operator - I want the extra cycles to prevent a premature timeout event.

@boazsegev
Copy link
Owner

@chuchuhao, I wanted to thank you again before I close this pull request.

I'm thankful for your attempt to collaborate and improve this library.

Please feel free to suggest any further improvements or reopen this pull request if you feel I made a mistake.

Kind regards,
Boaz.

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

2 participants