[callgrind] compiz spends about 51% of its CPU time in CompRegion construction/destruction

Bug #940139 reported by Daniel van Vugt
78
This bug affects 14 people
Affects Status Importance Assigned to Milestone
Compiz Core
Fix Released
High
Daniel van Vugt
compiz (Ubuntu)
Fix Released
Undecided
Daniel van Vugt

Bug Description

I've only been profiling compiz with callgrind for a while, but the first thing that stands out as unusual is CompRegion. It appears to be such a heavily used class that compiz spends 51% of its CPU time constructing and destructing CompRegions...

1,173,358,129 = total execution time
223,504,551 = time in (and below) CompRegion::~CompRegion
218,441,501 = time in (and below) CompRegion::CompRegion(CompRect const&)
158,941,179 = time in (and below) CompRegion::CompRegion()

The primary reasons for this slowness appear to be:
  (a) XCreateRegion is slow.
  (b) new/delete is too slow to be used as frequently as CompRegion uses it.

Instead of fixing CompRegion itself though, we should first look at the call graph and see if there are any callers that dominate.

Related branches

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Inclusive subroutine costs.

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Exclusive subroutine costs.

Changed in compiz-core:
assignee: nobody → Daniel van Vugt (vanvugt)
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

I have some simple fixes in progress already. But out of time to test them today.

Changed in compiz-core:
status: Triaged → In Progress
milestone: none → 0.9.5.92
milestone: 0.9.5.92 → 0.9.7.2
importance: Medium → High
Revision history for this message
Sam Spilsbury (smspillaz) wrote :

Its the decor plugin :/

lp:~smspillaz/compiz-core/compiz-core.fix_slow_movement has some optimizations to be less aggressive on this.

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

You branch looks quite complicated. Also, my profiling told me that only about 40% of that 51% (=20%) was the decor plugin. But still the biggest single contributor from memory.

My fixes are much smaller and simpler, and will be guided by the profiler results and call graphs (which I haven't attached). Still need to refine and test my fixes. Another day.

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Sam -
Something I am not doing yet but we need to seriously consider in future, is to flatten the CompRegion class. Remove the priv member and just use a Region member. No new or delete. I know it goes against the established design and would require an ABI bump, but I think that would provide the biggest reduction in CPU usage. And is simple to implement.

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Actually, we don't need to break or bump the ABI at all. "Region" is a pointer by definition so it should be fairly easy to transparently replace the priv pointer with Region without affecting the ABI.

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Good news. I have a working solution that drops the CompRegion overhead from 51% to 9% without any ABI breakage. That's an average reduction in compiz CPU usage of 42%!

I will spend some more time testing and trying to improve it further before I propose it.

Revision history for this message
Sam Spilsbury (smspillaz) wrote : Re: [Bug 940139] Re: [callgrind] compiz spends about 51% of its CPU time in CompRegion construction/destruction

On Sun, 26 Feb 2012, Daniel van Vugt wrote:

> Good news. I have a working solution that drops the CompRegion overhead
> from 51% to 9% without any ABI breakage. That's an average reduction in
> compiz CPU usage of 42%!
>
> I will spend some more time testing and trying to improve it further
> before I propose it.
>

You are a hero, as usual :)

> --
> You received this bug notification because you are a member of Compiz
> Maintainers, which is the registrant for Compiz Core.
> https://bugs.launchpad.net/bugs/940139
>
> Title:
> [callgrind] compiz spends about 51% of its CPU time in CompRegion
> construction/destruction
>
> Status in Compiz Core:
> In Progress
>
> Bug description:
> I've only been profiling compiz with callgrind for a while, but the
> first thing that stands out as unusual is CompRegion. It appears to be
> such a heavily used class that compiz spends 51% of its CPU time
> constructing and destructing CompRegions...
>
> 1,173,358,129 = total execution time
> 223,504,551 = time in (and below) CompRegion::~CompRegion
> 218,441,501 = time in (and below) CompRegion::CompRegion(CompRect const&)
> 158,941,179 = time in (and below) CompRegion::CompRegion()
>
> The primary reasons for this slowness appear to be:
> (a) XCreateRegion is slow.
> (b) new/delete is too slow to be used as frequently as CompRegion uses it.
>
> Instead of fixing CompRegion itself though, we should first look at
> the call graph and see if there are any callers that dominate.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/compiz-core/+bug/940139/+subscriptions
>

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Merged into lp:compiz-core at revision 3023

Changed in compiz-core:
status: In Progress → Fix Committed
Changed in compiz-core:
milestone: 0.9.7.2 → 0.9.7.0
Changed in compiz-core:
status: Fix Committed → Fix Released
Revision history for this message
Diska (discardi-nicola) wrote :

someone know when this patch will enter in ubuntu official repository?

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

The fix is in the compiz official release 0.9.7.0, which is revision 3033. The fix itself was revision 3023. Ubuntu 12.04 has a pre-release version based on revision 2995.

Once ubuntu gets a compiz release based on bzr3023 or higher then you will have this fix.

Revision history for this message
Diska (discardi-nicola) wrote :

ok there a ppa to use or it will include in precise?

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Judging by recent conversations with the packager, it sounds like this fix has made it into beta-2.

Revision history for this message
Diska (discardi-nicola) wrote :

thx a lot

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

This bug was fixed in the package compiz - 1:0.9.7.0+bzr3035-0ubuntu1

---------------
compiz (1:0.9.7.0+bzr3035-0ubuntu1) precise; urgency=low

  [ Łukasz 'sil2100' Zemczak ]
  * New upstream snapshot:
    - Fix gtk-window-decorator crash upon demaximizing a window (LP: #930071)
    - Fix core keybindings (LP: #930412)
    - Fixes compiz crash with SIGSEGV on shutdown (LP: #931283)
    - Plugins can't tell the difference between a key-tap and modifier
      key-release (LP: #925293)
    - compiz-core r3001 (and 3002) ftbfs (LP: #933226)
    - Semi-maximized windows have no shadow or frame (LP: #924736)
    - Untranslated strings in gtk-window-decorator (LP: #780505)
    - Initialize the _NET_WM_STATE_FOCUSED (LP: #932087)
    - [regression] Customized shortcuts don't work (LP: #931927)
    - Window stacking problem (LP: #936675)
    - Quickly demaximized windows can receive maximized window decorations if
      they were initially maximized (LP: #936778)
    - Maximized windows do not get shadows at all (LP: #936774)
    - [regression] Launcher, top panel and keyboard un-responsive after using
      any Super-x shortcut (LP: #934058)
    - No draggable border if mutter isn't installed (LP: #936781)
    - Fix compiz crash with SIGSEGV in XDefineCursor() (LP: #936487)
    - Fixes memory leak at DecorWindow::updateSwitcher() (LP: #940115)
    - Unresolved symbols in plugins cause compiz to exit (LP: #938478)
    - Fix compiz spending about 51% of its CPU time in CompRegion
      construction/destruction (LP: #940139)
    - Fix Conditional jump or move depends on uninitialised value(s) in
      decor_match_pixmap (LP: #940066)
    - Fix 'show desktop' behaviour (LP: #871801)
    - Tweak algorithm used to cast shadows on maximized windows (LP: #936784)
    - "Svg" and "Png" should be "SVG and "PNG" (LP: #942890)
    - Fix invalid memory usage after free() in DecorWindow (LP: #943116)
    - Fix alt + F10 (LP: #943223)
  * Removed cherry-picked patches
  * debian/patches/fix_944631.patch:
    - Always replay the keyboard if something was grabbed and didn't trigger
      an action and don't trigger actions which aren't added accidentally
      (LP: #943612) (LP: #944631)
  * debian/patches/fix_923683.patch:
    - Backports a patch which prevents the shift race condition

  [ Didier Roche ]
  * debian/patches/fix_alt_pressing.patch:
    - Patch from ddv to fix all the regressions with the alt key fix and other
      (LP: #943851, #945373)
    - Fix Quicklist are not showing if right-clicking a launcher icon in Expo
      mode if triggered by Super + S (LP: #944979)
  * debian/patches/fix_806255.patch:
    - Unity/compiz intercepts keystrokes from grabbed windows (LP: #806255)
  * debian/patches/fix_943194.patch:
    - second part for the alt key fix (LP: #943194)
  * debian/patches/additional_alt_tapping_fix.patch:
    - again another alt tapping related fix for some regressions from the
      previous branch. Taken from "tapping-panacea" upstream branch.
 -- Didier Roche <email address hidden> Mon, 12 Mar 2012 10:22:10 +0100

Changed in compiz (Ubuntu):
status: New → Fix Released
assignee: nobody → Daniel van Vugt (vanvugt)
Revision history for this message
Luke Hoersten (lukehoersten) wrote :

CPU usage seems much better in an unpatched Precise 12.04. Thanks a lot!

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.