[dev] [commits] Horde branch master updated. 89bca7f749db1780dca80363d6f3d8c89f899df4
Michael M Slusarz
slusarz at horde.org
Wed Mar 10 05:20:25 UTC 2010
Quoting Jan Schneider <jan at horde.org>:
> Zitat von Michael M Slusarz <slusarz at horde.org>:
>> commit 97fd203dcaaf5ddf7c9c2252eb44ac76e8d84846
>> Author: Michael M Slusarz <slusarz at curecanti.org>
>> Date: Sun Mar 7 20:15:31 2010 -0700
>> Add Horde_Themes:: library.
>> Move all themes related code to a single place (includes both image and
>> Horde::img(), through Horde_Themes::img(), will now attempt to auto
>> determine a graphic location. It looks in the following directories with
>> this priority:
>> 1. App theme
>> 2. Horde theme
>> 3. App default
>> 4. Horde default
>> This prevents us from having to copy all over all graphics for a theme -
>> now, only need the graphics that differ from defaults (caveat: if
>> loading graphics from CSS, copies of graphics need to continue to live
>> in theme directory).
> Please revert, at least the new autodetection code. Using
> themed_graphics and caching it in the session is absolutely
> necessary for perfomance reasons. Without that we are filestat'ing
> for directories and images in *every* img() call, which puts a huge
> burden on the filesystem IO, compared to the once-per-session
> stat'ing of themed_graphics.
> This was added after benchmarking massive performance degration when
> introducing themes with graphics.
I was not aware of the crippling IO issue. My next job was going to
be to save the image data via Horde_Cache (similar to how we cache CSS
scripts). As mentioned in the past, it has already become almost
mandatory to have a Horde_Cache driver installed for various Horde
functions for any sort of decent performance, so this is not a
But I am open to other ways of making this behavior better. But not
returning to the old behavior, which was defective in many ways:
1) Absolute overkill when it comes to having to copy *every* graphic
to *every* theme that wants to add 1 graphic.
2) #2 makes it impossible to reliably determine which graphics have
been overriden in a theme.
3) #2 makes it virtually impossible to keep themes up-to-date.
4) It was impossible to override a graphic via an app theme if the
Horde::img() call specifically referenced a horde graphic (which sort
of defeats the whole point of themes).
5) There is no concept of default graphics imported from horde. This
is counter-intuitive as compared with the way CSS files work.
However, I'll admit that there are powerful incentives to require
*all* graphics used by an app to have a physical copy of the app (i.e.
an app never loads graphics from horde). First, this makes the
applications more self-contained. More importantly, there is no
practical way to load graphics from CSS files outside of making it an
absolute requirement that apps MUST live under Horde.
Brainstorming, I have the following ideas:
1) Caching as described above.
Pros: automatically determined (themes are drop-in replaceable).
Cons: adds a bit of overhead the first time a graphic is loaded;
fallback to file stat() calls on Horde_Cache. As mentioned
previously, the latter issue is not critical IMHO.
2) Add theme definition script. The idea: after changing theme
graphics, this script is run and it produces a PHP data file that
contains the graphics list for the theme (i.e. which location each
graphic of the theme is located.)
Pro: Arguably identical overhead than the old method - instead of
doing a file_exists() call, we simply include 1 or 2 PHP files per
Cons: This script would have to be run manually whenever a graphic is
changed. However, themes/graphics theoretically won't be changed all
that often, and this script could be called as part of the build
process. Additionally, this is no more burdensome than the previous
method (requiring calling a script to determine which graphics are
missing from a particular theme).
Michael Slusarz [slusarz at horde.org]
More information about the dev