release vs. debug version

[Ilan]

I want to use Rosegarden for my wife, and per chance I decided to listen to aveverum again.
To my surprise there were hesitations in the playing of the piece.
I had heard this piece many times under Classic and I couldn't remember such activity.
I ran it 3 times and it hesitated at different bars each time:

17,33,42,45
27,34,40
4,6,10,24,32,34,38,41

I decided to try it with ./configure without --enable-debug
and now there were no hesitations.
The difference in CPU usage was 30%-45% with debug and 20%-35% without debug.
Even 30-45% isn't a tremendous use of CPU, but I am wondering if people with older hardware aren't going to have problems. (My laptop is about 1 year old.)

First I would like to know if anyone with --enable-debug can duplicate the problem?
Can anyone with older hardware see the problem in the release version?

Ilan

[cannam]

On Fri, Oct 16, 2009 at 6:43 PM, Ilan <ilan.tal@...> wrote:
> I decided to try it with ./configure without --enable-debug
> and now there were no hesitations.

A debug build running with its console output going to a terminal or
IDE is unlikely ever to perform very well.  We probably just have to
accept that.

A release build is another matter, and I think it's apparent that we
have some work to do on that.

(Were you using Rosegarden for MIDI only in your tests, i.e. with an
external synth such as qsynth?)

There are I think two areas in which we do worse than Classic:

 * Rendering speed for general user interface elements.

Refreshes of stylesheet-based widgets with heavy use of gradient
brushes can be disappointingly slow, depending (a great deal) on your
video card.

Although there were a lot of speed improvements in Qt 4.5, caching for
things like backgrounds of toolbar buttons and combo boxes with
complex rendering procedures doesn't seem to have been among them.

Updating the transport window can also be substantially slower than it
was, probably because the digits are alpha-blended.  (We could avoid
that by pre-rendering them onto black backgrounds in pixmaps without
alpha channels -- we could do that in a few lines in the code when
setting up the transport dialog to avoid spoiling our nice png files
by adding backgrounds to them.)  Of course we render far more digits
than are really useful in the transport, but then we've always done
that.

 * Locking in the sequencer.

The Classic sequencer is a separate process from the GUI, and
communication between the two is more or less lock-free.  In Thorn the
sequencer is a thread and we generally take out a lock when calling
from GUI to sequencer.

These locks are only on the userspace MIDI threads -- we don't lock
the real-time JACK threads and we also don't do precision event
dispatching in the locked threads (that happens in the ALSA sequencer
driver in the kernel, we just schedule events ahead of time) but
waiting on locks could still be a significant source of overhead.
We'll need to review these a bit more closely.

I don't think either case presents an insurmountable problem, and I
don't think in either case that we should go back on the changes
concerned.  We just need to look a bit more closely at the adverse
consequences and how we can mitigate them.


Chris

------------------------------------------------------------------------------
Come build with us! The BlackBerry(R) Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9 - 12, 2009. Register now!
http://p.sf.net/sfu/devconference
_______________________________________________
Rosegarden-devel mailing list
Rosegarden-devel@... - use the link below to unsubscribe
https://lists.sourceforge.net/lists/listinfo/rosegarden-devel

[D. Michael McIntyre]

On Friday 16 October 2009, Chris Cannam wrote:

> A debug build running with its console output going to a terminal or
> IDE is unlikely ever to perform very well.  We probably just have to
> accept that.

I'm running a debug build (messages to /dev/null) and I'm not experiencing
timing problems with this piece.  I have a killer quad core machine, but I
also have a completely broken audio environment with no realtime and a low-
resolution timer.

> Refreshes of stylesheet-based widgets with heavy use of gradient
> brushes can be disappointingly slow, depending (a great deal) on your
> video card.

Would there be less overhead if we replaced the gradient brushes with pixmaps?

> Updating the transport window can also be substantially slower than it
> was, probably because the digits are alpha-blended.  (We could avoid
> that by pre-rendering them onto black backgrounds in pixmaps without
> alpha channels....

Well, that sort of defeats the purpose of redoing the transport pixmaps in the
first place.  They used to have built-in black backgrounds, and the flashing
metronome looked like utter shit, which is why they have an alpha channel now.
--
D. Michael McIntyre

------------------------------------------------------------------------------
Come build with us! The BlackBerry(R) Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9 - 12, 2009. Register now!
http://p.sf.net/sfu/devconference
_______________________________________________
Rosegarden-devel mailing list
Rosegarden-devel@... - use the link below to unsubscribe
https://lists.sourceforge.net/lists/listinfo/rosegarden-devel

[cannam]

On Sat, Oct 17, 2009 at 12:16 AM, D. Michael McIntyre
<michael.mcintyre@...> wrote:
> On Friday 16 October 2009, Chris Cannam wrote:
>> Refreshes of stylesheet-based widgets with heavy use of gradient
>> brushes can be disappointingly slow, depending (a great deal) on your
>> video card.
>
> Would there be less overhead if we replaced the gradient brushes with pixmaps?

I don't know.  I said "gradient brushes", but I was really just
loosely thinking "complex brushes".  This sort of thing is very hard
to benchmark, of course.

I seem to remember noticing (when profiling with sysprof) that a lot
of time was spent rendering toolbar buttons and combo boxes, so I
suppose that I'm really talking about is whatever they use (at least
on the machine in question).

>> Updating the transport window can also be substantially slower than it
>> was, probably because the digits are alpha-blended.  (We could avoid
>> that by pre-rendering them onto black backgrounds in pixmaps without
>> alpha channels....
>
> Well, that sort of defeats the purpose of redoing the transport pixmaps in the
> first place.  They used to have built-in black backgrounds, and the flashing
> metronome looked like utter shit, which is why they have an alpha channel now.

No, no, if we do it programmatically we can just render and cache for
each foreground/background colour combo.  Or render and cache for
black backgrounds and render directly for others.  Or whatever.  No
loss in the pixmaps themselves.  If I get a moment (ha) I'll code it
up and see if it makes any difference.


Chris

------------------------------------------------------------------------------
Come build with us! The BlackBerry(R) Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9 - 12, 2009. Register now!
http://p.sf.net/sfu/devconference
_______________________________________________
Rosegarden-devel mailing list
Rosegarden-devel@... - use the link below to unsubscribe
https://lists.sourceforge.net/lists/listinfo/rosegarden-devel

[cannam]

On Sat, Oct 17, 2009 at 9:54 AM, Chris Cannam
<cannam@...> wrote:

> On Sat, Oct 17, 2009 at 12:16 AM, D. Michael McIntyre
> <michael.mcintyre@...> wrote:
>> Well, that sort of defeats the purpose of redoing the transport pixmaps in the
>> first place.  They used to have built-in black backgrounds, and the flashing
>> metronome looked like utter shit, which is why they have an alpha channel now.
>
> No, no, if we do it programmatically we can just render and cache for
> each foreground/background colour combo.  Or render and cache for
> black backgrounds and render directly for others.  Or whatever.  No
> loss in the pixmaps themselves.  If I get a moment (ha) I'll code it
> up and see if it makes any difference.

It didn't.  Patch attached in case anyone else wants to try it, but it
did nothing for me here and I'm not going to commit (I'd ask that
nobody else commits it either, at least without reporting some
improvement from it here first).


Chris

[rg-prerender-transport-pixmaps.diff]

---

Index: src/gui/dialogs/TransportDialog.cpp
===================================================================
--- src/gui/dialogs/TransportDialog.cpp (revision 11066)
+++ src/gui/dialogs/TransportDialog.cpp (working copy)
@@ -47,6 +47,7 @@
 #include <QWidget>
 #include <QHBoxLayout>
 #include <QDesktopWidget>
+#include <QPainter>
 
 
 
@@ -56,6 +57,7 @@
 TransportDialog::TransportDialog(QWidget *parent):
     QDialog(parent, 0),
     m_transport(0),
+    m_lcdList(0),
     m_lastTenHours(0),
     m_lastUnitHours(0),
     m_lastTenMinutes(0),
@@ -356,15 +358,32 @@
 void
 TransportDialog::loadPixmaps()
 {
-    m_lcdList.clear();
+    m_transparentLcdList.clear();
 
     for (int i = 0; i < 10; i++) {
-        m_lcdList[i] = IconLoader().loadPixmap(QString("led-%1").arg(i));
+        m_transparentLcdList[i] =
+            IconLoader().loadPixmap(QString("led-%1").arg(i));
     }
 
-    // Load the "negative" sign pixmap
+    // Load the "negative" sign pixmap as special index -1
     //
-    m_lcdNegative = IconLoader().loadPixmap("led--");
+    m_transparentLcdList[-1] = IconLoader().loadPixmap("led--");
+
+    // Pre-render these on a black background.  These will be used
+    // only when drawing on black -- on other colours (when using the
+    // visual metronome) we draw directly from the transparent pixmaps.
+    //
+    for (int i = -1; i < 10; ++i) {
+        QPixmap source = m_transparentLcdList[i];
+        QPixmap pm(source.size());
+        pm.fill(Qt::black);
+        QPainter paint(&pm);
+        paint.drawPixmap(0, 0, source);
+        paint.end();
+        m_blackLcdList[i] = pm;
+    }
+
+    m_lcdList = &m_blackLcdList;
 }
 
 void
@@ -553,10 +572,10 @@
 
     // If time is negative then reverse the time and set the minus flag
     //
-    if (st < RealTime::zeroTime) {
+    if (st < RealTime::zeroTime && m_lcdList) {
         st = RealTime::zeroTime - st;
         if (!m_lastNegative) {
-            m_transport->NegativePixmap->setPixmap(m_lcdNegative);
+            m_transport->NegativePixmap->setPixmap((*m_lcdList)[-1]);
             m_lastNegative = true;
         }
     } else // don't show the flag
@@ -601,10 +620,10 @@
 
     // If time is negative then reverse the time and set the minus flag
     //
-    if (st < RealTime::zeroTime) {
+    if (st < RealTime::zeroTime && m_lcdList) {
         st = RealTime::zeroTime - st;
         if (!m_lastNegative) {
-            m_transport->NegativePixmap->setPixmap(m_lcdNegative);
+            m_transport->NegativePixmap->setPixmap((*m_lcdList)[-1]);
             m_lastNegative = true;
         }
     } else // don't show the flag
@@ -658,10 +677,10 @@
 
     // If time is negative then reverse the time and set the minus flag
     //
-    if (st < RealTime::zeroTime) {
+    if (st < RealTime::zeroTime && m_lcdList) {
         st = RealTime::zeroTime - st;
         if (!m_lastNegative) {
-            m_transport->NegativePixmap->setPixmap(m_lcdNegative);
+            m_transport->NegativePixmap->setPixmap((*m_lcdList)[-1]);
             m_lastNegative = true;
         }
     } else // don't show the flag
@@ -707,10 +726,10 @@
 
     // If time is negative then reverse the time and set the minus flag
     //
-    if (bar < 0) {
+    if (bar < 0 && m_lcdList) {
         bar = -bar;
         if (!m_lastNegative) {
-            m_transport->NegativePixmap->setPixmap(m_lcdNegative);
+            m_transport->NegativePixmap->setPixmap((*m_lcdList)[-1]);
             m_lastNegative = true;
         }
     } else // don't show the flag
@@ -777,11 +796,19 @@
 {
     Profiler profiler("TransportDialog::updateTimeDisplay");
 
+    if (!m_lcdList) return;
+
+    // m_lcdList is a pointer to either m_blackLcdList or
+    // m_transparentLcdList, depending on whether we have a black
+    // background or not
+    //
+    std::map<int, QPixmap> &lcds = *m_lcdList;
+
     if (m_tenThousandths != m_lastTenThousandths) {
         if (m_tenThousandths < 0)
             m_transport->TenThousandthsPixmap->clear();
         else
-            m_transport->TenThousandthsPixmap->setPixmap(m_lcdList[m_tenThousandths]);
+            m_transport->TenThousandthsPixmap->setPixmap(lcds[m_tenThousandths]);
         m_lastTenThousandths = m_tenThousandths;
     }
 
@@ -789,7 +816,7 @@
         if (m_thousandths < 0)
             m_transport->ThousandthsPixmap->clear();
         else
-            m_transport->ThousandthsPixmap->setPixmap(m_lcdList[m_thousandths]);
+            m_transport->ThousandthsPixmap->setPixmap(lcds[m_thousandths]);
         m_lastThousandths = m_thousandths;
     }
 
@@ -797,7 +824,7 @@
         if (m_hundreths < 0)
             m_transport->HundredthsPixmap->clear();
         else
-            m_transport->HundredthsPixmap->setPixmap(m_lcdList[m_hundreths]);
+            m_transport->HundredthsPixmap->setPixmap(lcds[m_hundreths]);
         m_lastHundreths = m_hundreths;
     }
 
@@ -805,7 +832,7 @@
         if (m_tenths < 0)
             m_transport->TenthsPixmap->clear();
         else
-            m_transport->TenthsPixmap->setPixmap(m_lcdList[m_tenths]);
+            m_transport->TenthsPixmap->setPixmap(lcds[m_tenths]);
         m_lastTenths = m_tenths;
     }
 
@@ -813,7 +840,7 @@
         if (m_unitSeconds < 0)
             m_transport->UnitSecondsPixmap->clear();
         else
-            m_transport->UnitSecondsPixmap->setPixmap(m_lcdList[m_unitSeconds]);
+            m_transport->UnitSecondsPixmap->setPixmap(lcds[m_unitSeconds]);
         m_lastUnitSeconds = m_unitSeconds;
     }
 
@@ -821,7 +848,7 @@
         if (m_tenSeconds < 0)
             m_transport->TenSecondsPixmap->clear();
         else
-            m_transport->TenSecondsPixmap->setPixmap(m_lcdList[m_tenSeconds]);
+            m_transport->TenSecondsPixmap->setPixmap(lcds[m_tenSeconds]);
         m_lastTenSeconds = m_tenSeconds;
     }
 
@@ -829,7 +856,7 @@
         if (m_unitMinutes < 0)
             m_transport->UnitMinutesPixmap->clear();
         else
-            m_transport->UnitMinutesPixmap->setPixmap(m_lcdList[m_unitMinutes]);
+            m_transport->UnitMinutesPixmap->setPixmap(lcds[m_unitMinutes]);
         m_lastUnitMinutes = m_unitMinutes;
     }
 
@@ -837,7 +864,7 @@
         if (m_tenMinutes < 0)
             m_transport->TenMinutesPixmap->clear();
         else
-            m_transport->TenMinutesPixmap->setPixmap(m_lcdList[m_tenMinutes]);
+            m_transport->TenMinutesPixmap->setPixmap(lcds[m_tenMinutes]);
         m_lastTenMinutes = m_tenMinutes;
     }
 
@@ -845,7 +872,7 @@
         if (m_unitHours < 0)
             m_transport->UnitHoursPixmap->clear();
         else
-            m_transport->UnitHoursPixmap->setPixmap(m_lcdList[m_unitHours]);
+            m_transport->UnitHoursPixmap->setPixmap(lcds[m_unitHours]);
         m_lastUnitHours = m_unitHours;
     }
 
@@ -853,7 +880,7 @@
         if (m_tenHours < 0)
             m_transport->TenHoursPixmap->clear();
         else
-            m_transport->TenHoursPixmap->setPixmap(m_lcdList[m_tenHours]);
+            m_transport->TenHoursPixmap->setPixmap(lcds[m_tenHours]);
         m_lastTenHours = m_tenHours;
     }
 }
@@ -1136,7 +1163,6 @@
 TransportDialog::slotSetBackground(QString c)
 {
     if (!m_haveOriginalBackground) {
-        //m_originalBackground = m_transport->LCDBoxFrame->paletteBackgroundColor();
         m_originalBackground = "black";
         m_haveOriginalBackground = true;
     }
@@ -1152,6 +1178,7 @@
     // flashes evenly.  I like it. (dmm)
     m_transport->LCDBoxFrame->setStyleSheet(localStyle);
 
+    m_lcdList = &m_transparentLcdList;
     m_isBackgroundSet = true;
 }
 
@@ -1161,6 +1188,7 @@
     if (m_isBackgroundSet) {
         slotSetBackground(m_originalBackground);
     }
+    m_lcdList = &m_blackLcdList;
     m_isBackgroundSet = false;
 }
 
Index: src/gui/dialogs/TransportDialog.h
===================================================================
--- src/gui/dialogs/TransportDialog.h (revision 11066)
+++ src/gui/dialogs/TransportDialog.h (working copy)
@@ -167,8 +167,9 @@
 
     Ui_RosegardenTransport* m_transport;
 
-    std::map<int, QPixmap> m_lcdList;
-    QPixmap m_lcdNegative;
+    std::map<int, QPixmap> m_transparentLcdList;
+    std::map<int, QPixmap> m_blackLcdList;
+    std::map<int, QPixmap> *m_lcdList; // points to one of the prior two
 
     int m_lastTenHours;
     int m_lastUnitHours;

---

------------------------------------------------------------------------------
Come build with us! The BlackBerry(R) Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9 - 12, 2009. Register now!
http://p.sf.net/sfu/devconference
_______________________________________________
Rosegarden-devel mailing list
Rosegarden-devel@... - use the link below to unsubscribe
https://lists.sourceforge.net/lists/listinfo/rosegarden-devel