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

METRON-2272: [UI] Performance: Switching manual filtering on and off multiple times leads slow typing #1524

Closed
wants to merge 1 commit into from

Conversation

@ruffle1986
Copy link
Contributor

ruffle1986 commented Oct 3, 2019

Contributor Comments

Link to the original Jira ticket: https://issues.apache.org/jira/browse/METRON-2272

The problem

Every time we instantiate an ace editor instance on an angular component, it immediately forces angular to re-render the entire application very frequently (every 300-500 ms). For the first time, it's not that noticeable but after a while it can be annoying because it leads to performance issues and when you type something an ace editor input, it's gonna be really slow.

The performance slowdown is even more noticeable when you change between manual and normal query mode many times (This is how we have caught it).

Under the hood the frequent updates make Angular's zone manager very busy because it forces it to do a lot of dirty checking. To be fair it really is an issue only in development mode when dirty checking require more resource consumption.

Testing

Checkout the latest master branch and open the Alerts UI. Go to the alerts-list page and start typing something in the search input field at the top of the page just to get the feeling how it should behave normally. Then change between "Manual query" and "Query builder" mode a few times (e.g. 15-20 times). Make sure you're in "Query builder" mode and type something again in the search input field. See how slow and laggy it is?

Now checkout METRON-2272 and repeat the previous steps. Like a breath of a fresh air, isn't it?

Further

As a bonus I also destroy the ace instances when angular is about to detach the component from the DOM. My assumption is that we could achieve even bigger performance boost if we convert our ace editor directive to a component and change the changeDetection strategy to onPush. I didn't want to do that in this PR in order to make the review process easier.

Pull Request Checklist

Thank you for submitting a contribution to Apache Metron.
Please refer to our Development Guidelines for the complete guide to follow for contributions.
Please refer also to our Build Verification Guidelines for complete smoke testing guides.

In order to streamline the review of the contribution we ask you follow these guidelines and ask you to double check the following:

For all changes:

  • Is there a JIRA ticket associated with this PR? If not one needs to be created at Metron Jira.
  • Does your PR title start with METRON-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
  • Has your PR been rebased against the latest commit within the target branch (typically master)?

For code changes:

  • Have you included steps to reproduce the behavior or problem that is being changed or addressed?

  • Have you included steps or a guide to how the change may be verified and tested manually?

  • Have you ensured that the full suite of tests and checks have been executed in the root metron folder via:

    mvn -q clean integration-test install && dev-utilities/build-utils/verify_licenses.sh 
    
  • Have you written or updated unit tests and or integration tests to verify your changes?

  • If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?

  • Have you verified the basic functionality of the build by building and running locally with Vagrant full-dev environment or the equivalent?

For documentation related changes:

  • Have you ensured that format looks appropriate for the output in which it is rendered by building and verifying the site-book? If not then run the following commands and the verify changes via site-book/target/site/index.html:

    cd site-book
    mvn site
    
  • Have you ensured that any documentation diagrams have been updated, along with their source files, using draw.io? See Metron Development Guidelines for instructions.

Note:

Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.
It is also recommended that travis-ci is set up for your personal repository such that your branches are built there before submitting a pull request.

@sardell

This comment has been minimized.

Copy link
Contributor

sardell commented Oct 3, 2019

+1. Tested with Full Dev. This is a great catch, and I'm fully on-board with converting the ace editor directive to a component in the future to utilize onPush. In fact, I think a lot of our components could eventually be changed to onPush to improve performance, but I might be getting a little ahead of myself here. 😄

@asfgit asfgit closed this in 03e0977 Oct 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.