Don't abuse timers #1108
Merged
Don't abuse timers #1108
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@chancancode to clarify, there was not a 300ms timer that fired infinitely. Updates were fired when the |
rwjblue
approved these changes
Dec 18, 2019
|
@rwwagner90 This is unrelated to how we update the render tree. This is just how we update the currently inspected object. When you call updateCurrentObject (which we always do in init as of now), at the end of the method, it will always unconditionally (even when there isn’t a current object) schedule a setTimeout that calls itself again, run-wrapped, which will then schedule another timer, etc. Worse, i think it’s actually possible to call this method multiple times and end up with a set of 300ms infinitely repeating timers. |
wycats
force-pushed
the
no-timers
branch
2 times, most recently
from
de58714 to
53b453d
Compare
|
Awesome! Really good work. |
wycats
approved these changes
Dec 18, 2019
Previously, the object inspector would schedule an infinite timer that fires every 300ms to update the current object (it actually fires even where there isn't a current object to update). This is extremely inefficient, and it causes a lot of extra work in big apps, since the timer starts a runloop which results in a full top-down revalidation. The more appropriate thing to do is tap into the runloop callbacks, since as far as Ember is concerned, if there are no runloops, there are no observable changes.
rwjblue
approved these changes
Dec 18, 2019
| import { typeOf } from './utils/type-check'; | ||
|
|
||
| const Ember = window.Ember; | ||
| const { | ||
| Object: EmberObject, inspect: emberInspect, meta: emberMeta, | ||
| computed, get, set, guidFor, isNone, | ||
| cacheFor, VERSION | ||
| cacheFor, VERSION, run, |
There was a problem hiding this comment.
Thank you for not nesting this!
nummi
pushed a commit
to nummi/ember-inspector
that referenced
this pull request
Apr 1, 2020
Previously, the object inspector would schedule an infinite timer that fires every 300ms to update the current object (it actually fires even where there isn't a current object to update). This is extremely inefficient, and it causes a lot of extra work in big apps, since the timer starts a runloop which results in a full top-down revalidation. The more appropriate thing to do is tap into the runloop callbacks, since as far as Ember is concerned, if there are no runloops, there are no observable changes.
nummi
pushed a commit
to nummi/ember-inspector
that referenced
this pull request
Apr 5, 2020
Previously, the object inspector would schedule an infinite timer that fires every 300ms to update the current object (it actually fires even where there isn't a current object to update). This is extremely inefficient, and it causes a lot of extra work in big apps, since the timer starts a runloop which results in a full top-down revalidation. The more appropriate thing to do is tap into the runloop callbacks, since as far as Ember is concerned, if there are no runloops, there are no observable changes.
patricklx
pushed a commit
to patricklx/ember-inspector
that referenced
this pull request
Sep 19, 2022
Previously, the object inspector would schedule an infinite timer that fires every 300ms to update the current object (it actually fires even where there isn't a current object to update). This is extremely inefficient, and it causes a lot of extra work in big apps, since the timer starts a runloop which results in a full top-down revalidation. The more appropriate thing to do is tap into the runloop callbacks, since as far as Ember is concerned, if there are no runloops, there are no observable changes.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, the object inspector would schedule an infinite timer that fires every 300ms to update the current object (it actually fires even where there isn't a current object to update). This is extremely inefficient, and it causes a lot of extra work in big apps, since the timer starts a runloop which results in a full top-down revalidation. The more appropriate thing to do is tap into the runloop callbacks, since as far as Ember is concerned, if there are no runloops, there are no observable changes.
Split from #1088. I'll rebase that PR once this lands.