Allocate Style Contexts from arenas: complete analysis

VERIFIED FIXED in mozilla0.9.2

Status

()

Core
CSS Parsing and Computation
VERIFIED FIXED
18 years ago
17 years ago

People

(Reporter: Marc Attinasi, Assigned: David Hyatt)

Tracking

({perf})

Trunk
mozilla0.9.2
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment)

(Reporter)

Description

18 years ago
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.)
(Reporter)

Comment 1

18 years ago
Accepting.
Status: NEW → ASSIGNED
Target Milestone: --- → Future
(Reporter)

Comment 2

18 years ago
Created attachment 22500 [details] [diff] [review]
Patch that puts style contexts and style context data into arenas (if #define POOL_STYLECONTEXTS is uncommented in nsIStyleSet.h)
QA Contact: chrisd → ian
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...
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
(Reporter)

Comment 5

17 years ago
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...

Updated

17 years ago
Keywords: perf

Updated

17 years ago
Blocks: 7251

Updated

17 years ago
Blocks: 74309
As per the performance meeting last week, I'm going to update this patch.

Updated

17 years ago
Blocks: 76151

Comment 7

17 years ago
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"?

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.
Be aware that if bug 78695 is fixed, much of the style context sharing code will
be obsolete...

Comment 10

17 years ago
if that's true, we should definitely wait for hyatt's changes for bug 78695
(Assignee)

Comment 11

17 years ago
I'm taking this.  All data is now allocated from the shell's arena on my branch.
Assignee: attinasi → hyatt
Status: ASSIGNED → NEW
(Assignee)

Comment 12

17 years ago
Moving in and marking dependent on 76895.
Status: NEW → ASSIGNED
Depends on: 78695
Target Milestone: Future → mozilla0.9.2
(Assignee)

Comment 13

17 years ago
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.
(Assignee)

Comment 14

17 years ago
Fixed.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
Rubber stamp verification.
Status: RESOLVED → VERIFIED

Updated

17 years ago
No longer blocks: 7251

Updated

17 years ago
Blocks: 7251
You need to log in before you can comment on or make changes to this bug.