Esri / esri-loader

A tiny library to help load ArcGIS API for JavaScript modules in non-Dojo applications
Apache License 2.0
459 stars 79 forks source link

Extreme slowness when rendering hundreds of rows and dozens of columns w/ Angular #215

Closed rustygreen closed 4 years ago

rustygreen commented 4 years ago

Issue Description

There is an issue when using the ArcGIS JavaScript API in an Angular application when the Angular application is either rendering a lot of data or there are a lot of elements in the DOM. There is a minimal example below (Stackblitz) that demonstrates the issue by simply including the IdentityManager package into the page, which exponentially degrades performance (immediately upon adding the IdentityManager package.

Background

This issue can be replicated with or without the use of the esri-loader package. However, I was instructed by Esri support to file this issue on this repository (even though I don't think it is related to the esri-loader project). Nonetheless, the Esri case # is #02498050.

Example

In the example below, the angular application will run perfectly fine, but as soon as you add the Esri IdentityManager package to the page it will severely degrade the performance of the page (even without showing a map or using the IdentityManager).

Steps to reproduce:

  1. Load the following demo page: https://see-grid-demo.firebaseapp.com/
  2. Wait for the grid to load (will take a couple of minutes), hover over the grid rows, and scroll up and down (notice the quick response)
  3. Click the "Add Esri IdentityManager JS" button. This will simply load the "IdentityManager" package into the page (the text at the top of the grid will tell you when it is done loading).
  4. Hover of the grid rows and scroll up and down, the entire page will become extremely slow.

The demo page source code can be view/ran from this Stackblitz page: https://stackblitz.com/edit/kendo-grid-80-cols-esri-loader?file=index.html

Other Notes:

andygup commented 4 years ago

@rustygreen did you try this in a stand-alone JS app without Angular?

andygup commented 4 years ago

Maybe there is another table grid library you can try? Here's an example of a table handling 50,000 rows: https://codepen.io/benesri/pen/JjdEvEX that the tech support analyst put together.

I was able to repro this behavior using the stackblitz sample + JS API 4.14. I also did some digging and didn't see anything obvious. IdentityManager initializes itself and returns a singleton, so there's no telling where the conflict might be between that and Kendo Grid. Since we don't officially support Kendo Grid or Angular there's nothing we can fix at this time.

rustygreen commented 4 years ago

Hey, @andygup. Thanks for looking into this. Unfortunately, the example you provided negates the suspected underlying issue (a large amount of elements in the DOM or Angular digest cycles). If we use a grid with virtual scrolling (as your example does) then the issue can not be reproduced - that is already known. The issue occurs when there is a lot of Angular digest/watches in place.

My theory is that something in the Esri JavaScript API (likely with dojo or dojo/watch functionality) is interfering or colliding with the Angular watch/digest lifecycle.

Here is another example of the issue occurring without the use of Kendo. The same behavior can be demonstrated (page is fast without Esri JS API but slow after adding Esri JS API - including IdentityManager in this scenario):

Demo/Example: https://angular-kdgjkm.stackblitz.io/

Here is the Stackblitz code if you want to see the code or play with it: https://stackblitz.com/edit/angular-kdgjkm?file=src%2Fapp%2Ftable-basic-example.ts

Steps to reproduce:

  1. Load the following demo page: https://see-grid-demo.firebaseapp.com/
  2. Wait for the grid to load (will take a couple of minutes), hover over the grid rows, and scroll up and down (notice the quick response)
  3. Click the "Add Esri IdentityManager JS" button. This will simply load the "IdentityManager" package into the page (the text at the top of the grid will tell you when it is done loading).
  4. Hover of the grid rows (pretty quickly) and scroll up and down, the hover effect will be delayed. The issue gets exponentially worse the more data that is added.
andygup commented 4 years ago

One other suggestion, test again using our /next (a.k.a slash-next) build. I gave it a quick try and saw improvements, but you're the final judge on this. It ran much better on my machine when I tested it with Chrome. /next gives you early dev access to enhancements coming out in the next build of the API, which in this case is 4.15. With reach release we bolt in more under-the-hood performance improvements. Additional info: https://github.com/Esri/feedback-js-api-next.

And on a related note, we are working hard on removing dojo dependencies with each release, for example: https://developers.arcgis.com/javascript/latest/guide/release-notes/#api-modernization. Our ultimate goal is ESM compliance, btw. So please stay tuned!

    import { setDefaultOptions } from 'esri-loader';

    setDefaultOptions({ url: `https://js.arcgis.com/next/` });  // **** 

    loadModules(["esri/identity/IdentityManager"])
      .then(([IdentityManager]) => {
        this.instructions =
          "The ESRI package has been loaded. Continue to move your mouse rapidly, you should see highlighting slow down exponentially... 😟";
      })
      .catch(console.error);
tomwayson commented 4 years ago

Thanks for the detailed repro cases @rustygreen.

I took a look at the code for anything that jumped out at me that would cause problems w/ lots of digest cycles, but I see that you're not even using the IdentityManager(), just loading it.

If you really only need identity, you could try @esri/arcgis-rest-auth.

benelan commented 4 years ago

Hi @rustygreen,

I put together another sample without virtual scrolling: https://codepen.io/benesri/pen/abOWpWK

The issue still does not seem to be occurring in a standalone JS app.

Thanks, Ben

tomwayson commented 4 years ago

Thanks for that @benelan!

I forked your pen and used esri-loader to lazy load the JSAPI instead of loading it up front, to make sure that the script injection wasn't causing any issues. I observed the same behavior (no problems scrolling through the list nor w/ hover behavior).

https://codepen.io/tomwayson/pen/zYGwNLL

My take on this is that the issue is on the Kendo or Angular side, and not an esri-loader nor ArcGIS API for JavaScript issue.

andygup commented 4 years ago

It's not an Angular 8 issue either. Here's an Angular 8 example (using @tomwayson's codepen): https://stackblitz.com/edit/angular-arcgis-datatable-test. It's working okay on my machine with Chrome, FF and Safari.

rustygreen commented 4 years ago

One other suggestion, test again using our /next (a.k.a slash-next) build.

Thanks, @anandxb. It seems like using the pre-release (next) version of the ArcGIS JS API helped quite a bit. I'll do some more testing and get back to you.

rustygreen commented 4 years ago

It's not an Angular 8 issue either. Here's an Angular 8 example (using @tomwayson's codepen): https://stackblitz.com/edit/angular-arcgis-datatable-test. It's working okay on my machine with Chrome, FF and Safari.

@anandxb, thanks for putting this together. I think this confirms that the issue is related to Angular and digest/watch cycles conflicting with ArcGIS JS/dojo (not the number of elements on the page). Using jQuery to generate the tables does not result in the issue because Angular will not watch any of the rows created through jQuery.

Here is an example (based on yours but without jQuery) that is just Angular (no other libraries): https://angular-arcgis-datatable-test-nojquery.stackblitz.io/

Source code: https://stackblitz.com/edit/angular-arcgis-datatable-test-nojquery?file=src%2Fapp%2Fapp.component.ts

I think that example confirms that the issue is with ArcGIS JS/dojo when running in an Angular application where Angular has a large number of items it is "watching/tracking".

The next release did seem to help a little in this example, but not entirely.

rustygreen commented 4 years ago

Issue Summary/Findings

I think at this point, it is safe to say that our examples/findings conclude that there is an issue when the Esri JavaScript API is used within Angular (but only when a certain criterion is met within the Angular app). The criteria for this issue to occur is a large amount of data or items (tracked in Angular) are present on the page (such as a large grid without virtual scrolling). Once this criterion is met, just loading the IdentityManager package will result in extreme slowness within the page (while the page was performing perfectly fine before the Esri module was loaded). This behavior can be demonstrated with the following example:

https://stackblitz.com/edit/angular-arcgis-datatable-test-nojquery

Next Steps

So, the question is; is there anything that can be done in the ArcGIS JS API to account for this?

Thanks for everyone's help on this peculiar issue.

tomwayson commented 4 years ago

It could be that all the dynamic injection of script tags into the DOM done by Dojo's AMD loader is what causes the issue.

If that's the case, it might be worth trying https://github.com/Esri/angular-cli-esri-map/tree/arcgis-webpack-angular

@andygup would know better about the caveats and constraints.

In that case you could choose to not lazy load the ArcGIS API, which I think would solve this problem, but come w/ a initial load performance hit. If you did, still need to lazy loading, it would be done via dynamic import(), which will still likely load dozens of bundles, but maybe, just maybe the mechanism by which webpack's loader does that would not cause this issue.

andygup commented 4 years ago

@rustygreen I did two quick tests using your latest codepen + https://github.com/Esri/angular-cli-esri-map/tree/arcgis-webpack-angular (Angular 9 and ArcGIS API for Javascript 4.14).

Test 1 - 1500 rows and 90 columns the performance was okay. When I dropped the number of rows down to 200 then performance seemed pretty snappy to me.

Test 2 - I know your codepen didn't have a map, but I added a vector map to increase processor load and add additional memory pressure. By reducing the row and/or column count, I was able to find a point where performance on my machine was pretty good, for example: 200 row and 20 columns as shown below in the .gif, but I also tried 50 rows and 90 columns with similar performance:

a

So, the question is; is there anything that can be done in the ArcGIS JS API to account for this?

Not at this time. My recommendation is give the webpack build a try and experiment with tweaking the number of row and/or columns to get your desired performance.

Since there are workarounds as documented above and, as you mentioned, this is a very specific set of circumstances, at this point in time I see this as a no fix.

rustygreen commented 4 years ago

Thanks, @andygup. I did some testing using the webpack build. I found that using the webpack build produced the same slowness issue as the esri-loader. Below are results using the webpack build (using Esri's arcgis-webpack-angular example project):

Results

Left Side: No Esri packages imported Right Side: Exact same code but with the IdentityManager package imported (not even used):

import IdentityManager from "esri/identity/IdentityManager";
const x = IdentityManager; // This is only here to make sure the unused import doesn't get automatically removed.

esri-packages

*Let me know if you want me to post the source code for this example.

andygup commented 4 years ago

@rustygreen Does reducing the number of rows and columns in your app speed things up, e.g. 200 x 90, 200 x 20, etc?

I'm curious have you considered paginating, or doing some sort of infinite scroll? Those patterns were designed with this and similar issues in mind.

andygup commented 4 years ago

@rustygreen try this new stackblitz and let us know if this works better on your machine? https://stackblitz.com/edit/angular-arcgis-datatable-test-nojquery-changedetect?file=src/app/app.component.ts

I added ChangeDetectorRef to detach the view from the change-detection tree and then manually reattach it at set intervals. This seems to significant reduce the change detection thrashing issue with a very large table.

Not sure how these tweaks will affect the rest of your code but I saw zero performance degradation when I double checked things on an old 2013 Macbook with 8GB of RAM.

andygup commented 4 years ago

@rustygreen I'm going to go ahead and close this. If something new comes up feel free to ping me. Or, if needed lets set up a follow-up call.