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

Serious Performance Issue with New $interval added 1.2.2 #235

Closed
onemenny opened this issue Jan 20, 2016 · 33 comments
Closed

Serious Performance Issue with New $interval added 1.2.2 #235

onemenny opened this issue Jan 20, 2016 · 33 comments

Comments

@onemenny
Copy link

@sergiovilar Commited a change 68c505d

This causes a serious performance degregation since the count was removed from the $interval call - it causes a digest loop to run indefinitely which will slow down to the ground every application

The new version 1.2.2 is just not usable and should revert to 1.2.0

@sroze
Copy link
Owner

sroze commented Jan 20, 2016

Can't it be the same as #186 ?
On Wed, 20 Jan 2016 at 09:07, Menny Rachmanny (onemenny) <
[email protected]> wrote:

@sergiovilar https://github.com/sergiovilar Commited a change
https://githubcom/sroze/ngInfiniteScroll/commit/68c505d131e14b6e4997ab83dc675d67209634ad

This causes a serious performance degregation since the count was
removed from the $interval call - it causes a digest loop to run
indefinitely which will slow down to the ground every application

The new version 122 is just not usable and should revert to 120


Reply to this email directly or view it on GitHub
#235.

@onemenny
Copy link
Author

No, it does not matter if reach the end, It is related that the $interval method works indefinitely causing an endless digest cycles

@sergiopvilar
Copy link
Contributor

Hey, sorry about that. There's any suggestion to fix the issue?

@onemenny
Copy link
Author

Sorry, I don't know what this fix meant to do, I have no problems with 1.2.0. One can always use the disable option, If you can elaborate I'll be happy to help

@morgoth
Copy link

morgoth commented Jan 22, 2016

I confirm the issue. Angular tests in our app started to fail with timeout. Reverting to 1.2.1

@pudo
Copy link

pudo commented Jan 22, 2016

Confirmed here, too. Showstopper.

@pierrebeitz
Copy link

+1

@dirkgroenen
Copy link

Same issues over here, looks like the $interval on line 203 is running each ms. Didn't check why the interval needs to run, but it might be worthy to check if we can't change it to the browser's native setInterval so we don't call a digest cycle each millisecond.

@dirkgroenen
Copy link

@sergiovilar before making any suggestions, what was the reason for adding the interval and making it run the digest cycle?

@timburnham73
Copy link

+1 I have the same problem. It broke our protractor tests in chrome only. Firefox worked which is strange.

@scottmollon-invicara
Copy link

+1 breaking all our protractor tests!

@000panther
Copy link

+1 I can confirm this issue, and confirm that it works when you change the code back. what does the new code do anyway?

@sroze
Copy link
Owner

sroze commented Feb 9, 2016

@sergiovilar could you PR a fix please?

This was referenced Feb 10, 2016
@jmlora
Copy link

jmlora commented Feb 19, 2016

+1 I have this problem with 1.2.2, not in 1.2.0.

@feimosi
Copy link

feimosi commented Feb 27, 2016

+1, also happens on 1.2.2

@pelizza
Copy link

pelizza commented Mar 4, 2016

+1, happens on 1.2.2 but not on 1.2.1.

@belfz
Copy link

belfz commented Mar 7, 2016

+1, although for me it happens on 1.2.1.

@onemenny
Copy link
Author

onemenny commented Mar 8, 2016

@sroze @sergiovilar
Can someone please revert this commit, this is starting to be embarrassing.

@janhartigan
Copy link

@onemenny just revert to an old release on your app until this is fixed.

@david-gang
Copy link

In my opinion it should be at least mentioned in the readme just to use 1.2.0

@onemenny
Copy link
Author

onemenny commented Mar 8, 2016

@janhartigan That's exactly what I did, but still, this is kind of ridicules that the main version is not functioning for that long. @david-gang is right

@scanferla
Copy link

+1

Took me and my team quite some time to figure this one out 😢

@sroze
Copy link
Owner

sroze commented Mar 9, 2016

True, I'll fix that today.
On Wed, 9 Mar 2016 at 04:25, Gustavo Scanferla [email protected]
wrote:

+1

Took me and my team quite some time to figure this one out 😢


Reply to this email directly or view it on GitHub
#235 (comment)
.

@stephenycchang
Copy link

I'm having an issue where there are over 200 digests per second just from implementing this plugin in a sample barebone project. Think it's related as well.

@miguelolmo
Copy link

+1 reverting to 1.2.1 fixed it for me

@belfz
Copy link

belfz commented Apr 4, 2016

What's the status of that? Version 1.2.0 doesn't work for me (it doesn't have infiniteScrollListenForEvent), and version 1.2.1 (I'm using npm) was not fixed yet.

@woutrbe
Copy link

woutrbe commented Apr 13, 2016

Reverting to 1.2.1 or 1.2.0 didn't work for me. Is there any update on this?

@belfz
Copy link

belfz commented Apr 13, 2016

There was a pull request merged and closed recently, but no new version released yet. @sroze could you release at least patch version? Thank you in advance.

@peder
Copy link

peder commented Apr 29, 2016

Some users report that this is not working in 1.2.1 either. Can anyone confirm that the Pull Request changes fix the issue? Also, any update on publishing 1.2.3?

@graingert
Copy link
Collaborator

NPM "ng-infinite-scroll": "1.2.0", is the only one that works for me.

@eXaminator
Copy link

We are also experiencing this, more than 3 month after this serious bug was reported it is still not resolved. Please release a new version with a fix for this (as someone stated it seems to be merged) and publish it to NPM (it seems the last version on NPM is 1.2.1 while github already has 1.2.2).

georgebabey pushed a commit to georgebabey/angular-multi-select that referenced this issue Jun 14, 2016
Downgrading ng-infinite-scroll to 1.2.0 to avoid infinite digest loops
which kills performance. See sroze/ngInfiniteScroll#235
georgebabey added a commit to ApplauseOSS/angular-multi-select that referenced this issue Jun 14, 2016
Downgrading ng-infinite-scroll to 1.2.0 to avoid infinite digest loops
which kills performance. See sroze/ngInfiniteScroll#235
@graingert
Copy link
Collaborator

@onemenny @eXaminator please try 1.3.0

@mephinet
Copy link

mephinet commented Jul 4, 2016

Tested 1.3.0 - $interval has it's 3rd parameter again, performance is back to normal. Thanks, @graingert for fixing this!

@graingert graingert changed the title Seriouse Performance Issue with New $interval added 1.2.2 Serious Performance Issue with New $interval added 1.2.2 Jul 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests