On Mon, 11 Oct 2010 at 11:52:36 -0400, Brad Jorsch wrote:
> On Mon, Oct 11, 2010 at 12:17:06PM +0200, Carlos R. Mafra wrote:
> >
> > It feels better to have a light code path during app-openning time and
> > do only the strictly necessary stuff. So I prefer to avoid having to rewrite
> > the icon every single time.
> >
> > I think we can go one step further and my patch is one possible attempt
> > at that. I'll probably commit it.
>
> I did a little testing, the slowest part is re-saving WMWindowAttributes
> when it hasn't been changed, which is easy to check for. The attached
> patch fixes that nicely.
Well spotted. There is no point in saving WMWindowAttributes if no changes are
expected.
But the 3 lines of patch context is not sufficient to get the whole picture,
so when reading that function I realized that with your patch applied the
precise value of 'val' computed in the "if (adict)" case is not really needed,
so it becomes superfluous to compute it.
That suggests the diff below instead of your patch. It compiles here, but
I haven't tested it. But it saves one call to WMGetFromPLDictionary() and
accomplishes what your original patch did too, unless I am missing something.
diff --git a/src/application.c b/src/application.c
index fad9186..a093bc2 100644
--- a/src/application.c
+++ b/src/application.c
@@ -152,20 +152,18 @@ void wApplicationSaveIconPathFor(char *iconPath, char
*wm_instance, char *wm_cla
iconk = WMCreatePLString("Icon");
- if (adict) {
- val = WMGetFromPLDictionary(adict, iconk);
- } else {
+ if (!adict) {
/* no dictionary for app, so create one */
adict = WMCreatePLDictionary(NULL, NULL);
WMPutInPLDictionary(dict, key, adict);
WMReleasePropList(adict);
- val = NULL;
- }
- if (!val) {
val = WMCreatePLString(iconPath);
WMPutInPLDictionary(adict, iconk, val);
WMReleasePropList(val);
+ } else {
+ val = NULL;
}
+
WMReleasePropList(key);
WMReleasePropList(iconk);
> I doubt anyone would really notice the remaining hundredths of a second
> you save unless they were looking for it. And if they really do, it can
> be cut down to microseconds by specifying a non-cached icon instead of
> using the cached version (in which case it doesn't update the cache at
> all).
Well, nowadays it's never a matter of how many microseconds you save.
CPU's are quite fast and do many things in almost no time.
It's a matter of trying to do things as cleanly as possible. And saving
the icon everytime you open a docked app because the icon might have
changed does not seem to be elegant, IMHO.
But hey, your series already made the whole thing more elegant anyway.
I am not complaining about that. I am just not a fan of creating hard disk
activity if we can avoid it.
> diff --git a/src/application.c b/src/application.c
> index fad9186..f3993ff 100644
> --- a/src/application.c
> +++ b/src/application.c
> @@ -165,6 +165,8 @@ void wApplicationSaveIconPathFor(char *iconPath, char
> *wm_instance, char *wm_cla
> val = WMCreatePLString(iconPath);
> WMPutInPLDictionary(adict, iconk, val);
> WMReleasePropList(val);
> + } else {
> + val = NULL;
> }
> WMReleasePropList(key);
> WMReleasePropList(iconk);
--
To unsubscribe, send mail to [email protected].
