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

Avoid infinite digest loop caused by $watch and $timeout #10036

Merged
merged 1 commit into from
Jan 31, 2017

Conversation

weltenwort
Copy link
Member

@weltenwort weltenwort commented Jan 24, 2017

#9326 introduced the usage of a custom debounce implementation in the fixed-scroll directive used for the discover table. That custom debounce implementation uses Angular's $timeout, which interacts unfavourably with the unconditional $watch handler used in the fixed-scroll directive. It results in a digest being triggered about every 100ms, even when the page is otherwise idle:

  1. unconditional $watch starts $timeout
  2. $timeout elapses, triggers a digest
  3. goto 1

image

Preventing debounce from always triggering a digest cycle in combination with an explicit $apply does not exhibit that behaviour:

image

It also avoids some additional digest cycles when the scroll width has not changed.

@weltenwort weltenwort added Feature:Discover Discover Application :Discovery bug Fixes for quality problems that affect the customer experience v5.3.0 v6.0.0 review labels Jan 24, 2017
@weltenwort weltenwort force-pushed the fix-debounce-digest-loop branch 2 times, most recently from 695b9c1 to 23471b2 Compare January 25, 2017 12:27
@Bargs Bargs self-assigned this Jan 26, 2017
@Bargs
Copy link
Contributor

Bargs commented Jan 27, 2017

jenkins, test this

@Bargs
Copy link
Contributor

Bargs commented Jan 27, 2017

Test failure looked unrelated. Once they pass this LGTM. We should backport to 5.2 as well.

@Bargs Bargs added the v5.2.1 label Jan 27, 2017
@Bargs Bargs removed their assignment Jan 27, 2017
@weltenwort
Copy link
Member Author

thanks, @Bargs. The failure seems to be related to the map tile changes. I'll rebase on master and try again.

The custom debounce implementation uses Angular's `$timeout`, which
interacts unfavourably with the unconditional `$watch` handler used in
the `fixed-scroll` directive. It results in an infinite digest being
triggered about every 100ms. To avoid that, this commit uses the
`invokeApply` option of `$timeout` and instead calls `$scope.$apply`
conditionally.
@weltenwort weltenwort force-pushed the fix-debounce-digest-loop branch from 23471b2 to 13c677d Compare January 27, 2017 16:36
Copy link
Member

@lukasolson lukasolson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@weltenwort weltenwort merged commit 3dacbae into elastic:master Jan 31, 2017
weltenwort added a commit that referenced this pull request Jan 31, 2017
Backports PR #10036

**Commit 1:**
Avoid infinite digest loop in debounce

The custom debounce implementation uses Angular's `$timeout`, which
interacts unfavourably with the unconditional `$watch` handler used in
the `fixed-scroll` directive. It results in an infinite digest being
triggered about every 100ms. To avoid that, this commit uses the
`invokeApply` option of `$timeout` and instead calls `$scope.$apply`
conditionally.

* Original sha: 13c677d
* Authored by Felix Stürmer <stuermer@weltenwort.de> on 2017-01-24T11:30:41Z
weltenwort pushed a commit that referenced this pull request Jan 31, 2017
Backports PR #10036

**Commit 1:**
Avoid infinite digest loop in debounce

The custom debounce implementation uses Angular's `$timeout`, which
interacts unfavourably with the unconditional `$watch` handler used in
the `fixed-scroll` directive. It results in an infinite digest being
triggered about every 100ms. To avoid that, this commit uses the
`invokeApply` option of `$timeout` and instead calls `$scope.$apply`
conditionally.

* Original sha: 13c677d
* Authored by Felix Stürmer <stuermer@weltenwort.de> on 2017-01-24T11:30:41Z
weltenwort added a commit that referenced this pull request Jan 31, 2017
Backports PR #10036

**Commit 1:**
Avoid infinite digest loop in debounce

The custom debounce implementation uses Angular's `$timeout`, which
interacts unfavourably with the unconditional `$watch` handler used in
the `fixed-scroll` directive. It results in an infinite digest being
triggered about every 100ms. To avoid that, this commit uses the
`invokeApply` option of `$timeout` and instead calls `$scope.$apply`
conditionally.

* Original sha: 13c677d
* Authored by Felix Stürmer <stuermer@weltenwort.de> on 2017-01-24T11:30:41Z
weltenwort pushed a commit that referenced this pull request Jan 31, 2017
Backports PR #10036

**Commit 1:**
Avoid infinite digest loop in debounce

The custom debounce implementation uses Angular's `$timeout`, which
interacts unfavourably with the unconditional `$watch` handler used in
the `fixed-scroll` directive. It results in an infinite digest being
triggered about every 100ms. To avoid that, this commit uses the
`invokeApply` option of `$timeout` and instead calls `$scope.$apply`
conditionally.

* Original sha: 13c677d
* Authored by Felix Stürmer <stuermer@weltenwort.de> on 2017-01-24T11:30:41Z
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience Feature:Discover Discover Application review v5.2.1 v5.3.0 v6.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants