From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=46575 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1OLH9l-0003Ed-30
	for qemu-devel@nongnu.org; Sun, 06 Jun 2010 10:48:58 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69)
	(envelope-from <corentin.chary@gmail.com>) id 1OLH9j-0000NR-32
	for qemu-devel@nongnu.org; Sun, 06 Jun 2010 10:48:56 -0400
Received: from mail-bw0-f45.google.com ([209.85.214.45]:47411)
	by eggs.gnu.org with esmtp (Exim 4.69)
	(envelope-from <corentin.chary@gmail.com>) id 1OLH9i-0000NK-TX
	for qemu-devel@nongnu.org; Sun, 06 Jun 2010 10:48:55 -0400
Received: by bwz4 with SMTP id 4so879789bwz.4
	for <qemu-devel@nongnu.org>; Sun, 06 Jun 2010 07:48:54 -0700 (PDT)
MIME-Version: 1.0
In-Reply-To: <4C0BAC89.10804@redhat.com>
References: <1275657620-26226-1-git-send-email-corentincj@iksaif.net>
	<1275657620-26226-3-git-send-email-corentincj@iksaif.net>
	<4C0BAC89.10804@redhat.com>
Date: Sun, 6 Jun 2010 16:48:53 +0200
Message-ID: <AANLkTim1yZSzLWLFm0J-CWijGjzkFhNy_LLObEnB8xr4@mail.gmail.com>
Subject: Re: [Qemu-devel] [PATCH v2 2/2] vnc: threaded VNC server
From: Corentin Chary <corentin.chary@gmail.com>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Avi Kivity <avi@redhat.com>
Cc: Anthony Liguori <aliguori@linux.vnet.ibm.com>, Alexander Graf <agraf@suse.de>, Qemu-development List <qemu-devel@nongnu.org>, Gautham R Shenoy <ego@in.ibm.com>

On Sun, Jun 6, 2010 at 4:11 PM, Avi Kivity <avi@redhat.com> wrote:
> On 06/04/2010 04:20 PM, Corentin Chary wrote:
>>
>> + =A0 =A0if (vnc_trylock_display(vd)) {
>> + =A0 =A0 =A0 =A0vd->timer_interval =3D VNC_REFRESH_INTERVAL_BASE;
>> + =A0 =A0 =A0 =A0qemu_mod_timer(vd->timer, qemu_get_clock(rt_clock) +
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 vd->timer_interval);
>> + =A0 =A0 =A0 =A0return;
>> + =A0 =A0}
>> +
>> =A0 =A0 =A0has_dirty =3D vnc_refresh_server_surface(vd);
>> + =A0 =A0vnc_unlock_display(vd);
>>
>
> This could delay the update by quite a bit, no?

Yep, but it's far better than waiting the lock because it doesn't slow
down the main thread.
I played big buck bunny trailler (33sec) in mplayer and tight encoding:
- ~40 sec with the non-threaded server
- ~37 sec with a lock
- ~33 sec with a try_lock

> A more elaborate approach would be to enqueue the refresh job into the
> queue. =A0May need the iothread enabled so we have qemu_mutex.

Maybe, but I'd like to wait the generic async work subsystem before
adding different kind of jobs to the queue. And it's already a big
improvment over the current code :).

> btw, I could not find other uses of vd->mutex, shouldn't it protect again=
st
> the work thread?

Check vnc-jobs.c, there is a qemu_mutex_lock(vs->vd->mutex);

--=20
Corentin Chary
http://xf.iksaif.net