When loading KML documents we will spread out loading to prevent freezing

[tfili]

We load the first 128 features, then give up time to the render loop. This is using setTimeout with a value of 0 since we don't have a modern promise library.

Running some tests, it was slightly slower but prevented freezing. With a 51MB KMZ file the loading went from 18s to 23s, but the overall hanging went from 18s to a single 3s freeze.

[cesium-concierge]

Thanks for the pull request @tfili!

• ✔️ Signed CLA found.
• ❔ CHANGES.md was not updated.
  • If this change updates the public API in any way, please add a bullet point to CHANGES.md.
• ❔ Unit tests were not updated.
  • Make sure you've updated tests to reflect your changes, added tests for any new code, and ran the code coverage tool.

Reviewers, don't forget to make sure that:

• Cesium Viewer works.
• Works in 2D/CV.
• Works (or fails gracefully) in IE11.

[OmarShehata]

🎉 🎉 🎉 🎉 🎉 🎉

[hpinkos]

The code changes look fine to me.

Loading the KML examples in sandcastle took noticeably longer to render in this branch. I can see that this is a win for very large KML files with lots of features, but would we have a way to only do the deferred loading once the file size or number of features is larger than some threshold? Why did you pick 128? Maybe make that value larger?

[hpinkos]

Is this meant to replace #3970?

[tfili]

Loading the KML examples in sandcastle took noticeably longer to render in this branch.

I didn't see that in my testing. The times were almost the same. The GDP one was even faster. The facilities was slightly slower.

[tfili]

Why did you pick 128? Maybe make that value larger?

Larger values made the mouse movement more choppy and smaller values didn't really help much.

[hpinkos]

The facilities one was the one that was the most noticeable. In master it loads almost instantly. In this branch it took a few seconds.

[tfili]

With a build version of the app in incognito mode, in master facilities loaded in 800ms and in this branch it was 1200ms. The GDP kml was near identical (master was less than 100ms faster, which may be noise) and the bike race document was actually faster in this branch.

Upping the number from 128 will make them closer, but with larger documents it will increase jumpyness, but it will still be interactive to a point. The size of the file isn't a very good measure. A lot of documents have full HTML pages embedded in them that show up in the info bubble for a feature, which can make them giant but with only a few features and facilities is like 600KB with a crap ton of features. I guess it comes down to if we care about small documents being a little slower to load or not. I have some more mid-range sized files I can test with to see how changing that number effects them. I was testing with a pretty large document.

[hpinkos]

Okay, that sounds fine to me @tfili. Thanks for taking the time to get some more numbers.

Someone else should take a quick look at this, but it's okay with me to merge =) @mramato do you have a minute to check it out?

[tfili]

Here is the loading time comparing master, branch with 128 entity limit and branch with 512 entity limit. The 512 one seems more jumpy and master freezes. This is the times for just the parsing of the KML file. Times don't include KMZ decompressing and creation of graphics resources. Times in ms.

File	master	128	512
1.8MB KMZ (13.7MB KML)	6376	7611	5541
5MB KML	2304	3146	2533
9MB KMZ (38MB KML)	2219	3351	2174
48MB KMZ (182MB KML)	14523	16714	15400

[tfili]

This is updated to now use time instead of the number of features since they can vary pretty widely. It allows 1 sec to load off the bat and then follow up loading happens in 0.1 sec increments. This allows smallish documents to load as fast as they did before and larger documents to load in reasonable time.

[mramato]

Update CHANGES.md

[tfili]

Done

[mramato]

I think that's all I have @tfili while I still hold out hope for a shared time-slicing system in CesiumJS down the line, this is a decent incremental change and should be easy enough to tweak as needed if we see any unwanted behavior. The 1 second load before slicing seems like a great idea that should work well in practice as well.

I still wish this (optionally) tied into the render loop somehow, but that could be part of the larger time-slicing system (one day).

[mramato]

Thanks again, @tfili!

[OmarShehata]

This is supposed to supersede #3970 right?

[mramato]

Yes, thanks/.

[maxvonhippel]

Was this also done for GeoJSON?