169247 - Investigate allocating gecko events (nsIRunnable impls) using a recycler

Status: RESOLVED INCOMPLETE

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[Peter Lubczynski]

from bug 132759:

Avoiding unneeded malloc's
==========================
The patches I have seen so far put all Flash messages on the PLEvent queue. This 
causes us to use a malloc'd class to hold the event info. In general I prefer to 
avoid malloc'ing when not necessary. For Flash messages which are projected to 
come in at 120 messages per second per Flash area I really would strongly prefer 
not to malloc. For example, the www.sina.com.cn page today has 9 Flash animations
which if there were 120 messages per second would mean 1080 malloc's per second. 
I believe that to avoid malloc'ing for every message we  will need to test for 
starvation locally and only defer the message when starvation is happening.
Perhaps we can reuse the PLEvent starvation logic locally.

"Avoiding unneeded malloc's"

this is probably an issue for Mozilla timers as well since they are implemented
as a separate thread which posts a PL_Event when the timer expires. DHTML
animations often have at least one timer and some have many timers running
simultaneously. Maybe we need to create an arena for PL_Events?


----------------------------------
This bug is being opened to investigate if using a custom allocator using areans
for PLEvents would help performance.

Comment 1

[Brendan Eich [:brendan]]

you want more than an arena, you also need a freelist.  The combination is often
called a "recycler", and is implemented generally for more than one size class
by nsFixedSizeAllocator
(http://lxr.mozilla.org/mozilla/source/xpcom/ds/nsFixedSizeAllocator.h).  The
downside is that you may tie up too much memory in the idle or least-use case,
so tune the arena chunk size well.

/be

Comment 2

[Markus Hübner]

Is this something that can be addressed for 1.4 ?

Comment 3

[Markus Hübner]

Maybe this is also related to bug 210602 comment #1

Comment 4

[Alec Flett]

this is a cool idea.. but would this need to be threadsafe? Or are we talking
about events a thread posts to itself? We also have nsRecyclingAllocator which
will avoid free()'ing until a timer goes off.

Comment 5

[Darin Fisher]

hmm... up until recently, PLEvents had a much larger footprint (see bug 204962),
so maybe this bug is less significant or perhaps the overhead of a recycling
allocator for PLEvents would be significantly less.

Comment 6

[David Bradley]

I've attached a patch in bug 213813 that might help with some of these DHTML
issues. I don't expect tremendous gains, but it might be a small step in
improving performance. I'm going to try and get some numbers, but I wanted to
get this out to others that might have more time than I have current. The patch
is at http://bugzilla.mozilla.org/attachment.cgi?id=128862&action=edit

Comment 7

[Pham]

.

Comment 8

[Jo Hermans]

Is this still planned for the future, or is it a lost cause ?

Comment 9

[Brendan Eich [:brendan]]

We need evidence that this bug is worth "fixing".  Profiling cycle and memory
use would be helpful.  Comment 0 on its face is not convincing -- avoiding
malloc is a good way to hoard memory in private heaps, reinvent various
thread-safety and memory pressure wheels, etc.

/be

Comment 10

[Boris Zbarsky [:bzbarsky]]

I haven't run into PLEvent allocation as an issue in DHTML per se (haven't seen
it show up in profiles).  I just checked some profiles I have lying about, and
this is pretty typical:

Under PresShell::PostReflowEvent, the breakdown of time spent is the following:

 220 hits in the function itself.
3807 nsEventQueueServiceImpl::GetSpecialEventQueue(int, nsIEventQueue**)
 108 nsCOMPtr_base::~nsCOMPtr_base()
  11 nsEventQueueServiceImpl::GetThreadEventQueue(PRThread*, nsIEventQueue**)
  10 nsCOMPtr_base::begin_assignment()
   8 nsEventQueueImpl::PostEvent(PLEvent*)
   7 nsEventQueueImpl::Release()
   4 ReflowEvent::ReflowEvent(nsIPresShell*)
   1 nsCOMPtr_base::assign_with_AddRef(nsISupports*)

So the actual event creation is at most 5% of the time needed to post the event.
 More likely, it's below 1%...  Similarly, under
nsCSSFrameConstructor::PostRestyleEvent only 2 hits out of 7800 show |operator
new| for the event.  The rest are doing other things.  That's about 0.1% of
total time.

All that said, the issue this was initially filed on is not at all DHTML
related; I don't know why "DHTML" was added to the summary (so removing it).

Back to the original issue in this bug, can someone point me to a flash page
that's actually... like using CPU or something?  The website cited in comment 0
(http://www.sina.com.cn) uses exactly 0 CPU over here (current trunk Linux
build).  So based on that page, this PLEvent thing is quite simply a non-issue.

Comment 11

[Darin Fisher]

Morphing this bug to cover nsIRunnable objects, which now replace PLEvent (see bug 326273).  Unfortunately, now we would only be looking at recycling certain nsIRunnable implementations.

Comment 12

[Benjamin Smedberg]

If this is an issue (especially with timer events), I'd be interested in using a slab-like allocator for these objects.

Comment 13

[Mihail Daskalov]

(In reply to comment #10)
...
> Back to the original issue in this bug, can someone point me to a flash page
> that's actually... like using CPU or something?
...

see bug 341380

Comment 14

[Boris Zbarsky [:bzbarsky]]

That doesn't show any event allocations in the profile, just lots of X/GTK stuff.