-
Notifications
You must be signed in to change notification settings - Fork 246
Slightly improve initial sync hangs #1140
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
Slightly improve initial sync hangs #1140
Conversation
| local getProperty = require(Plugin.Reconciler.getProperty) | ||
|
|
||
| local function yieldIfNeeded(clock) | ||
| if os.clock() - clock > 1 / 20 then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1/20 of a second feels like an arbitrary time. task.wait() is something like 1/30th of a second, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's actually just to the next Heartbeat, which could be 1/240th of a second. You're thinking of the old wait(). 1/20 is arbitrary, but I played with a couple values and this one felt like a good balance between slowing it down and making it not freeze up.
|
See #1176 for potential fix on performance in terms of total data throughput |
The majority of the hang is actually just
HttpService:JSONDecodetaking ages on massive trees.This PR does not address that problem. It does, however, make a couple small improvements.
Demo.mp4