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

Stabilize Tick Rate in sslclient #536

Merged
merged 1 commit into from
Oct 30, 2022

Conversation

Anotra
Copy link
Contributor

@Anotra Anotra commented Oct 30, 2022

ssl client has

1:  if (last_tick != time(nullptr)) {
2:    this->one_second_timer();
3:    last_tick = time(nullptr);
4:  }

The problem is, is that the time could change between the condition on line 1, and setting it on line 3. This can't be changed, as it would cause bugs in other places. The poll syscall further down can be reworked to wake up at the beginning of the second to make it less likely for the time difference on lines 1, and 3 to occur. Other parts of the code rely on specific times to operate, and they may be skipped over regardless, this just makes it far less likely to occur.

I'm aware, that sslclient is running on its own thread, and there may be many running, it might be best to stagger the wake up at a random interval (beginning of second + rand(0 to 100) milliseconds).

@netlify
Copy link

netlify bot commented Oct 30, 2022

Deploy Preview for dpp-dev ready!

Name Link
🔨 Latest commit b86963a
🔍 Latest deploy log https://app.netlify.com/sites/dpp-dev/deploys/635e5c34f0f1680008ca1c08
😎 Deploy Preview https://deploy-preview-536--dpp-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@braindigitalis
Copy link
Contributor

thanks for the pr, it appears to not compile on all systems due to a missing include. can you fix this please?
one question though, how much more does the poll loop run than before? if it means that lots more cpu is eaten to ensure we don't skip a second this could cause issues at scale.

@Anotra
Copy link
Contributor Author

Anotra commented Oct 30, 2022

how much more does the poll loop run than before?
A decent amount more, yea. I've fixed that with this new change.
It will now poll 1000ms if in the lower 600ms of the second, otherwise it will poll (time_left + time_left / 3 + 1). That means if it's 700ms into the curent second, (300 + 300 / 3 + 1) and will wake up 401ms later and being 101ms into the next second. It also doesn't wake up that much more than the original version now.

@braindigitalis braindigitalis merged commit e4f0ff5 into brainboxdotcc:dev Oct 30, 2022
@braindigitalis
Copy link
Contributor

thanks, ive merged this. if you're on the discord, let me know and i'll give you a contributor role.

@braindigitalis braindigitalis added hacktoberfest-accepted Issues tagged for hacktoberfest bug Something isn't working labels Oct 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working hacktoberfest-accepted Issues tagged for hacktoberfest
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants