Closed Bug 584293 Opened 14 years ago Closed 14 years ago

Speed up .style on HTML and XUL elements

Categories

(Core :: DOM: Core & HTML, defect, P1)

x86
macOS
defect

Tracking

()

RESOLVED FIXED
mozilla2.0b4
Tracking Status
blocking2.0 --- beta4+

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

As the summary says: make .style faster. Measurements suggest the upcoming patch makes it 3.5x faster. The change to nsXULElement::GetStyle overlaps with a similar change in bug 569719; hence the dependency. Zack should land his stuff first.
Attached patch Fix (obsolete) (deleted) — Splinter Review
Two notes: 1) This doesn't handle SVG elements. That's a different interface, but would be easy to do if we have a lot of free bits in the typemapping. Do we? 2) This is still slower than webkit. Remaining time spent is 55% under xpc_qsXPCOMObjectToJsval (self time, time in NativeInterface2JSObject, time in FindInJSObjectScope, QI on the decl). 22% is spent in the GetStyle quickstub (a lot of this setting up the ToJsval function call with its many arguments). 5% is spent calling GetStyle on the element. 9% is spent in trace-generated code, and 6% in GetPropertyWithNativeGetter.
Attachment #462694 - Flags: review?(peterv)
Attachment #462694 - Flags: approval2.0?
Depends on: 569719
Summary: Speed up .style on HTML and XUL elements → [FIX]Speed up .style on HTML and XUL elements
Comment on attachment 462694 [details] [diff] [review] Fix >+nsIDOMCSSStyleDeclaration* >+nsStyledElement::GetStyle(nsresult* retval) > { >+ nsXULElement* xulElement = nsXULElement::FromContent(this); >+ if (xulElement) { >+ nsresult rv = xulElement->EnsureLocalStyle(); >+ if (NS_FAILED(rv)) { It would be nice if EnsureLocalStyle returned void (all it throws is OUT_OF_MEMORY?). Oh well. > nsGenericElement::nsDOMSlots *slots = GetDOMSlots(); >- NS_ENSURE_TRUE(slots, NS_ERROR_OUT_OF_MEMORY); >+ if (!slots) { I don't think this can happen anymore? >diff --git a/content/html/content/src/nsGenericHTMLElement.cpp b/content/html/content/src/nsGenericHTMLElement.cpp >+ NS_IMETHOD GetStyle(nsIDOMCSSStyleDeclaration** aStyle) { Brace on its own line. >diff --git a/content/xul/content/src/nsXULElement.cpp b/content/xul/content/src/nsXULElement.cpp >+ NS_IMETHOD GetStyle(nsIDOMCSSStyleDeclaration** aStyle) { Same here. The bitmap has 32 bits. Adding nsSVGStylableElement looks fine to me.
Attachment #462694 - Flags: review?(peterv) → review+
> all it throws is OUT_OF_MEMORY? I think so, but I don't think it's following infallible-only codepaths yet. Once it is, I'd hope static analysis would win for us. > I don't think this can happen anymore? Indeed. Took that null-check out. > Brace on its own line. Done, both places. I added nsSVGStylableElement.
Attached patch Updated to comments (deleted) — Splinter Review
Attachment #462694 - Attachment is obsolete: true
Attachment #462819 - Flags: approval2.0?
Attachment #462694 - Flags: approval2.0?
Summary: [FIX]Speed up .style on HTML and XUL elements → Speed up .style on HTML and XUL elements
Whiteboard: [need approval]
blocking2.0: --- → beta4+
Attachment #462819 - Flags: approval2.0?
Whiteboard: [need approval] → [need landing]
Status: NEW → RESOLVED
Closed: 14 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Whiteboard: [need landing]
Target Milestone: --- → mozilla2.0b4
Some combination of this and bug 584287 gave us a 5% win on Dromaeo (DOM) on tinderbox on Linux and Mac.
Depends on: 585745
Depends on: 628794
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: