Modify

Opened 4 years ago

Last modified 9 months ago

#7617 assigned defect

datefield.js is not under the chrome directory

Reported by: andrew@… Owned by: rjollos
Priority: normal Component: DateFieldPlugin
Severity: normal Keywords: JavaScript htdocs location virtual
Cc: hasienda Trac Release: 0.12

Description (last modified by rjollos)

This makes it more difficult to classify datefield.js as a cacheable static resource.

The url is currently: /trac_env/datefield/datefield.js
The optimal would be: /trac_env/chrome/datefield/datefield.js

Attachments (6)

DateFieldNotUnderChrome.png (16.0 KB) - added by rjollos 3 years ago.
DateFieldPluginNotCachingResource.png (48.6 KB) - added by rjollos 3 years ago.
th7617-Trac0.12.patch (5.8 KB) - added by rjollos 3 years ago.
ExplicitRefresh.png (39.7 KB) - added by anonymous 3 years ago.
FromCache.png (19.3 KB) - added by anonymous 3 years ago.
OccasionallyOnNavigation.png (43.1 KB) - added by rjollos 3 years ago.

Download all attachments as: .zip

Change History (26)

comment:1 Changed 3 years ago by rjollos

I'm not sure that DateFieldPlugin has control over this. Isn't this under the control of Trac? DateFieldPlugin just calls add_script. Do you have a suggested fix?

comment:2 Changed 3 years ago by debacle@…

With Apache, I believe this works:

# matches trac static files
AliasMatch ^/(.+)/chrome/common/(.*) /usr/share/pyshared/trac/htdocs/$2
# matches trac plugins static files, e.g. datefields
AliasMatch ^/(.+)/chrome/([^/]*)/(.*) /usr/share/pyshared/$2/htdocs/$3

comment:3 Changed 3 years ago by rjollos

  • Cc hasienda added
  • Description modified (diff)
  • Resolution set to invalid
  • Status changed from new to closed

I don't think this is a plugin issue, but rather an installation issue. Please reopen if this is not the case.

comment:4 Changed 3 years ago by rjollos

  • Resolution invalid deleted
  • Status changed from closed to reopened

Changed 3 years ago by rjollos

comment:5 Changed 3 years ago by rjollos

Okay, yes I see what you are talking about now:


comment:6 follow-up: Changed 3 years ago by rjollos

From trac.web.chrome:

def add_script(req, filename, mimetype='text/javascript'):
    """Add a reference to an external javascript file to the template.
    
    If the filename is absolute (i.e. starts with a slash), the generated link
    will be based off the application root path. If it is relative, the link
    will be based off the `/chrome/` path.
    """

From post_process_request in datefield.filter:

add_script(req, 'datefield/js/jquery-ui-1.8.16.custom.min.js')
# virtual script
add_script(req, '/datefield/datefield.js')
add_stylesheet(req, 'datefield/css/jquery-ui-1.8.16.custom.css')

Is there any reason that datefield starts with a slash? Does this need to be treated special because it is a virtual script. I'm not sure what that means.

Changed 3 years ago by rjollos

comment:7 in reply to: ↑ 6 Changed 3 years ago by rjollos

Replying to rjollos:

Is there any reason that datefield starts with a slash? Does this need to be treated special because it is a virtual script. I'm not sure what that means.

My interpretation is that it is considered a virtual because since the javascript is contained in the datefield.html template. I see the same pattern in the code for EstimationToolsPlugin. Based on reading code and experimenting, I'm also assuming there is a reason that a virtual script can't be located under /chrome.

My motivation for investigating this issue is that the datefield.js virtual script seems to be one of the big contributors to my page load times.


Last edited 12 months ago by rjollos (previous) (diff)

comment:8 follow-up: Changed 3 years ago by hasienda

Had a hard time too before I understood, what's going on there, and especially why.

As I see it, this "virtual script" approach has been chosen to bring in all the variables from Python to JavaScript. But since Trac-0.12dev-r8884 we have add_script_data, and we should possibly backport this to 0.11 to get out of this rather insane situation.

Interesting profiling results. This underlines how much more preferable it is to add a JS file than to add a XHTML template. JS is seemingly light-fast, while (X)HTML is not. I may guess wrong, but don't you think too, that this is most likely due to Genshi's template processing? I bet, changing this to a JS file and adding data with add_script_data will save you roughly a second.

comment:9 in reply to: ↑ 8 ; follow-up: Changed 3 years ago by rjollos

Replyng to hasienda:

.. and we should possibly backport this to 0.11 to get out of this rather insane situation.

Are you suggesting that the Trac team needs to backport add_script_data and create Trac 0.11.8? Or is there something we could do ourselves? I read a comment by cboos in which he said that barring a major security issue, there will be no more releases on the 0.11.x line.

JS is seemingly light-fast, while (X)HTML is not. I may guess wrong, but don't you think too, that this is most likely due to Genshi's template processing? I bet, changing this to a JS file and adding data with add_script_data will save you roughly a second.

I don't know enough to even venture a guess, but I will definitely work up a patch today and retest to try to prove your theory. Thank you for your help on this!

If this works, there is potential for speedup on my site by making similar changes to the QuietPlugin.

comment:10 in reply to: ↑ 9 ; follow-up: Changed 3 years ago by hasienda

Replying to rjollos:

Replyng to hasienda:

.. and we should possibly backport this to 0.11 to get out of this rather insane situation.

Are you suggesting that the Trac team needs to backport add_script_data and create Trac 0.11.8? Or is there something we could do ourselves? I read a comment by cboos in which he said that barring a major security issue, there will be no more releases on the 0.11.x line.

Oh, sorry. My wording was misleading here. I know that there's no more work on 0.11, but it's up to us to provide enhanced methods that are not available to older Trac version inside a plugin.

JS is seemingly light-fast, while (X)HTML is not. I may guess wrong, but don't you think too, that this is most likely due to Genshi's template processing? I bet, changing this to a JS file and adding data with add_script_data will save you roughly a second.

I don't know enough to even venture a guess, but I will definitely work up a patch today and retest to try to prove your theory. Thank you for your help on this!

I was lucky to talk to Carsten Klein who did in-deep research in order to improve Genshi in 2009/10. IIRC there's a lot of regenerating objects (deep copy) and recursion in the inner loop of this template engine. And it's rather resource hungry and slow compared to other current alternatives. Still it's versatile and well accepted in the Trac domain, and the discussion about a move towards Jinja2 has never heated again.

So I'm looking forward to your findings.

Changed 3 years ago by rjollos

comment:11 Changed 3 years ago by rjollos

I've attached a patch which seems to work fine in my development environment. I'm going to do some more testing this evening, and then maybe push this to a dev line in the t-h.o repository.

comment:12 in reply to: ↑ 10 ; follow-up: Changed 3 years ago by rjollos

Replying to hasienda:

Replying to rjollos:

Replyng to hasienda:

.. and we should possibly backport this to 0.11 to get out of this rather insane situation.

Are you suggesting that the Trac team needs to backport add_script_data and create Trac 0.11.8? Or is there something we could do ourselves? I read a comment by cboos in which he said that barring a major security issue, there will be no more releases on the 0.11.x line.

Oh, sorry. My wording was misleading here. I know that there's no more work on 0.11, but it's up to us to provide enhanced methods that are not available to older Trac version inside a plugin.

Yeah, that is a good idea. In fact, we could make a compatibility class that contains javascript_quote, add_script_data and whatever else we might want to utilize in the future. This script could reside in a single location in the t-h.o repository, and be pulled in to the plugins that need it using an svn:externals property. At a minimum, you and I would probably utilize this in half a dozen plugins. This could be a project on trac-hacks, and the project page could be a place to catalog good practices for making plugin source code work across 0.11 - 0.13dev releases.

comment:13 Changed 3 years ago by rjollos

  • Owner changed from doki_pen to rjollos
  • Status changed from reopened to new

Referenced the wrong ticket.

(In [11247]) Refs #9763 (2.0.0dev): Replaced the virtual script (template) datefield.js with a true script. This was possible now that add_script_data is available in Trac 0.12.

Changed 3 years ago by anonymous

Changed 3 years ago by anonymous

Changed 3 years ago by rjollos

comment:14 follow-up: Changed 3 years ago by rjollos

If I navigate around the site, usually I get this result:


Occasionally I get this result:


If I do an explicit page refresh I get:


What I am seeing is that jquery-ui-1.8.16.custom.min.js will be loaded from the cache, except for the case of a page refresh. I'm not sure why datefield.js is usually, but not always loaded from the cache. Any ideas? It's not a big deal though, I'm seeing big improvements from this.

Last edited 12 months ago by rjollos (previous) (diff)

comment:15 Changed 3 years ago by rjollos

Opened #9764, hoping we can do the same thing for the QuietPlugin and maybe I'll get some further improvement in page load times. I'm seeing serious performance issues with permissions checking in the PrivateTicketsPlugin though (4-5 seconds added to /newticket and /ticket), so I'll have to shift my attention to that for the time being.

I'd really appreciate if anyone is able to do testing and give feedback on this.

comment:16 Changed 3 years ago by rjollos

Also opened #9765 for possible similar issue with TracTicketTemplatePlugin.

comment:17 in reply to: ↑ 12 Changed 3 years ago by hasienda

  • Keywords JavaScript htdocs location virtual added

Replying to rjollos:

Replying to hasienda:

Replying to rjollos:

Replyng to hasienda:

.. and we should possibly backport this to 0.11 to get out of this rather insane situation.

Are you suggesting that the Trac team needs to backport add_script_data and create Trac 0.11.8? Or is there something we could do ourselves? I read a comment by cboos in which he said that barring a major security issue, there will be no more releases on the 0.11.x line.

Oh, sorry. My wording was misleading here. I know that there's no more work on 0.11, but it's up to us to provide enhanced methods that are not available to older Trac version inside a plugin.

Yeah, that is a good idea. [...]

This could be a project on trac-hacks, and the project page could be a place to catalog good practices for making plugin source code work across 0.11 - 0.13dev releases.

Pretty much the same as I've been thinking for weeks. A place for good practices, code snippets for common tasks, etc. Glad, you think the same way. Right, I'll push this too, and as soon as there is something visible, announce it to fellow developers on the mailing-list as well.

comment:18 in reply to: ↑ 14 ; follow-up: Changed 3 years ago by hasienda

Replying to rjollos:

What I am seeing is that jquery-ui-1.8.16.custom.min.js will be loaded from the cache, except for the case of a page refresh. I'm not sure why datefield.js is usually, but not always loaded from the cache. Any ideas? ...

Might be a browser cache optimization effect. The cache size is depending on your settings, but always limited. To maximize effect, a cache cleanup policy could include purging small files faster than large ones, because re-getting them wouldn't hurt that much anyway. datefield.js is quite small, in fact one of the smallest pieces in this puzzle, right?

But this is just an "informed guess" from what I've read about HTTP caching strategies.

comment:19 in reply to: ↑ 18 Changed 3 years ago by rjollos

Replying to hasienda:

... because re-getting them wouldn't hurt that much anyway. datefield.js is quite small, in fact one of the smallest pieces in this puzzle, right?

Yeah, pretty small. When I do more profiling I'll pay attention whether the same behavior exists for other small JS files.

comment:20 Changed 9 months ago by rjollos

  • Status changed from new to assigned

Add Comment

Modify Ticket

Action
as assigned .
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.