Library bundling, filesize, etc #68
Comments
|
(FYI this has been something on our minds all along, but the discussion in #67 prompted creating this issue.) |
|
Also wanted to follow up on @jgaehring's comment in #67 : #67 (comment):
The So I think the first thing we would need to think through is how to make it possible to selectively import layer types and behaviors. Those seem like the pieces that would offer the biggest "wins" in terms of overall filesize. |
|
Ran a few quick tests of
So based on that (assuming I didn't miss anything), it looks like the behaviors are the big culprit, responsible for ~+100K. I wouldn't be surprised if Google is the biggest chunk of that. But even so... the library is still 400K even with all that stuff disabled. |
|
If I also remove all "extra" default controls and interactions (LayerSwitcher, FullScreen, Rotate, ScaleLine, Geolocate, Geocoder, DragRotateAndZoom, and PinchRotate), the file size is: 349652 bytes So those add ~50K. |
|
I pushed these three removal commits to a |
Just to clarify, I bring up the global object, not because that alone increases the filesize somehow, but because I believe it impairs Webpack's ability to automatically remove unused modules from the bundle when applications use farmOS-map. This is the "tree-shaking" concept: https://webpack.js.org/guides/tree-shaking/. |
|
Removing ONLY the This seems like a good first "low-hanging fruit" to start with. Field Kit does not use Google layers. |
Oh interesting! So would Webpack be smart enough to NOT include Google code even though |
Correct. I think the most reasonable thing for us to do is just to make sure the library doesn't break tree-shaking for the application code using it, so it's totally in the hands of the consumer which features they use or don't use, and which features get bundled or don't. It's less aggravation and fewer decisions for us, and more control in the hands of the consumer. |
Ok, I'd love to do a rough test of this in Field Kit to see if it reduces the filesize of that in the same way! But... I think Field Kit is using the global |
|
Yea, it would take some refactoring. I wonder if it's worth trying to bundle the example index.html you have in this library to run tests? |
|
I also wonder if it's worth having two separate I think there are probably far better strategies, where we could optimize the bundle for Drupal as well, but this strikes me as the easiest solution for right now. If I understood better how Drupal uses that global, and had time to think about, I'm sure we could optimize for the Drupal context as well. |
|
I just ran one more test, where I only removed the pieces that Field Kit doesn't use/need (I think), including:
That brings the library size from 518987 bytes to 423106 bytes, which is a difference of: 95881 bytes. What this tells me (if assumptions are correct) is that if Webpack's "tree-shaking" can be made to work, that is roughly how much we would reduce the size of Field Kit. I ran a quick So back of the envelope math (rough, and admittedly there is more to this): 1512846 - 95881 = 1416965 bytes which is a 6% decrease. That's not nothing. But as of right now I think we have more important things to work on. Saving 96K won't make that much of a difference, I don't think. So I think my vote would be to shelve these ideas for now, and come back to it in the future. What do you think @jgaehring ? I pushed these commits to a second branch: |
That approach makes a lot of sense to me! |
Totally fine! I just wanted to make sure I registered my concern about the global, and that we didn't take any drastic measures or reject PR's on the basis of bundle size, if there were better alternatives. Let's circle back to this when things slow down. I wouldn't mind shaving off that 6%. I think the other big contributor is Bootstrap, but that's another issue. |
|
Agreed! And it's nice to have these numbers - they definitely put to rest my reservation about adding additional layer type options. That seems to be a very small contributor. |
|
The issue of the In the context of farmOS (Drupal), maps need to be displayed in various contexts. And each context will want to have different features enabled on the map. For example:
Furthermore, and most importantly: farmOS modules can add additional features to maps and/or add their own maps to other places in the UI. For example:
We have no way of know which modules will be installed. And a site admin can enable/disable modules whenever they want, without any "build" process. This gets to the other important point... Drupal does not run Webpack* - All it can do is add pre-made JS files to a given page. (*It may be possible to figure out a way to run Webpack from Drupal, but there are a lot more considerations there, including added requirements for hosting/installing/updating/etc.) So, in the farmOS/Drupal context... we basically have a situation where there is a map on a page, and any enabled modules can add things to that map. But the only way they can do that is by adding a JS file to the page. (Oh and one other "furthermore": you can potentially have multiple maps on a page.) Thus, the This global provides a few things:
So, to outline how a map is built in farmOS:
Thus, modules can make modifications to maps by simply adding a JS file to the page. That's the overview... hope it all makes sense. I'll save my "next steps" thoughts for future comments. I have some ideas... :-) |
|
Next steps: Apart from the farmOS/Drupal (and similar) context, the other context farmOS-map could be used in is as a component of another JavaScript application. In that context, farmOS-map would be managed as a dependency via In that context, the ability to add behaviors to a map via The ability to create map instances IS still necessary. Currently that is done via Then... there's the matter of methods on the With all that in mind, I'm curious to understand which pieces in particular are currently preventing Webpack's "tree shaking". It feels to me like the global |
|
Another important consideration here is the effect of extending farmOS-map on the final page size. As an example, I created a module which provides a snapping grid for use with farmOS-map. There doesn't seem to be a way to "link" such a module such that it can leverage the library code that farmOS-map already bundles. This means that the independently bundled code for farm_map_snapping_grid is more than 200KB. |
|
@symbioquine yea that's a great example of this tricky issue of bundling OpenLayers in farmOS-map, but wanting to re-use OpenLayers code in farmOS-map behaviors (or any other JS scripts) that are loaded dynamically on a page (not bundled with farmOS-map.js itself). One idea there was to essentially expose some OpenLayers code on the In farmOS itself, the considerations are different because we don't have the ability to bundle everything together. Modules can be added which provide their own pre-bundled JS files (like yours), and webpack is not part of the farmOS build/deployment process. In the old days, you would just load Those are all the "general" thoughts I have on the topic.... but getting back to your module specifically... I'm also curious if it would make sense to just merge that in as a behavior that farmOS-map itself provides. That would allow it to be bundled together. What do you think? Worth considering? |
Those are great points. What about the idea of running the Webpack build on module installation? I think it was mentioned above, but I'm not sure I understood whether that concept was already thoroughly explored...
Absolutely! I only published it as a separate module to gather an initial round of feedback and get it into production for my use-case quicker. I consider the current implementation a bit of a prototype. In particular I'd like feedback on the UI/UX choices that I made and whether the grid size normalization which I'm doing to convert between flat/spherical coordinates introduces too much error for folks in practice. I'll open an issue to track this potential integration in the farmOS-map issue queue. |
It hasn't been thoroughly explored. But it definitely introduces complexity for self-hosting farmOS. I'd love to think it through though, perhaps in a separate issue, to map out all the considerations. I'd also be curious to see if the Drupal community at large has put any thought into this, because it is a general Drupal deployment question in my mind. It's certainly possible, it would just come down to the trade-offs involved. And one of my goals is always trying to keep the hosting process relatively simple so it doesn't become more of a barrier to entry for self-hosting. |
It looks like there's already some existing work on this front in the larger Drupal community targeting >= D8; https://www.drupal.org/project/webpack Would it make sense to keep a tracking issue for that potential integration in the main farmOS issue queue? |
Makes sense to me! Thanks for digging in @symbioquine ! |
Actually it looks like a better strategy is using diff --git a/modules/core/map/farm_map.libraries.yml b/modules/core/map/farm_map.libraries.yml
index 82f0dc59..8a9928ab 100644
--- a/modules/core/map/farm_map.libraries.yml
+++ b/modules/core/map/farm_map.libraries.yml
@@ -5,8 +5,16 @@ farmOS-map:
url: https://github.com/farmOS/farmOS-map/blob/master/LICENSE
gpl-compatible: true
js:
+ /libraries/farmOS-map/dist/ol.js:
+ minified: true
/libraries/farmOS-map/dist/farmOS-map.js:
+ # Skip aggregating farmOS-map.js with other JS since that
+ # breaks the lazy loading of behavior chunks.
+ preprocess: false
minified: true
+ css:
+ theme:
+ /libraries/farmOS-map/dist/ol.css: { }
dependencies:
- core/drupalSettings |
|
I've reviewed all the proposed changes in #109 and they look really good @symbioquine ! I like where this is headed! Perhaps it makes sense to organize/summarize the "next steps" (maybe in the PR description?) so we can be sure everything is accounted for... including relevant PRs for Field Kit and farmOS 2.x. Just a couple of notes I made while reviewing:
|
|
Would it make sense to add an agenda item to discuss these changes and next steps on the 5/12 monthly call (this Wednesday)? I don't think we have anything else planned. Or, if we want to find a separate meeting time that works too. Would love to have both @paul121 and @jgaehring join. |
|
Awesome, thanks for looking over it all @mstenta!
Definitely!
Will fix those.
I'm open to either or both. If I don't hear a preference for a stand-alone meeting from @paul121 and @jgaehring here, I'll add it this evening to the Monthly Call agenda. |
|
On a related note, in the last 36 hours or so I've been experimenting with a strategy that would allow us to have our cake and eat it too! Specifically, we can somewhat avoid the costs of loading all of OpenLayers while still having it all available to contrib modules. The key to doing that is by creating an intermediate package which leverages a new feature provided by Webpack called Module Federation - kind of like dynamic linking. Naively using that feature would yield one huge chunk. However, with a bit of scripting, the intermediate package can expose each importable package from OpenLayers as a separate "module" in the resulting federated "container". In short, this will allow dynamic/lazy loading of individual imports from OpenLayers as cachable chunks. Of course, nothing is free and the cost to the strategy described above is that we'd be trading increased network requests and packaging complexity for reduced bandwidth. I think this may be a good tradeoff especially since one of the main contributors to OpenLayers has indicated that the all-in-one builds of OpenLayers are "legacy" and "[they] discourage its use". As such, it may be better to incur the cost of a breaking change now, moving to this module federation strategy where we would control the packaging than to have the 2.x contrib ecosystem built against something which OpenLayers considers legacy and discouraged. |
|
Awesome! I haven't had a chance to get caught up on this thread but eager to talk with y'all about it. Standalone meeting or monthly call is fine for me. |
|
Sorry for the radio silence here. I've actually been putting in quite a bit of time on this, but I didn't have anything to report back until now... I have been evaluating the Webpack Module Federation feature I mentioned above to determine whether it is worth the cost of increased packaging complexity. BLUF¹Module Federation is better than using the official legacy For the examples tested, Module Federation can result in about 45% less JS data transferred over the network and 60% less JS data loaded client side compared with the official legacy Using module federation to package all of OpenLayers is (as expected) slower than the control case of including only the bits we need. For the map tested, this works out to about 45% (~640ms) longer loading times than the control in our "DSL network constrained" scenario. Those loading time percentages above remain pretty consistent at different network speeds/latencies, but it is important to note that at 2G networks speeds, Module Federation could turn a 10s uncached page load into a 7s one - but is still worse than the 5s page load without including all of OpenLayers. DetailsI created a test bed to compare the performance of various packaging strategies; https://github.com/symbioquine/ol-federated/tree/main/examples All examples render the same map scene - a stretch of OSM ocean with a LineString (code here);
I ran this performance test against each packaging strategy 10 times. The performance test gathers metrics for 5 scenarios ( The packaging strategies differ only in their dependencies and
For the given map scene those scenarios yield the following amount of JS sent over the network;
Also, here's the breakdown of the number of JS resources involved in each;
And the total bytes decoded/decompressed;
The most important metric (I've found so far) is what I'm calling
Zooming in on the
We can see that the static Zooming in on the
We can see that Module Federation out-performs even the control case. This probably warrants further investigation, but my guess would be that the browser is able to better parallelize loading with the increased chunk count. This may be suggestive of the expected performance on modern mobile devices that have more (slower) cores. I also measured the time to
first-contentful-paint.csv |
Ref: farmOS#68 (comment) Will be squashed in final mergeable pull-request version.
I've addressed all these things - except for the "update instructions" part which is included in the new "Known Remaining Work" section of the PR description. The next step is I need alignment on whether it makes sense to pursue the module federation strategy instead of externalizing to the legacy static |
Just a quick status update here. I'm still looking for input on this point and am willing abandon it if there's significant pushback about the idea, but I have started exploring how best to use Module Federation for farmOS-map. It's looking a bit more complicated than I originally anticipated. Hopefully I'll have more details soon that can feed into the decision... |
|
Incredible work @symbioquine - thanks for taking such a thorough and detailed look at all of this! I will defer to you and @jgaehring and @paul121 on the best way forward, but at quick glance it sounds like the module federation strategy is the best from a performance perspective! Curious to hear what other complexities you're finding too. I generally prefer simplicity unless there's a good reason (and bandwidth-constrained situations is a good reason), and the tradeoffs makes sense. "Long term maintenance" considerations are a big part of "complexity" as I define it - not necessarily just "how much extra code is involved" (although they are related). I'm curious to understand a bit more about how it "works" in the farmOS context. I can read up a bit more on that myself, but if you have any good links/resources to bring me up to speed I'd welcome them! The naive sense I got was that it involves Also, are there any considerations around browser compatibility we need to be aware of? |
Agreed that complexity is multi-faceted. The biggest complexities I've been trying to overcome relate to how farmOS-map needs to cater to contrib modules which are written in plain JS or built with other bundlers that don't play nicely with Webpack. At some level I'd love if we could just go with externalizing the OpenLayers dependency to the legacy From that perspective the performance opportunity almost feels secondary.
The best resources right now are the high-level description which is present in the official Webpack documentation (https://webpack.js.org/concepts/module-federation/) and the examples at https://github.com/module-federation/module-federation-examples One of the challenges with Module Federation is that dependency management in general is already a complex topic and it gets way more complex with dynamic linking and the amount of surface area represented by non-standardized NPM packaging practices. Module Federation is quite new and the body of official/community documentation doesn't fully do it justice. The most pessimistic viewpoint is that the lack of documentation is a ploy to sell the primary contributors' book on the topic. That might be true, but it's also a very powerful feature and I expect the documentation gap to be pretty short-lived. All that said, I'm including a "TL;DR;" summary version below.
No worries, even if this were totally obvious from the PR it would be good to summarize it here for posterity. There's a couple things going... Webpack has a concept of "entrypoints" which are the JS files that are intended to be directly included via Prior to the introduction of Module Federation, Webpack provided some mechanisms for designating which bundled dependencies go into which entrypoints and dynamically creating appropriately sized chunks. That all works pretty optimally in the context of a single Webpack build, but falls down when separately-built JS code would bundle the same dependencies. Module Federation pulls the concept of package dependency and version management into the runtime layer. That allows two or more builds to both bundle overlapping shared dependencies, but put them into chunks - meaning they can potentially get deduplicated and only loaded once at runtime. Using Module Federation necessarily creates a new entrypoint - another file that would need to be included via a One challenge is that Webpack builds with Module Federation have (at least) two conflicting goals;
Since we know that we want all of OpenLayers available to contrib modules we need to tell Webpack not to tree-shake any of it. One mechanism I've found to defeat the tree-shaking is to build one of the precursors to the legacy We also want to somehow tell Webpack that any overlapping subsets of OpenLayers may be used and organize it chunks such that there's a reasonable number of chunks and most pages don't tend to need all of them. As I indicated above, the biggest challenge is how to cater to contrib modules which aren't built with Webpack. It is possible to expose each public module in OpenLayers separately in the federated container. That allows for a trivial mechanism to access any part of OpenLayers - even from plain-JS/non-Webpack code(example) - but seems to produce way too many tiny chunks. Having so many chunks creates its own performance problems - as I demonstrated in my performance testing above. What I'm exploring now is how to get Module Federation to work such that it automatically hits a sweet spot in terms of the number of chunks and also makes it trivial for arbitrary JS code to gain access to any part of OpenLayers. |
|
Hey @symbioquine apologies for my silence as well. I think this is my first time chiming in on this thread but I have been following the convo :-) This is great - appreciate all the research & time you've put into this! Especially the explanations! Module Federation does seem to be a great option especially considering that the legacy
Same question, this hasn't quite clicked for me.. important we don't overlook this!
So for Drupal we would likely be aggregating these chunks together rather than being loaded asynchronously? I'm curious if we could get asynchronous loading to work, but it seems like it would require some configuration within the Module Federation plugin to specify where the chunks (remotes?) are located? Even if async isn't possible in Drupal there still seems to be worthwhile benefits with Module Federation, so not a deal breaker.. I guess I'm assuming each chunk would require a separate
So yea, this makes sense as a next step! I built the A condensed version of the optimal Drupal libraries I'm imagining: https://gist.github.com/paul121/e3f701976bea216146e59289c4927a8e Obviously async would be preferable to this... but I think the Drupal library system could work. Sure hard to beat the convenience of |
Agreed.
Actually, (when code splitting is used) Webpack produces code that loads the chunks asynchronously at runtime - by default using the fetch API and assumes that the chunks are at the same location as the entrypoints. Chunks are normally named with numbers or hashes - e.g. Having Drupal aggregate the chunks for each page would also defeat the network level request caching in the browser. Instead Drupal would only know about the entrypoints. My lightweight testing has shown that Webpack's async chunk loading works fine in Drupal 9 that way as long as
A third party module shouldn't need to know anything about how OL is packaged other than the interface of how it is exposed. e.g.
I'm thinking something like that breakdown too - though as described above those chunks wouldn't need to be registered in Drupal, only the entrypoints (something like
Yeah, that's what I'm trying to overcome. |
|
A little out of band, but wanted to get some recent discussions with @jgaehring and @wgardiner included here (for lack of a better place - maybe this deserves a separate issue)... Outside of the farmOS context, we also want to be able to support projects that pull farmOS-map in as a node dependency. Right now, we publish to NPM, but there are some things we're doing that are not ideal:
Field Kit is currently working around this, and SurveyStack (which @wgardiner is working on) will be doing the same. One idea that @wgardiner suggested is to create an init function that wraps the logic for creating And @jgaehring mentioned in chat:
@symbioquine what are your thoughts on these points, and how do they relate to these proposed changes? I know that the |
That's really cool - so what that means (if I'm understanding correctly) is that we would still just have a single "library" in farmOS (like we do for |
Thanks for bringing this up! I was trying to figure out when to work it into the discussion, but didn't want to distract from the I do think that addressing how farmOS-map is packaged/exposed would be an auspicious thing to change as part of the 2.x major version since it necessarily involves breaking changes.
I don't have a ready answer other than that I'm excited to brainstorm with you folks to see if we can find a more coherent approach... One thought is that by changing import farmOSMap from 'farmOS-map/main';
farmOSMap.behaviors.myAwesomeBehavior = {
attach(instance) {
...
},
};Of course, it's arguably a bad idea to mutate the things you import since that's akin to global state/variables. It might make sense for us to look at what plugin models isomorphic JS libraries are using. Maybe we can learn something from those...
Exactly! :) The main challenges in my opinion are;
|
|
I've moved the conversation on NPM packaging, default exports, and the |
|
Based on my 1.x performance baseline results and the learning that Google maps is the main driver of extraneous loading latency, I've rerun the benchmarks with Google maps and with/without the Without Google maps; diff --git a/examples/extended-behavior/static/index.html b/examples/extended-behavior/static/index.html
index 887fe81..eff1a52 100644
--- a/examples/extended-behavior/static/index.html
+++ b/examples/extended-behavior/static/index.html
@@ -21,7 +21,6 @@
</head>
<body>
<div id="map"></div>
- <script type="text/javascript" src="https://maps.googleapis.com/maps/api/js?v=3&key=AIzaSyBEB1xUvz8LG_XexTVKv5ghXEPaCzF9eng"></script>
<script src="./ol.js"></script>
<script src="./farmOS-map.js"></script>
<script src="./farmOS-map-example-ext-behavior.js"></script>
@@ -31,7 +30,6 @@
units: 'metric',
}
var myMap = farmOS.map.create("map", options);
- myMap.addBehavior("google");
myMap.addBehavior("edit").then(() => {
myMap.addBehavior("measure", { layer: myMap.edit.layer });
myMap.edit.wktOn("featurechange", console.log);
diff --git a/examples/simple-html-consumer/static/index.html b/examples/simple-html-consumer/static/index.html
index 33e796d..cf99e48 100644
--- a/examples/simple-html-consumer/static/index.html
+++ b/examples/simple-html-consumer/static/index.html
@@ -21,7 +21,6 @@
</head>
<body>
<div id="map"></div>
- <script type="text/javascript" src="https://maps.googleapis.com/maps/api/js?v=3&key=AIzaSyBEB1xUvz8LG_XexTVKv5ghXEPaCzF9eng"></script>
<script src="./ol.js"></script>
<script src="./farmOS-map.js"></script>
<script src="./mapbox.js"></script>
@@ -30,7 +29,6 @@
units: 'metric',
}
var myMap = farmOS.map.create("map", options);
- myMap.addBehavior("google");
myMap.addBehavior("edit").then(() => {
myMap.addBehavior("measure", { layer: myMap.edit.layer });
myMap.edit.wktOn("featurechange", console.log);
diff --git a/examples/test/index.spec.js b/examples/test/index.spec.js
index 26d7efe..1963006 100644
--- a/examples/test/index.spec.js
+++ b/examples/test/index.spec.js
@@ -36,8 +36,8 @@ exampleApps.forEach((app) => {
expect(result).toEqual(true);
}, 60 * 1000);
- describe(`get metrics from 3 perf.test.js runs`, () => {
- for (let index = 0; index < 3; index++) {
+ describe(`get metrics from 10 perf.test.js runs`, () => {
+ for (let index = 0; index < 10; index++) {
it(`${app} run #${index}`, async () => {
const env = Object.assign({}, process.env);
env.TEST_PORT_NUM = await getPort({port: getPort.makeRange(3000, 3100)});Without Google maps and without externalizing to diff --git a/examples/extended-behavior/static/index.html b/examples/extended-behavior/static/index.html
index 887fe81..21275c8 100644
--- a/examples/extended-behavior/static/index.html
+++ b/examples/extended-behavior/static/index.html
@@ -21,8 +21,6 @@
</head>
<body>
<div id="map"></div>
- <script type="text/javascript" src="https://maps.googleapis.com/maps/api/js?v=3&key=AIzaSyBEB1xUvz8LG_XexTVKv5ghXEPaCzF9eng"></script>
- <script src="./ol.js"></script>
<script src="./farmOS-map.js"></script>
<script src="./farmOS-map-example-ext-behavior.js"></script>
<script src="./mapbox.js"></script>
@@ -31,7 +29,6 @@
units: 'metric',
}
var myMap = farmOS.map.create("map", options);
- myMap.addBehavior("google");
myMap.addBehavior("edit").then(() => {
myMap.addBehavior("measure", { layer: myMap.edit.layer });
myMap.edit.wktOn("featurechange", console.log);
diff --git a/examples/extended-behavior/webpack.config.js b/examples/extended-behavior/webpack.config.js
index 63175a3..2a6f5e4 100644
--- a/examples/extended-behavior/webpack.config.js
+++ b/examples/extended-behavior/webpack.config.js
@@ -28,18 +28,4 @@ module.exports = {
],
}),
],
- externals: function ({context, request}, callback) {
- // Externalize all OpenLayers `ol` imports
- if (/^ol(\/.*)?$/.test(request)) {
- const modifiedRequest = request
- // Remove '.js' suffix - if present
- .replace(/\.js$/, "")
- // Replace filesystem separators '/' with module separators '.'
- .replace(/\//g, ".");
- return callback(null, modifiedRequest);
- }
-
- // Continue without externalizing the import
- callback();
- },
};
diff --git a/examples/minimal-html-consumer/static/index.html b/examples/minimal-html-consumer/static/index.html
index f2c97c8..4dfcb9c 100644
--- a/examples/minimal-html-consumer/static/index.html
+++ b/examples/minimal-html-consumer/static/index.html
@@ -21,7 +21,6 @@
</head>
<body>
<div id="map"></div>
- <script src="./ol.js"></script>
<script src="./farmOS-map.js"></script>
<script>
var options = {
diff --git a/examples/simple-html-consumer/static/index.html b/examples/simple-html-consumer/static/index.html
index 33e796d..465765b 100644
--- a/examples/simple-html-consumer/static/index.html
+++ b/examples/simple-html-consumer/static/index.html
@@ -21,8 +21,6 @@
</head>
<body>
<div id="map"></div>
- <script type="text/javascript" src="https://maps.googleapis.com/maps/api/js?v=3&key=AIzaSyBEB1xUvz8LG_XexTVKv5ghXEPaCzF9eng"></script>
- <script src="./ol.js"></script>
<script src="./farmOS-map.js"></script>
<script src="./mapbox.js"></script>
<script>
@@ -30,7 +28,6 @@
units: 'metric',
}
var myMap = farmOS.map.create("map", options);
- myMap.addBehavior("google");
myMap.addBehavior("edit").then(() => {
myMap.addBehavior("measure", { layer: myMap.edit.layer });
myMap.edit.wktOn("featurechange", console.log);
diff --git a/webpack.config.js b/webpack.config.js
index e0ecfd8..868bd08 100644
--- a/webpack.config.js
+++ b/webpack.config.js
@@ -1,6 +1,5 @@
const webpack = require('webpack');
const CopyWebpackPlugin = require('copy-webpack-plugin');
-const SaveRemoteFilePlugin = require('save-remote-file-webpack-plugin');
const info = require('./package.json');
@@ -42,36 +41,11 @@ module.exports = {
},
plugins: [
new webpack.BannerPlugin(`farmOS-map ${info.version}`),
- new SaveRemoteFilePlugin([
- {
- url: 'https://raw.githubusercontent.com/openlayers/openlayers.github.io/master/en/v6.5.0/build/ol.js',
- filepath: 'ol.js',
- hash: false,
- },
- {
- url: 'https://raw.githubusercontent.com/openlayers/openlayers.github.io/master/en/v6.5.0/css/ol.css',
- filepath: 'ol.css',
- hash: false,
- },
- ]),
new CopyWebpackPlugin({
patterns: [
{ from: './examples/simple-html-consumer/static' },
+ { from: './node_modules/ol/ol.css' },
],
}),
],
- externals: function ({context, request}, callback) {
- // Externalize all OpenLayers `ol` imports
- if (/^ol(\/.*)?$/.test(request)) {
- const modifiedRequest = request
- // Remove '.js' suffix - if present
- .replace(/\.js$/, "")
- // Replace filesystem separators '/' with module separators '.'
- .replace(/\//g, ".");
- return callback(null, modifiedRequest);
- }
-
- // Continue without externalizing the import
- callback();
- },
};1.x vs 2.x (without
|
| 1.x Unthrottled | 2.x Unthrottled | 1.x Regular4G | 2.x Regular4G | 1.x Regular2G | 2.x Regular2G | |
|---|---|---|---|---|---|---|
| minimal-html-consumer/uncached | 1013 | 1200 | 1546 | 1620 | 8761 | 8361 |
| minimal-html-consumer/cached | 971 | 973 | 970 | 973 | 2059 | 2053 |
| simple-html-consumer/uncached | 1467 | 1550 | 1544 | 1745 | 9028 | 9341 |
| simple-html-consumer/cached | 980 | 1084 | 1679 | 1302 | 2023 | 2055 |
| extended-behavior/uncached | 1367 | 1411 | 1541 | 1503 | 9346 | 9389 |
| extended-behavior/cached | 990 | 1091 | 1329 | 1302 | 2032 | 2053 |
transfer/decoded (bytes)
| 1.x transferred | 2.x transferred | 1.x decoded | 2.x decoded | |
|---|---|---|---|---|
| minimal-html-consumer/uncached | 142388 | 115662 | 566734 | 440896 |
| minimal-html-consumer/cached | 0 | 0 | 566734 | 440896 |
| simple-html-consumer/uncached | 147957 | 144425 | 579733 | 525956 |
| simple-html-consumer/cached | 0 | 0 | 579733 | 525956 |
| extended-behavior/uncached | 149230 | 145703 | 581207 | 527436 |
| extended-behavior/cached | 0 | 0 | 581207 | 527436 |
2.x without vs with Externalization to ol.js Results
Full test output: 2021_05_30_2.x_perf_test_results_less_gmaps_with_ol_dot_js.txt
Some key observations;
nav-to-net-idle (ms)
| 2.x Unthrottled | 2.x (with ol.js) Unthrottled | 2.x Regular4G | 2.x (with ol.js) Regular4G | 2.x Regular2G | 2.x (with ol.js) Regular2G | |
|---|---|---|---|---|---|---|
| minimal-html-consumer/uncached | 1200 | 1394 | 1620 | 1823 | 8361 | 13078 |
| minimal-html-consumer/cached | 973 | 973 | 973 | 973 | 2053 | 2049 |
| simple-html-consumer/uncached | 1550 | 1537 | 1745 | 1907 | 9341 | 13552 |
| simple-html-consumer/cached | 1084 | 1205 | 1302 | 1308 | 2055 | 2052 |
| extended-behavior/uncached | 1411 | 2031 | 1503 | 2046 | 9389 | 15190 |
| extended-behavior/cached | 1091 | 2071 | 1302 | 2058 | 2053 | 2073 |
transfer/decoded (bytes)
| 2.x transferred | 2.x (with ol.js) transferred | 2.x decoded | 2.x (with ol.js) decoded | |
|---|---|---|---|---|
| minimal-html-consumer/uncached | 115662 | 265576 | 440896 | 1061968 |
| minimal-html-consumer/cached | 0 | 0 | 440896 | 1061968 |
| simple-html-consumer/uncached | 144425 | 281015 | 525956 | 1101154 |
| simple-html-consumer/cached | 0 | 0 | 525956 | 1101154 |
| extended-behavior/uncached | 145703 | 282005 | 527436 | 1101790 |
| extended-behavior/cached | 0 | 0 | 527436 | 1101790 |
Discussion
For these tests I disabled Google maps integration (like the most recent 1.x performance tests) and tested with and without the externalization of OpenLayers to ol.js. What that means is that I'm independently testing the performance consequences of making behavior loading lazy/async and externalizing OpenLayers to ol.js.
The most interesting observation concerns the "extended-behavior" example. Without externalizing to ol.js that example involves 1.3KB/2KB JS transferred/decoded vs only 1KB/0.5KB JS transferred/decoded with externalizing to ol.js. However, surprisingly the performance penalty for loading the extra behavior is actually much worse at all levels of throttling with externalizing to ol.js.
Thus, it appears that - at least for modest amounts of duplicate bundling of OpenLayers functionality - it's better to just allow those to be duplicated than to try avoiding the duplication via externalizing to ol.js.
In theory Webpack Module Federation could yield the best of both worlds, but in practice I haven't found a way to configure it that yields performance similar to that which we get by bundling OpenLayers normally. In some cases I was able to beat the performance of externalizing to ol.js, but not by enough for me to feel comfortable recommending that as a viable path forward at the moment.
To summarize, my tentative conclusions are that farmOS-map 2.x should;
- Do make behaviors async and use lazy chunks for the included behaviors - allows farmOS-map core behaviors to keep growing without necessarily incurring incremental performance costs for all use-cases
- Do remove core support for Google maps - a related change should involve packaging up Google maps as an optional contrib module
- Not externalize to
ol.js- this is expensive from a performance perspective and would involve depending on a legacy packaging option for OpenLayers - Not adopt Webpack Module Federation for OpenLayers at this time - this seems elegant, but in practice the tooling isn't there yet to intelligently tune the chunks sizes for a large dependency like OpenLayers
**Why?** Better support alternative bundling strategies
and scenarios where having mutable state in the global
`window.farmOS.map` is ugly/limiting.
These changes should allow farmOS-map to also be used
as follows;
```sh
npm install @farmos.org/farmos-map@2
```
```js
import MapInstanceManager from '@farmos.org/farmos-map/MapInstanceManager';
const maps = new MapInstanceManager();
const map = maps.create('my-map');
```
Some background can be found in the following issues;
* farmOS#113
* farmOS#68
**Why?** Better support alternative bundling strategies
and scenarios where having mutable state in the global
`window.farmOS.map` is ugly/limiting.
These changes should allow farmOS-map to also be used
as follows;
```sh
npm install @farmos.org/farmos-map@2
```
```js
import MapInstanceManager from '@farmos.org/farmos-map/MapInstanceManager';
const maps = new MapInstanceManager();
const map = maps.create('my-map');
```
Some background can be found in the following issues;
* farmOS#113
* farmOS#68
**Why?** Better support alternative bundling strategies
and scenarios where having mutable state in the global
`window.farmOS.map` is ugly/limiting.
These changes should allow farmOS-map to also be used
as follows;
```sh
npm install @farmos.org/farmos-map@2
```
```js
import MapInstanceManager from '@farmos.org/farmos-map/MapInstanceManager';
const maps = new MapInstanceManager();
const map = maps.create('my-map');
```
Some background can be found in the following issues;
* farmOS#113
* farmOS#68
|
I think we can close this along with #113 - what do you think @symbioquine ? We've made great progress on these fronts, and this issue documents a lot of it - but I don't think it needs to remain open, does it? |
|
Agreed. To summarize;
|
The bundled
farmOS-map.jsfile is pretty big. As of this writing, it clocks in at just over half a megabyte in size.Most of this size comes from including various modules from the OpenLayers library. The approach we've taken with farmOS-map is to essentially create a pre-bundled, pre-configured, OpenLayers map that includes all the things that farmOS itself needs for its maps, along with some wrappers and helper functions for making it easier for farmOS modules to do common things (like add a WKT or GeoJSON layer). This approach allows us to create a standalone
farmOS-map.jsfile that can be included in a non-JS application (like farmOS/Drupal).The main drawback of this approach is that it's an "all or nothing" library. You can't, for example, say "I want farmOS-map.js with just the WKT and GeoJSON layer support". This sort of selective bundling would be really useful in the context of JS applications like Field Kit, which doesn't need everything that is included in farmOS-map.
It would be worth doing a quick test where we strip out everything that Field Kit doesn't need, build that, and see how much of a difference it actually makes. Maybe the "extras" are not as much as the "core" OpenLayers code itself, which would need to be included anyway. That would be a good first thing to test.
If that does make a significant difference, the next challenge is thinking about how to restructure this library so that it can more easily be imported into another JS application in pieces. Here is a quick list of the "pieces" that could potentially be made optional:
The text was updated successfully, but these errors were encountered: