730240 - While playing CSS3 animation bunch of CPU used by memcpy

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Oleg Romashin (:romaxa)]

Not sure hom much valid this bug, but I guess it would be nice to avoid memcpy from profile... not?

50        5.2466  libc-2.13.so             b2g   memcpy
39        4.0923  libxul.so                b2g   gfx3DMatrix::Transform3D(gfxPoint3D const&) const
15        1.5740  libm-2.13.so             b2g   __dubsin
13        1.3641  libxul.so                b2g   gfx3DMatrix::operator*(gfx3DMatrix const&) const
12        1.2592  libxul.so                b2g  pixman_composite_src_n_8888_asm_neon
11        1.1542  pvrsrvkm                 b2g  /pvrsrvkm
10        1.0493  libc-2.13.so             b2g   memset
10        1.0493  libm-2.13.so             b2g  sinl
10        1.0493  libxul.so                b2g SearchTable(PLDHashTable*, void const*, unsigned int, PLDHashOperator)
9         0.9444  libm-2.13.so             b2g  cosl
8         0.8395  libnspr4.so              b2g PR_GetCurrentThread
7         0.7345  libxul.so                b2g gfx3DMatrix::Transform(gfxPoint const&) const
7         0.7345  libxul.so                b2g gfx3DMatrix::TransformBounds(gfxRect const&) const
7         0.7345  libxul.so                b2g gfx3DMatrix::Translate(gfxPoint3D const&)
7         0.7345  libxul.so                b2g mozilla::layers::SortLayersBy3DZOrder(nsTArray<mozilla::layers::Layer*, nsTArrayDefaultAllocator>&)
7         0.7345  libxul.so                b2g nsRect::ScaleToOutsidePixels(float, float, int) const
7         0.7345  vmlinux                  b2g  __schedule
6         0.6296  libxul.so                b2g PL_DHashTableOperate
6         0.6296  libxul.so                b2g nsDisplayTransform::GetResultingTransformMatrix(nsIFrame const*, nsPoint const&, float, nsRect const*, nsIFrame**)


#0  memcpy () at ../ports/sysdeps/arm/memcpy.S:67
#1  0x40b739ee in nsStyleCoord (aUnit=<optimized out>, aValue=<optimized out>, this=0xbed8af30) at layout/style/nsStyleCoord.h:278
#2  nsStyleSides::Get (this=<optimized out>, aSide=<optimized out>) at layout/style/nsStyleCoord.h:384
#3  0x40b900f0 in nsStyleBorder::GetImageOutset (this=0x4b4865b0) at layout/style/nsStyleStruct.cpp:514
#4  0x40b2e12a in ComputeOutlineAndEffectsRect (aFrame=0x4b486ca0, aAnyOutlineOrEffects=0xbed8b0c7, aOverflowRect=<optimized out>, aNewSize=..., aStoreRectProperties=true)
    at layout/generic/nsFrame.cpp:4835
#5  0x40b2ea1c in nsIFrame::FinishAndStoreOverflow (this=0x4b486ca0, aOverflowAreas=..., aNewSize=...) at layout/generic/nsFrame.cpp:6736
#6  0x40b2ee3e in RecomputePreserve3DChildrenOverflow (aFrame=0x4b4858a8, aBounds=0xbed8b394) at layout/generic/nsFrame.cpp:6896
#7  0x40b2e650 in nsIFrame::ComputePreserve3DChildrenOverflow (this=0x4b4858a8, aOverflowAreas=..., aBounds=...) at layout/generic/nsFrame.cpp:6932
#8  0x40b2eb18 in nsIFrame::FinishAndStoreOverflow (this=0x4b4858a8, aOverflowAreas=..., aNewSize=...) at layout/generic/nsFrame.cpp:6771
#9  0x40ae4246 in nsCSSFrameConstructor::ProcessRestyledFrames (this=0x49dfee00, aChangeList=...) at layout/base/nsCSSFrameConstructor.cpp:7890
#10 0x40ae4496 in nsCSSFrameConstructor::RestyleElement (this=0x49dfee00, aElement=<optimized out>, aPrimaryFrame=0x4b4858a8, aMinHint=0, aRestyleTracker=..., aRestyleDescendants=false)
    at layout/base/nsCSSFrameConstructor.cpp:7980
#11 0x40ada9b0 in mozilla::css::RestyleTracker::ProcessOneRestyle (this=0x49dfef40, aElement=0x4d093100, aRestyleHint=<optimized out>, aChangeHint=0)
    at layout/base/RestyleTracker.cpp:157
#12 0x40adadde in mozilla::css::RestyleTracker::DoProcessRestyles (this=0x49dfef40) at layout/base/RestyleTracker.cpp:242
#13 0x40ae4420 in ProcessRestyles (this=0x49dfef40) at layout/base/RestyleTracker.h:101
#14 nsCSSFrameConstructor::ProcessPendingRestyles (this=0x49dfee00) at layout/base/nsCSSFrameConstructor.cpp:11558

Comment 1

[David Baron :dbaron:]

C++2011 seems to allow assignment of unions, though C++1998 didn't (as far as I can tell).  I wonder if we use it elsewhere in our tree.

Comment 2

[David Baron :dbaron:]

Attachment: patch 1: use assignment and copy construction of unions

Try seems happy with this (so far, not quite done):
https://tbpl.mozilla.org/?tree=Try&rev=956a9a1293aa
so I think it's portable now.

Comment 3

[David Baron :dbaron:]

Attachment: patch 2: make nsStyleCoord members private

It said "FIXME", so I did, while I was nearby.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 600582 [details] [diff] [review]
patch 1: use assignment and copy construction of unions

r=me

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 600583 [details] [diff] [review]
patch 2: make nsStyleCoord members private

r=me

Comment 6

[Oleg Romashin (:romaxa)]

Great, it helps!

Thanks

Comment 7

[Oleg Romashin (:romaxa)]

Attachment: More memcpy's

I see couple of other places involved into CSS3 rendering..
not sure that all can be optimized, but OverflowClip at least could be.

Comment 8

[David Baron :dbaron:]

https://hg.mozilla.org/integration/mozilla-inbound/rev/350d812f93b9
https://hg.mozilla.org/integration/mozilla-inbound/rev/b958923e04ba

Comment 9

[Oleg Romashin (:romaxa)]

Attachment: simple "for", with optimization enrolled into 8 k1[i]=k2[i]

Comment 10

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/350d812f93b9
https://hg.mozilla.org/mozilla-central/rev/b958923e04ba

Comment 11

[Oleg Romashin (:romaxa)]

Comment on attachment 600641 [details] [diff] [review]
simple "for", with optimization enrolled into 8 k1[i]=k2[i]

Ok, after removing Invalidation problems for css3, I don't see much memcpy anymore... so it is probably not super important issue for now.