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

UI Hang When Clearing Search Filter #777

Closed
matthughes opened this issue Oct 15, 2013 · 12 comments
Closed

UI Hang When Clearing Search Filter #777

matthughes opened this issue Oct 15, 2013 · 12 comments

Comments

@matthughes
Copy link

I have a grid with ~2K rows. Occasionally, if I type a query in the filter and then clear it, Firefox will completely hang. Doing some profiling when it happens, I see this stack:

profiler_-http___10 10 2 109_9000_app_core___events_and_file_upload_in_topology-firebird-_confluence_and___ccad_development_workspaces_active_firebird_vms_plugins_topology_web_src_main_java_com_ccadllc_firebird_vms_topology_web_changese

This is using Angular/Angular-Animcate 1.2.0-rc2 and 2.0.7 of Grid.

I have upgraded to 1.2.0-rc3 and have not been able to reproduce since. Filtering seems more responsive overall with RC3.

@jesslilly
Copy link

I have the same issue! And I have a plunkr! I'm using exclamation points because I'm glad I'm not the only one! Also I know if we can reproduce it, we can fix it! I'm not sure, but I think the problem is in $digest. Or maybe the grid is adding too many watches or recursive watches.

Here's the plunkr:

http://plnkr.co/edit/vKhezr

With this issue I either have to sacrifice column sorting or filtering as a work around. This issue will kill my page because I often have thousands of rows.

@jesslilly
Copy link

IMO This should be a high priority.

@matthughes
Copy link
Author

Have you tried it with Angular 1.2.0.rc3? I have not been able to
reproduce since upgrading to that version. And there definitely were
animation improvements in that release.

On Wed, Oct 23, 2013 at 10:52 PM, Jess Lilly notifications@github.comwrote:

I have the same issue! And I have a plunkr! I'm using exclamation points
because I'm glad I'm not the only one! Also I know if we can reproduce it,
we can fix it! I'm not sure, but I think the problem is in $digest. Or
maybe the grid is adding too many watches or recursive watches.

Here's the plunkr:

http://plnkr.co/edit/vKhezr

With this issue I either have to sacrifice column sorting or filtering as
a work around. This issue will kill my page because I often have thousands
of rows.


Reply to this email directly or view it on GitHubhttps://github.com//issues/777#issuecomment-26963141
.

@jesslilly
Copy link

I will try it, but 1.0.8 is the highest stable release right now. Anything higher is unstable.

@jesslilly
Copy link

@matthughes, thank you for the response. I updated my app and the plunkr. The issue remains. Have you tried the plunkr? If you increase the number of rows in the grid from 1000 to 5000 you really see quite a big performance impact. It takes maybe 30 seconds to clear the filter field and update the grid.

@matthughes
Copy link
Author

Yeah I see it. It definitely seems to happen repeatedly if you do the sorting as your plunker describes.

profiler_-_http___plnkr co_edit_vkhezr_p_preview

I heard talk (#716) about addressing performance issues in ngGrid 1.0.7 but haven't seen any follow up.

@jaydamask
Copy link

I, too, have this problem.

I'm running (stable release) ng 1.0.8 in production, so I'm reluctant to upgrade to the rc (although I will if that helps). I'm using the latest ng-grid.

I cannot tell if it is the # of rows or the total number of cells that matter. My application .get()'s some tables with thousands of records. On the ng-grid the table populates quickly. Application of one or more filters also works well. However, setting filterText = '' (via a clear or other means) virtually crashes the app once I'm between 1k-2k records. (For this table I have ~15 columns).

It's suspicious that applying a non-empty filter is fast while clearing the filter is so slow. I've tried to catch the ng-grid code in the debugger to find where it's looping, but as per matthughes, the pb starts w/ jQuery.min.js (I'm on 2.10).

The asymmetry of the performance, filter-on vs filter-off, should mean that a fix is possible. Right now I find it hard to use ng-grid in production given that my app can spin forever and crash.

@jaydamask
Copy link

dpangler: thank you for working on this. Is it clear that filter clearing is slow only after a sort? I have found clear to be very slow with large tables (1k rows+) even without sort.

@dpangier
Copy link

No, it isn't clear that this is the only cause - the bug can be triggered whenever there is a filter in place which reduces the number of displayed rows below the "virtualisation threshold" and the scroll top position is changed.

@jaydamask
Copy link

I see. That may be the situation I'm observing, but I don't have it in front of me. I will get back on that.

Nonetheless, the code governs the behavior, and I appreciate that you've found another case, one that probably applies to me, where the bad behavior manifests itself.

dpangier added a commit to dpangier/ng-grid that referenced this issue Nov 15, 2013
…sort operation. The renderedRange was incorrectly being set to the whole of the source data.
@jaydamask
Copy link

Thanks dpangier for pushing a fix. I can confirm that, before the fix, a clear after filtering data causes a near hang when the before state is where ng-grid presents a scrollbar and has lots of records, and the after state is when the filter slims the selection down so much that the scrollbar disappears.

@dpangier
Copy link

There are still issues. When rows are added dynamically, the viewableRange is still being miscalculated. The change in pull request #821 appeared to work by reducing the rendered row count dramatically, but still got it wrong under common circumstances. I am submitting another PR with what I hope is a correct fix.

roblarsen added a commit that referenced this issue Dec 3, 2013
Second attempt to fix #777, still ensuring the rendering row count is sensible
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants