During the code review accompanying the viewer/composer refactoring, I noticed that page loading times can be reduced by two simple measures: 1. Do not use IFrames to embed maps =================================== As already done in the SearchTable component [1] (props to ltucker for the great work on this), it is possible to integrate maps directly into web pages. All that needs to be done is create a GeoExplorer(.Viewer) instance/subclass with a proper portalConfig, and Django writing the configuration to the main page's template directly. 2. Do not load unneeded JS and CSS resources ============================================ Currently, every page generated by Django uses a template that includes all JS and CSS resources. The extreme case is the start page: it does not need any of it, but loads all. And there is an IFrame loaded by the page which loads the same components itself. I think these two things are easy to fix for the more Django savvy team members. If you feel like wanting to take this on and have questions, please don't hesitate to contact me. -Andreas. [1] http://bit.ly/aov6xp -- Andreas Hocevar OpenGeo - http://opengeo.org/ Expert service straight from the developers.
On 06/24/2010 04:09 PM, Andreas Hocevar wrote: > During the code review accompanying the viewer/composer refactoring, I noticed that page loading times can be reduced by two simple measures: > > 1. Do not use IFrames to embed maps > =================================== > > As already done in the SearchTable component [1] (props to ltucker for the great work on this), it is possible to integrate maps directly into web pages. All that needs to be done is create a GeoExplorer(.Viewer) instance/subclass with a proper portalConfig, and Django writing the configuration to the main page's template directly. > Yep, sounds great. We do need to keep the iframe viewer around to support embedding maps in outside web pages though, right? Or do we have some new embeddable viewer based on jsonp or something? > 2. Do not load unneeded JS and CSS resources > ============================================ > > Currently, every page generated by Django uses a template that includes all JS and CSS resources. The extreme case is the start page: it does not need any of it, but loads all. And there is an IFrame loaded by the page which loads the same components itself. > Actually it's somewhat complicated to do and still support switching between minified and not. Ariel has some ideas for improving the situation (based on a Django extension: http://code.google.com/p/django-compress/) but I think some discussion is in order. In the meantime, I'll take a stab at just breaking things up a bit more with our current setup. > I think these two things are easy to fix for the more Django savvy team members. If you feel like wanting to take this on and have questions, please don't hesitate to contact me. > > -Andreas. > > [1] http://bit.ly/aov6xp >
Here's what I was thinking for an interim implementation of more
fine-grained JS includes:
http://github.com/dwins/geonode/commit/47f43fa54b87af8e980ec4005afd3023dce545b1
btw, if you want to play with this locally, do the following:
git remote add dwins git://github.com/dwins/geonode.git -f
# add 'dwins' as an alias for my github fork. the '-f' means "go ahead
and fetch from that repo now"
git checkout dwins/fewer-includes -b fewer-includes --track
# create a local branch (checkout -b) that you can work on that tracks
my github repo (eg, uses it for pull and merge)
There are two tricks going on here that I just didn't know about when we
wrote the current include system:
* {{ block.super }} renders the contents of the block that is being
overridden in a {% block foo %} section
* {% include "path" %} renders another template inline
So now we have four templates that do "something" for including ExtJS,
OpenLayers, GeoExt, and gxp + geonode's custom stuff, and can
selectively include whichever ones are needed in a page.
Playing with this exposed a couple of problems: lang.js doesn't
gracefully degrade when GeoExplorer is not in scope, and a number of our
other-wise non-spatial components are dependent upon GeoExt for similar
reasons. An example is the AME file upload dialog.
Anyway, just wanted to get some opinions before digging further on it.
--
David Winslow
OpenGeo - http://opengeo.org/
> Actually it's somewhat complicated to do and still support switching
> between minified and not. Ariel has some ideas for improving the
> situation (based on a Django extension:
> http://code.google.com/p/django-compress/) but I think some discussion
> is in order. In the meantime, I'll take a stab at just breaking things
> up a bit more with our current setup.
>
>
>> I think these two things are easy to fix for the more Django savvy team
members. If you feel like wanting to take this on and have questions,
please don't hesitate to contact me.
>>
>> -Andreas.
>>
>> [1] http://bit.ly/aov6xp
>>
>>
Hi, thanks David for looking into this! On Jun 25, 2010, at 00:27 , David Winslow wrote: > Here's what I was thinking for an interim implementation of more > fine-grained JS includes: > http://github.com/dwins/geonode/commit/47f43fa54b87af8e980ec4005afd3023dce545b1 > > btw, if you want to play with this locally, do the following: > git remote add dwins git://github.com/dwins/geonode.git -f > # add 'dwins' as an alias for my github fork. the '-f' means "go ahead > and fetch from that repo now" > git checkout dwins/fewer-includes -b fewer-includes --track > # create a local branch (checkout -b) that you can work on that tracks > my github repo (eg, uses it for pull and merge) Works great, but I have yet to figure out how to switch back to the main branch, and how to do merge in case one branch misses recent changes of the other. > There are two tricks going on here that I just didn't know about when we > wrote the current include system: > > * {{ block.super }} renders the contents of the block that is being > overridden in a {% block foo %} section > * {% include "path" %} renders another template inline > > So now we have four templates that do "something" for including ExtJS, > OpenLayers, GeoExt, and gxp + geonode's custom stuff, and can > selectively include whichever ones are needed in a page. This makes the whole template story much clearer! > Playing with this exposed a couple of problems: lang.js doesn't > gracefully degrade when GeoExplorer is not in scope, Good point. I have fixed this on trunk for all namespaces. Checking for GeoExplorer will fail, we have to check for window.GeoExplorer instead. > and a number of our > other-wise non-spatial components are dependent upon GeoExt for similar > reasons. An example is the AME file upload dialog. I think there should also be a template for ux. Right now the gxp template loads gxp and ux resources. In the case of AME upload, only ux is required, but not gxp. > Anyway, just wanted to get some opinions before digging further on it. My opinion? dwins++, and this is definitely the way to go. Thanks! Andreas. > > -- > David Winslow > OpenGeo - http://opengeo.org/ > >> Actually it's somewhat complicated to do and still support switching >> between minified and not. Ariel has some ideas for improving the >> situation (based on a Django extension: >> http://code.google.com/p/django-compress/) but I think some discussion >> is in order. In the meantime, I'll take a stab at just breaking things >> up a bit more with our current setup. >> >> >>> I think these two things are easy to fix for the more Django savvy team members. If you feel like wanting to take this on and have questions, please don't hesitate to contact me. >>> >>> -Andreas. >>> >>> [1] http://bit.ly/aov6xp >>> >>> > > -- Andreas Hocevar OpenGeo - http://opengeo.org/ Expert service straight from the developers.
On 06/25/2010 09:50 AM, Andreas Hocevar wrote: > I think there should also be a template for ux. Right now the gxp template loads gxp and ux resources. In the case of AME upload, only ux is required, but not gxp I broke things up this way, but the AME index page still fails to load its Ext grid if I omit gxp. I guess we will need to rearrange the build? I have updated my repo with the stuff I did so far (the embedded map viewers make a noticable difference even locally, I guess loading Ext once instead of twice is a big deal :) ) in case someone else wants to take a crack at it. Here is the current breakdown: http://github.com/dwins/geonode/tree/fewer-includes/src/GeoNodePy/geonode/templates/geonode/ maybe you had something else in mind? -- David Winslow OpenGeo - http://opengeo.org/
On 06/25/2010 12:54 PM, David Winslow wrote: > On 06/25/2010 09:50 AM, Andreas Hocevar wrote: > >> I think there should also be a template for ux. Right now the gxp template loads gxp and ux resources. In the case of AME upload, only ux is required, but not gxp >> > I broke things up this way, but the AME index page still fails to load > its Ext grid if I omit gxp. I guess we will need to rearrange the build? > > I have updated my repo with the stuff I did so far (the embedded map > viewers make a noticable difference even locally, I guess loading Ext > once instead of twice is a big deal :) ) in case someone else wants to > take a crack at it. > > Here is the current breakdown: > http://github.com/dwins/geonode/tree/fewer-includes/src/GeoNodePy/geonode/templates/geonode/ > maybe you had something else in mind? > > -- > David Winslow > OpenGeo - http://opengeo.org/ > I dug into this a bit today. I think that our dependencies are currently a bit too tangled for me to just peel out the non-spatial components; even the AME file browser uses pieces of GXP. So, any objections to just pushing what I have to the main repo? We can revisit the refactoring later, and maybe bring up some alternative tooling options. -d
Hey David, thanks for digging into this! And no objections against pushing what you already have into the main repo. If this breaks the AME upload page, I'll be glad to help fixing it. Just let me know. -Andreas. On Jun 28, 2010, at 21:36 , David Winslow wrote: > On 06/25/2010 12:54 PM, David Winslow wrote: >> On 06/25/2010 09:50 AM, Andreas Hocevar wrote: >> >>> I think there should also be a template for ux. Right now the gxp template loads gxp and ux resources. In the case of AME upload, only ux is required, but not gxp >>> >> I broke things up this way, but the AME index page still fails to load >> its Ext grid if I omit gxp. I guess we will need to rearrange the build? >> >> I have updated my repo with the stuff I did so far (the embedded map >> viewers make a noticable difference even locally, I guess loading Ext >> once instead of twice is a big deal :) ) in case someone else wants to >> take a crack at it. >> >> Here is the current breakdown: >> http://github.com/dwins/geonode/tree/fewer-includes/src/GeoNodePy/geonode/templates/geonode/ >> maybe you had something else in mind? >> >> -- >> David Winslow >> OpenGeo - http://opengeo.org/ >> > > I dug into this a bit today. I think that our dependencies are > currently a bit too tangled for me to just peel out the non-spatial > components; even the AME file browser uses pieces of GXP. > > So, any objections to just pushing what I have to the main repo? We can > revisit the refactoring later, and maybe bring up some alternative > tooling options. > > -d -- Andreas Hocevar OpenGeo - http://opengeo.org/ Expert service straight from the developers.
It doesn't break anything, it's just that pretty much everything relies on stuff that's in gxp (commonly gxp.util.dispatch()), which relies on stuff in geoext/ which relies on stuff in openlayers and extjs. So even for the AME page we need the full complement of JS libraries. Basically it will take a rewrite of the "non-spatial" widgets to make them really non-spatial. -d On 06/28/2010 04:21 PM, Andreas Hocevar wrote: > Hey David, > > thanks for digging into this! And no objections against pushing what you already have into the main repo. > > If this breaks the AME upload page, I'll be glad to help fixing it. Just let me know. > > -Andreas. > > On Jun 28, 2010, at 21:36 , David Winslow wrote: > > >> On 06/25/2010 12:54 PM, David Winslow wrote: >> >>> On 06/25/2010 09:50 AM, Andreas Hocevar wrote: >>> >>> >>>> I think there should also be a template for ux. Right now the gxp template loads gxp and ux resources. In the case of AME upload, only ux is required, but not gxp >>>> >>>> >>> I broke things up this way, but the AME index page still fails to load >>> its Ext grid if I omit gxp. I guess we will need to rearrange the build? >>> >>> I have updated my repo with the stuff I did so far (the embedded map >>> viewers make a noticable difference even locally, I guess loading Ext >>> once instead of twice is a big deal :) ) in case someone else wants to >>> take a crack at it. >>> >>> Here is the current breakdown: >>> http://github.com/dwins/geonode/tree/fewer-includes/src/GeoNodePy/geonode/templates/geonode/ >>> maybe you had something else in mind? >>> >>> -- >>> David Winslow >>> OpenGeo - http://opengeo.org/ >>> >>> >> I dug into this a bit today. I think that our dependencies are >> currently a bit too tangled for me to just peel out the non-spatial >> components; even the AME file browser uses pieces of GXP. >> >> So, any objections to just pushing what I have to the main repo? We can >> revisit the refactoring later, and maybe bring up some alternative >> tooling options. >> >> -d >> >
With +1's on this from Andreas and Ariel, I will go ahead and apply similar changes to the "fullscreen" apps, split "ux" off into a separate include, and remove usage of iframes for internal embedded maps. I'll notify this list when this is done. -- David Winslow OpenGeo - http://opengeo.org/ On 06/25/2010 09:50 AM, Andreas Hocevar wrote: > Hi, > > thanks David for looking into this! > > On Jun 25, 2010, at 00:27 , David Winslow wrote: > > >> Here's what I was thinking for an interim implementation of more >> fine-grained JS includes: >> http://github.com/dwins/geonode/commit/47f43fa54b87af8e980ec4005afd3023dce545b1 >> >> btw, if you want to play with this locally, do the following: >> git remote add dwins git://github.com/dwins/geonode.git -f >> # add 'dwins' as an alias for my github fork. the '-f' means "go ahead >> and fetch from that repo now" >> git checkout dwins/fewer-includes -b fewer-includes --track >> # create a local branch (checkout -b) that you can work on that tracks >> my github repo (eg, uses it for pull and merge) >> > Works great, but I have yet to figure out how to switch back to the main branch, and how to do merge in case one branch misses recent changes of the other. > > >> There are two tricks going on here that I just didn't know about when we >> wrote the current include system: >> >> * {{ block.super }} renders the contents of the block that is being >> overridden in a {% block foo %} section >> * {% include "path" %} renders another template inline >> >> So now we have four templates that do "something" for including ExtJS, >> OpenLayers, GeoExt, and gxp + geonode's custom stuff, and can >> selectively include whichever ones are needed in a page. >> > This makes the whole template story much clearer! > > >> Playing with this exposed a couple of problems: lang.js doesn't >> gracefully degrade when GeoExplorer is not in scope, >> > Good point. I have fixed this on trunk for all namespaces. Checking for GeoExplorer will fail, we have to check for window.GeoExplorer instead. > > >> and a number of our >> other-wise non-spatial components are dependent upon GeoExt for similar >> reasons. An example is the AME file upload dialog. >> > I think there should also be a template for ux. Right now the gxp template loads gxp and ux resources. In the case of AME upload, only ux is required, but not gxp. > > >> Anyway, just wanted to get some opinions before digging further on it. >> > My opinion? dwins++, and this is definitely the way to go. > > Thanks! > Andreas. > > >> -- >> David Winslow >> OpenGeo - http://opengeo.org/ >> >> >>> Actually it's somewhat complicated to do and still support switching >>> between minified and not. Ariel has some ideas for improving the >>> situation (based on a Django extension: >>> http://code.google.com/p/django-compress/) but I think some discussion >>> is in order. In the meantime, I'll take a stab at just breaking things >>> up a bit more with our current setup. >>> >>> >>> >>>> I think these two things are easy to fix for the more Django savvy team members. If you feel like wanting to take this on and have questions, please don't hesitate to contact me. >>>> >>>> -Andreas. >>>> >>>> [1] http://bit.ly/aov6xp >>>> >>>> >>>> >> >> >
On Thu, Jun 24, 2010 at 5:27 PM, David Winslow <dwinslow@opengeo.org> wrote:
[snip]
> Anyway, just wanted to get some opinions before digging further on it.
David, that solution looks great, I also like the fact that the
"geonode_media" template tag is not needed anymore in a a few places
and used mostly in the headers templates.
Right on, the templates look much cleaner now.
Ariel.
On Jun 24, 2010, at 22:17 , David Winslow wrote: > On 06/24/2010 04:09 PM, Andreas Hocevar wrote: >> During the code review accompanying the viewer/composer refactoring, I noticed that page loading times can be reduced by two simple measures: >> >> 1. Do not use IFrames to embed maps >> =================================== >> >> As already done in the SearchTable component [1] (props to ltucker for the great work on this), it is possible to integrate maps directly into web pages. All that needs to be done is create a GeoExplorer(.Viewer) instance/subclass with a proper portalConfig, and Django writing the configuration to the main page's template directly. >> > Yep, sounds great. We do need to keep the iframe viewer around to > support embedding maps in outside web pages though, right? Or do we > have some new embeddable viewer based on jsonp or something? Right now we always use a Django template which creates the viewer instance. This is not so bad IMHO (compared to JSONP or using XHR to load the config), because it saves us an extra request. The embeddable viewer to use in outside web pages is embed.html, which is only very little code. But yes, getting rid of the iframe does not save us from providing a standalone embeddable viewer. Including a viewer directly in an inside web page should work like this: <html> <head> <!-- css and js resources required for the map --> <!-- create a viewer instance --> <script type="text/javascript"> var app; Ext.onReady(function() { var config = Ext.apply({ useToolbar: false, proxy: "/proxy/?url=", rest: "/maps/", // tell the map viewer where and how to be rendered portalConfig: { height: 300, renderTo: "embedded_map" } }, {{config|safe}}); app = new GeoExplorer.Viewer(config); }); </script> </head> <body> <p>This is a nice map</p> <div id="embedded_map"></div> <p>The map shows blah blah blah</p> </body> </html> The only difference from the current iframe way is that the configuration is included in the page directly, and we have an "embedded_map" div instead of an iframe that references a Django endpoint with a configured map. > >> 2. Do not load unneeded JS and CSS resources >> ============================================ >> >> Currently, every page generated by Django uses a template that includes all JS and CSS resources. The extreme case is the start page: it does not need any of it, but loads all. And there is an IFrame loaded by the page which loads the same components itself. >> > Actually it's somewhat complicated to do and still support switching > between minified and not. Ariel has some ideas for improving the > situation (based on a Django extension: > http://code.google.com/p/django-compress/) but I think some discussion > is in order. In the meantime, I'll take a stab at just breaking things > up a bit more with our current setup. Thanks! One idea would be to use minified resources by default, and only use debug resources if ?debug=true is appended to the url. Would django be able to handle this? -Andreas. > >> I think these two things are easy to fix for the more Django savvy team members. If you feel like wanting to take this on and have questions, please don't hesitate to contact me. >> >> -Andreas. >> >> [1] http://bit.ly/aov6xp >> > -- Andreas Hocevar OpenGeo - http://opengeo.org/ Expert service straight from the developers.