65266 - Allocate Style Contexts from arenas: complete analysis

Status: VERIFIED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Marc Attinasi]

StyleContexts come and go frequently in many web page interaction scenarios, an
in some intensive DHTML pages. By using arenas, we may be able to speed up style
resolution in these cases. The affects on overall footprint need to be better
understood, as well.

The URL is inside the ns firewall, I'll update it when it is pushed to the
public site.

(note: this is a placeholder for work already started by back-burnered for a
little while.)

Comment 1

[Marc Attinasi]

Accepting.

Comment 2

[Marc Attinasi]

Attachment: Patch that puts style contexts and style context data into arenas (if #define POOL_STYLECONTEXTS is uncommented in nsIStyleSet.h)

Comment 3

[Hixie (not reading bugmail)]

Netscape's standard compliance QA team reorganised itself once again, so taking 
remaining non-tables style bugs. Sorry about the spam. I tried to get this done 
directly at the database level, but apparently that is "not easy because of the 
shadow db", "plus it screws up the audit trail", so no can do...

Comment 4

[Randell Jesup [:jesup] (needinfo me)]

We should commit this (as mentioned in bug 74309).  I have only one nit to pick
with the patch; PR_ARENA_CONST_ALIGN_MASK (currently 3) should be set to
something based on sizeof(whatever)-1.  Think about porting this to a 64-bit
environment, for example.  Other than that, for what it's worth ;-), r=rjesup

Comment 5

[Marc Attinasi]

Thanks for the comment, Randell. I talked to Pierre and he thinks that this
needs to wait until he has completed his sharing changes. Also, I strongly
suggest that we run the performance tests again to make sure that this is
showing a real benefit. Another data point too: Pierre suggested that startup
should speed up greatly from this, since we allocate/free thousands of temporary
style contexts in startup. I never profiled startup when I tested the patch
originally...

Comment 6

[Randell Jesup [:jesup] (needinfo me)]

As per the performance meeting last week, I'm going to update this patch.

Comment 7

[Daniel Bratell]

Randell, how is it going? How is this affected by bug 43457 "MEM: Style Context 
Sharing needs to be improved to increase sharing and reduce performance 
degradation"?

Comment 8

[Randell Jesup [:jesup] (needinfo me)]

FYI, I finally found some time and am working to merge the pool code for style
sharing (which is currently turned off!)  I'll upload a new patch once I get it
working.

Comment 9

[Hixie (not reading bugmail)]

Be aware that if bug 78695 is fixed, much of the style context sharing code will
be obsolete...

Comment 10

[Cathleen]

if that's true, we should definitely wait for hyatt's changes for bug 78695

Comment 11

[David Hyatt]

I'm taking this.  All data is now allocated from the shell's arena on my branch.

Comment 12

[David Hyatt]

Moving in and marking dependent on 76895.

Comment 13

[David Hyatt]

Do not check in this patch.  It is obsolete.

For the curious, the table stress test has dropped from 16-18 seconds down to 
4.5 seconds on the style branch.

Comment 14

[David Hyatt]

Fixed.

Comment 15

[Hixie (not reading bugmail)]

Rubber stamp verification.