Check out : http://lxr.mozilla.org/seamonkey/source/modules/libimg/gifcom/gif.cpp#1517 The problem is that 0 length delays are always being set to MINIMUM_DELAY_TIME (10). They should actually be going down the GETN(1, gif_image_start); path. This is causing tons of unecessary timers to be created. Here is the correct code: if (gs->delay_time){ if(gs->delay_time < MINIMUM_DELAY_TIME ) gs->delay_time = MINIMUM_DELAY_TIME; if(ic->imgdcb){ gs->delay_timeout = (void *) ic->imgdcb->ImgDCBSetTimeout(gif_delay_time_callback, gs->ic, gs->delay_time); } /* Essentially, tell the decoder state machine to wait forever. The timeout callback routine will wake up the state machine and force it to decode the next image. */ GETN(1L<<30, gif_image_start); gs->state = gif_delay; } else { GETN(1, gif_image_start); } The if check for MINIMUM_DELAY_TIME should be in the second if.

I'm putting an RTM+ in here because someone in Netscape land needs to look at this before shipping Netscape 6. This is a genuine performance bug.

Removing {rtm+] from status - that should only be added once you have r/sr. Looks right, and hasn't caused problems for the couple days I've had it in my tree. pnunn, could you review this?

At one time there was a Good Reason for the delay time to always be set at the minimum value. But that was the old code base. Time(rs) have changed. I'll need to pound on this one alittle to verify its goodness. -p

mkaply mailed me enquiring about when this might get r=/sr= (I think he composed his mail to me before pnunn's update of today). I don't know, but I thought I'd ask in the bug. /be

How bad is the performance problem? Can we go ahead with reviews? (If this doesn't happen ASAP, it's going to be minus by default because the RTM will have shipped.)

PDT marking [rtm-]. Looks like this patch has been languishing.

Yes, the patch is languishing. Pam, can we check it into the trunk so that everyone can pound on it? /be

I've been testing the patch in my local tree and it works fine (linux & NT). There was a particular reason why a minimun delay time was _always_ set before the timer code was changed. It appears a minimum of zero is now ok. This patch also appears to help fix or alleviate #50786, where the clean up after end of stream wins (or loses ) a race with the clean up after a bad image and crashes. r:pnunn for trunk who gives an sr: ?

I reviewed this earlier, and while reviewing bug 53597, and even whined about the non-four-space indentation at the deepest nesting level shown in the patch. Anyway, sr=brendan@mozilla.org. /be

*** Bug 58071 has been marked as a duplicate of this bug. ***

Make that bug#58071 (not 50786). (so thats what that loud, constant background noise is...)

My trunk build isn't up to date. Anyone have a fresh tree want to check this in? -p

Since it was originally mine, I'll check it in.

much thanks.

Fix checked in. And I fixed a couple of the bad indentations that brendan was talking about.

Coding issue, marking verified

MK: would you back out this patch for the minimum timer delay? See bug#59494. -pn

All pnunn bugs reassigned to Pav, who is taking over the imglib.

New imagelib doesn't use timers for anything except animations. old code no longer used. marking fixed.

Verified