parseMappedAttribute a lot of the time sets a local var to get the attribute value. In a lot of cases, as shown by the coverage results, this value is not used at all. This is very clear here: http://www.openembedded.org/~zecke/coverage/webkit/mac/__WebCore__ksvg2__svg__SVGZoomAndPan.cpp.html Also half the time here the value is not used: http://www.openembedded.org/~zecke/coverage/webkit/mac/__WebCore__ksvg2__svg__SVGLinearGradientElement.cpp.html Even though a compiler may be able to optimize the local var away, I think it is better to make it clear and be consistent with html code here too. Patch coming up.
Created attachment 14350 [details] First attempt Should be straightforward patch. Would be nice if somebody could benchmark what the change brings us :) Cheers, Rob.
Comment on attachment 14350 [details] First attempt *SVGAnimateMotionElement::parseMappedAttribute I'd lift add const String& value = attr-value() after the first name check. It seem icky to query multiple times. *SVGAnimateTransformElement::parseMappedAttribute I don't think this gains anything. *SVGAnimationElement::parseMappedAttribute in those cases where you access attr->value() multiple times i think you should just declare a var. eg. the attributeTypeAttr case. *SVGCircleElement::parseMappedAttribute *SVGClipPathElement::parseMappedAttribute *SVGCursorElement::parseMappedAttribute *SVGEllipseElement::parseMappedAttribute I'd consider a local const& to hold the name, but otherwise looks good *SVGExternalResourcesRequired::parseMappedAttribute looks good *SVGGradientElement::parseMappedAttribute local for the attribute name, and maybe use locals in those branches that query the value multiple times. *SVGImageElement::parseMappedAttribute *SVGLineElement::parseMappedAttribute *SVGLinearGradientElement::parseMappedAttribute *SVGMarkerElement::parseMappedAttribute *SVGMaskElement::parseMappedAttribute *SVGPathElement::parseMappedAttribute local for the name again *SVGPatternElement::parseMappedAttribute local for name, local for value on those branches that access the value multiple times *SVGRadialGradientElement::parseMappedAttribute *SVGRectElement::parseMappedAttribute local for the name *SVGSVGElement::parseMappedAttribute local for name, local for value those places it's used multiple times *SVGScriptElement::parseMappedAttribute ooh, good *and* fixed indenting *SVGStopElement::parseMappedAttribute *SVGTests::parseMappedAttribute *SVGTextContentElement::parseMappedAttribute *SVGTextPositioningElement::parseMappedAttribute *SVGUseElement::parseMappedAttribute local for name if it's used multiple imes *SVGViewElement::parseMappedAttribute good *SVGZoomAndPan::parseMappedAttribute local for value when it's used multiple times
Comment on attachment 14350 [details] First attempt Ignore last comment, seems like this patch matches the style elsewhere
Landed in r21272.