Delay load google-earth-dbroot-parser #7140
Conversation
`google-earth-dbroot-parser.js` is a huge dependency (487 KB source, 202KB minified). It's also only needed if you are using a Google Earth server. In order to avoid bloat and paying the penalty every time Cesium is loaded, this change loads it on demand the first time it's needed. I didn't use jsonp for this because there's no server to parse the query and wrap the code in a callback. Instead we just load the script directly into a unlikely to collide global variable. The file gets minified and copied to ThirdParty, similar to what we were already doing with the wasm file. Also fixed a bug in buildCesiumViewer where it wasn't copying all the necessary files to its own build output. Also extracted `loadAndExecuteScript` out of Resource.js, but had to keep a version of it in Resource.js because cleaning up test abuse of it is a much larger change best for another PR. (I'll write up an issue).
|
Thanks for the pull request @mramato!
Reviewers, don't forget to make sure that:
I am a bot who helps you make Cesium awesome! Contributions to my configuration are welcome. 🌍 🌎 🌏 |
|
FYI, full gzipped release went from 737.1 KB to 710.2 KB (~3.7% decrease) |
|
No need to update CHANGES as this was a completely behind the scenes change. |
|
Cool, I think a mention in CHANGES.md is valuable given the size difference for those not using GEE. |
| script.async = true; | ||
| script.src = url; | ||
|
|
||
| var head = document.getElementsByTagName('head')[0]; |
There was a problem hiding this comment.
Isn't there a (small) chance the page may not have a head element? Can we just append it to the document?
There was a problem hiding this comment.
The head element will always exist.
| .then(function() { | ||
| dbrootParser = window.cesiumGoogleEarthDbRootParser(protobufMinimal); | ||
| if (defined(oldValue)) { | ||
| window.cesiumGoogleEarthDbRootParser = oldValue; |
There was a problem hiding this comment.
It looks like we're using window.cesiumGoogleEarthDbRootParser for both the loaded script, and the function evaluated with protobufMinimal. Which means you have to set back the oldValue after evaluating. Would this be clearer if we just loaded the module to window.cesiumGoogleEarthDbRootModule or something similar?
There was a problem hiding this comment.
I'm not sure what you mean. Here's what's currently happening
- We get the old value of
cesiumGoogleEarthDbRootParseron the off-chance it exists - We load the script, which defines
cesiumGoogleEarthDbRootParser. - We call the
cesiumGoogleEarthDbRootParserfunction and store the result intodbrootParser(module scoped) - We set back the old value of
cesiumGoogleEarthDbRootParseror delete it completely if it didn't exist to begin with.
There was a problem hiding this comment.
Got it, sorry for the confusion. Makes sense.
|
Thanks @mramato! This is a nice improvement for most users, who are not using GEE. |
|
Looks good. Just update |
It's not the specs for Ready. |
google-earth-dbroot-parser.jsis a huge dependency (487 KB source, 202KB minified). It's also only needed if you are using a Google Earth server.In order to avoid bloat and paying the penalty every time Cesium is loaded, this change loads it on demand the first time it's needed.
I didn't use jsonp for this because there's no server to parse the query and wrap the code in a callback. Instead we just load the script directly into a unlikely to collide global variable. (also reset the old value if it does exist for some reason).
The file gets minified and copied to ThirdParty individually, similar to what we were already doing with the wasm file.
Also fixed a bug in buildCesiumViewer where it wasn't copying all the necessary files to its own build output.
Also extracted
loadAndExecuteScriptout of Resource.js, but had to keep a version of it in Resource.js because cleaning up test abuse of it is a much larger change best for another PR. (I'll write up an issue).I talked this over a bit with @shunter and he agreed this is a stopgap solution at best and we either need to leverage require more directly as a module loading system within Cesium itself, or better yet just spend the time to migrate to ES6 (which we want to do but simply don't have the bandwidth for). Until that happens, we might be able to use this approach for a few other large dependencies as well.
@likangning93 you should also be able to use
loadAndExecuteScriptin #6986, but you'll need to tweak it for Workers.@ggetz can you review of this? I think I covered every possible use case, but let me know if you find anything.