Don't warn when jsSchedulingOverhead is negative #5731
Conversation
|
By analyzing the blame information on this pull request, we identified @sahrens, @nicklockwood and @tadeuzagallo to be potential reviewers. |
facebook-github-bot
added
GH Review: review-needed
CLA Signed
|
We're getting more complaints over at #1598 and this error makes it difficult to debug using Chrome on a device effectively. CC'ing @nicklockwood and @tadeuzagallo again because you were active in that thread, does this PR seem like a reasonable solution? |
|
@corbt updated the pull request. |
1 similar comment
|
@corbt updated the pull request. |
|
Another approach would be to infer the clock skew and compensate accordingly. |
|
@ide this fix doesn't preclude doing something more sophisticated like that in the future, but it unblocks people who are developing with chrome on a device now. |
|
yea, not saying your solution is wrong. just throwing an idea out there. |
|
Ok cool. 👍 Since I haven't been able to get a code review for this and it's a pretty minor change from the current behavior, I'm going to go ahead and push it tomorrow unless anyone tells me that's a bad idea. |
|
@corbt updated the pull request. |
|
I'm not sure if it's a minor change. Negative skew could be an indicator of something serious... @tadeuzagallo can you take a look at this sometime? |
|
+1 |
|
Sorry for taking so long, can you simply delete the warning? It's not really doing anything here other then telling you that you passed a negative value (which works in the web btw). You could simply delete the whole if and do: - NSTimeInterval jsSchedulingOverhead = -jsSchedulingTime.timeIntervalSinceNow;
+ NSTimeInterval jsSchedulingOverhead = MAX(-jsSchedulingTime.timeIntervalSinceNow, 0) |
|
Cool |
corbt
force-pushed
the
jsscheduling-buffer
branch
from
a49677c to
47ade9f
Compare
|
@corbt updated the pull request. |
corbt
force-pushed
the
jsscheduling-buffer
branch
from
47ade9f to
c73cfa3
Compare
|
@corbt updated the pull request. |
|
@tadeuzagallo updated and rebased. |
|
@facebook-github-bot shipit |
facebook-github-bot
added
GH Review: accepted
and removed
GH Review: review-needed
labels
|
Thanks for importing. If you are an FB employee go to Phabricator to review. |
ghost
closed this
in
Summary:This commit modifies the jsSchedulingOverhead warning to only fire if the JS clock is more than 5 seconds ahead of the native clock. This fixes the issue in facebook#1598 for the common case when there's only a minor difference between the two clocks, while still keeping a sanity check if they're extremely off. cc nicklockwood tadeuzagallo Closes facebook#5731 Differential Revision: D3014985 Pulled By: tadeuzagallo fb-gh-sync-id: bf57e48b7d97ad02d2aefb6e5aac845824a6fdb0 shipit-source-id: bf57e48b7d97ad02d2aefb6e5aac845824a6fdb0
This commit modifies the jsSchedulingOverhead warning to only fire if the JS clock is more than 5 seconds ahead of the native clock. This fixes the issue in #1598 for the common case when there's only a minor difference between the two clocks, while still keeping a sanity check if they're extremely off.
cc @nicklockwood @tadeuzagallo