Bug 5677 – speed up KWQ timers by using CFRunLoopTimer instead of NSTimer

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Darin Adler]

I saw some NSTimer and Objective-C overhead when profiling benchmarks that can be eliminated. I'm 
working on a patch that speeds things up and also simplifies how the back/forward cache suspends and 
resumes timers.

I landed this once, but it caused regressions so we have to work on this more and test a bit more.

[Darin Adler]

Created attachment 4640 [details]
version that seems to work well

I'm not sure exactly what to do about this. I can't reproduce any problems with
this version of the patch, but Tim said he had big problems with Dashboard. I
can't seem to test this with Dashboard successfully. Someone else may have to
test and land this.

I think the speedup and simplification is worth it, and there might even be
some bug fixes in the cleaned up back/forward code.

[Darin Adler]

Comment on attachment 4640 [details]
version that seems to work well

Tim already reviewed this once -- I think it's time to review, test, and land
this version.

[Eric Seidel (no email)]

Comment on attachment 4640 [details]
version that seems to work well

Shouldn't PausedTimeouts create and destroy the actual array?  Or maybe it
should take them, but construction should be the last step in the fuction. 
Would be more obvious if we had PassRefPtr, but we don't yet.

Also, I'm curious as to why you might wish to use a raw c array and length
pointer (for the paused timeouts array), instead of something like a QMemArray
which would hold the two together for you?

Wow.  That was a big patch.  And I don't feel like my brain managed to soak in
every line of code change, but in general it looke sane, besides the manual
memory management noted above.

[Darin Adler]

(In reply to comment #3)
> (From update of attachment 4640 [details] [edit])
> Shouldn't PausedTimeouts create and destroy the actual array?  Or maybe it
> should take them, but construction should be the last step in the fuction. 
> Would be more obvious if we had PassRefPtr, but we don't yet.

The PausedTimeouts class's destructor does destroy the actual array, but the constructor doesn't create it -- instead it 
adopts it. That's for efficiency but maybe there's a better way to do it.

I'm not sure I understand your question. PassRefPtr wouldn't help much since the array is not a reference-counted object.

> Also, I'm curious as to why you might wish to use a raw c array and length
> pointer (for the paused timeouts array), instead of something like a QMemArray
> which would hold the two together for you?

A QMemArray would be OK, but it has extra overhead to accomodate resizing the array. This is a fixed-size array and we 
don't have a class that implements that.

[Eric Seidel (no email)]

(In reply to comment #4)
> (In reply to comment #3)
> The PausedTimeouts class's destructor does destroy the actual array, but the constructor doesn't 
create it -- instead it 
> adopts it. That's for efficiency but maybe there's a better way to do it.
> 
> I'm not sure I understand your question. PassRefPtr wouldn't help much since the array is not a 
reference-counted object.

A momentary lapse of judgement. :)

> > Also, I'm curious as to why you might wish to use a raw c array and length
> > pointer (for the paused timeouts array), instead of something like a QMemArray
> > which would hold the two together for you?
> 
> A QMemArray would be OK, but it has extra overhead to accomodate resizing the array. This is a 
fixed-size array and we 
> don't have a class that implements that.

Yes, that's a shame.  The encapsulation may still be worth the overhead.

[Darin Adler]

Comment on attachment 4640 [details]
version that seems to work well

This patch was missing at least one required nil check.