From bf05ce161fb0ace60aa621b6e5dae00b49729b29 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 26 Mar 2008 10:38:54 +0000 Subject: notify the decoder instead of polling 100hz When the decoder process is faster than the player process, all decodedd buffers are full at some point in time. The decoder has to wait for buffers to become free (finished playing). It used to do this by polling the buffer status 100 times a second. This generates a lot of unnecessary CPU wakeups. This patch adds a way for the player process to notify the decoder process that it may continue its work. We could use pthread_cond for that, unfortunately inter-process mutexes/conds are not supported by some kernels (Linux), so we cannot use this light-weight method until mpd moves to using threads instead of processes. The other method would be semaphores, which historically are global resources with a unique name; this historic API is cumbersome, and I wanted to avoid it. I came up with a quite naive solution for now: I create an anonymous pipe with pipe(), and the decoder process reads on that pipe. Until the player process sends data on it as a signal, the decoder process blocks. This can be optimized in a number of ways: - if the decoder process is still working (instead of waiting for buffers), we could save the write() system call, since there is nobody waiting for the notification. [ew: I tried this using a counter in shared memory, didn't help] - the pipe buffer will be full at some point, when the decoder thread is too slow. For this reason, the writer side of the pipe is non-blocking, and mpd can ignore the resulting EWOULDBLOCK. - since we have shared memory, we could check whether somebody is actually waiting without a context switch, and we could just not write the notification byte. [ew: tried same method/result as first point above] - if there is already a notification in the pipe, we could also not write another one. [ew: tried same method/result as first/third points above] - the decoder will only consume 64 bytes at a time. If the pipe buffer is full, this will result in a lot of read() invocations. This does not hurt badly, but on a heavily loaded system, this might add a little bit more load. The preceding optimizations however are able eliminate the this. - finally, we should use another method for inter process notifications - maybe kill() or just make mpd use threads, finally. In spite of all these possibilities to optimize this code further, this pipe notification trick is faster than the 100 Hz poll. On my machine, it reduced the number of wakeups to less than 30%. git-svn-id: https://svn.musicpd.org/mpd/trunk@7215 09075e82-0dd4-0310-85a5-a0d7c8717e4f --- src/Makefile.am | 2 ++ src/decode.c | 1 + src/notify.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/notify.h | 47 ++++++++++++++++++++++++++++++++++++ src/outputBuffer.c | 4 +++- src/outputBuffer.h | 2 ++ 6 files changed, 125 insertions(+), 1 deletion(-) create mode 100644 src/notify.c create mode 100644 src/notify.h diff --git a/src/Makefile.am b/src/Makefile.am index 82e46e6ee..91accd88b 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -29,6 +29,7 @@ mpd_inputPlugins = \ mpd_headers = \ + notify.h \ ack.h \ audio.h \ audioOutput.h \ @@ -87,6 +88,7 @@ mpd_SOURCES = \ $(mpd_headers) \ $(mpd_audioOutputs) \ $(mpd_inputPlugins) \ + notify.c \ audio.c \ audioOutput.c \ buffer2array.c \ diff --git a/src/decode.c b/src/decode.c index 316cd34ba..3681e8025 100644 --- a/src/decode.c +++ b/src/decode.c @@ -623,6 +623,7 @@ static void decodeParent(PlayerControl * pc, DecoderControl * dc, OutputBuffer * cb->begin = 0; } else cb->begin++; + signalNotify(&cb->notify); } else if (cb->begin != end && cb->begin == next) { if (doCrossFade == 1 && nextChunk >= 0) { nextChunk = cb->begin + crossFadeChunks; diff --git a/src/notify.c b/src/notify.c new file mode 100644 index 000000000..85db46192 --- /dev/null +++ b/src/notify.c @@ -0,0 +1,70 @@ +/* the Music Player Daemon (MPD) + * Copyright (C) 2008 Max Kellermann + * This project's homepage is: http://www.musicpd.org + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#include "notify.h" + +#include +#include +#include + +int set_nonblock(int fd) +{ + int ret; + + assert(fd >= 0); + + ret = fcntl(fd, F_GETFL, 0); + if (ret < 0) + return ret; + + return fcntl(fd, F_SETFL, ret|O_NONBLOCK); +} + +int initNotify(Notify *notify) +{ + int ret; + + ret = pipe(notify->fds); + if (ret < 0) + return -1; + + ret = set_nonblock(notify->fds[1]); + if (ret < 0) { + close(notify->fds[0]); + close(notify->fds[1]); + return -1; + } + + return 0; +} + +int waitNotify(Notify *notify) +{ + char buffer[64]; + ssize_t nbytes; + + nbytes = read(notify->fds[0], buffer, sizeof(buffer)); + return (int)nbytes; +} + +int signalNotify(Notify *notify) +{ + char buffer[1] = { 0 }; + + return (int)write(notify->fds[1], &buffer, sizeof(buffer)); +} diff --git a/src/notify.h b/src/notify.h new file mode 100644 index 000000000..ce6396a96 --- /dev/null +++ b/src/notify.h @@ -0,0 +1,47 @@ +/* the Music Player Daemon (MPD) + * Copyright (C) 2008 Max Kellermann + * This project's homepage is: http://www.musicpd.org + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#ifndef NOTIFY_H +#define NOTIFY_H + +/* + * This library implements inter-process signalling using blocking + * read() on an anonymous pipe. As a side effect, the read() system + * call has the same signal interruption behaviour as the old sleep + * function. + * + * As soon as mpd uses threading instead of fork()/shm, we can replace + * this library with a pthread_cond object. + * + * This code is experimental and carries a lot of overhead. Still, it + * uses less resources than the old polling code with a fixed sleep + * time. + * + */ + +typedef struct _Notify { + int fds[2]; +} Notify; + +int initNotify(Notify *notify); + +int waitNotify(Notify *notify); + +int signalNotify(Notify *notify); + +#endif diff --git a/src/outputBuffer.c b/src/outputBuffer.c index 25c9b3c12..cda56d4b1 100644 --- a/src/outputBuffer.c +++ b/src/outputBuffer.c @@ -45,6 +45,8 @@ void initOutputBuffer(OutputBuffer * cb, char *chunks) (float *)(((char *)cb->metaChunk) + buffered_chunks * sizeof(mpd_sint8)); cb->acceptMetadata = 0; + + initNotify(&cb->notify); } void clearAllMetaChunkSets(OutputBuffer * cb) @@ -131,7 +133,7 @@ int sendDataToOutputBuffer(OutputBuffer * cb, InputStream * inStream, } if (!inStream || bufferInputStream(inStream) <= 0) { - my_usleep(10000); + waitNotify(&cb->notify); } } if (dc->stop) diff --git a/src/outputBuffer.h b/src/outputBuffer.h index 9bd9d48c8..99d3010f0 100644 --- a/src/outputBuffer.h +++ b/src/outputBuffer.h @@ -26,6 +26,7 @@ #include "inputStream.h" #include "metadataChunk.h" #include "replayGain.h" +#include "notify.h" #define OUTPUT_BUFFER_DC_STOP -1 #define OUTPUT_BUFFER_DC_SEEK -2 @@ -45,6 +46,7 @@ typedef struct _OutputBuffer { mpd_sint8 metaChunkSet[BUFFERED_METACHUNKS]; mpd_sint8 *volatile metaChunk; volatile mpd_sint8 acceptMetadata; + Notify notify; } OutputBuffer; void initOutputBuffer(OutputBuffer * cb, char *chunks); -- cgit v1.2.3