<!DOCTYPE html><html><head><title></title><style type="text/css">p.MsoNormal,p.MsoNoSpacing{margin:0}
p.MsoNormal,p.MsoNoSpacing{margin:0}</style></head><body><div><br></div><div>On Mon, Feb 17, 2020, at 14:35, Rémi Denis-Courmont wrote:<br></div><blockquote id="qt" type="cite"><div>I want the APIs to work on all platforms, and this adds a generic POSIX implementation. Exactly what it says already. I don't know what else you want to explain.<br></div></blockquote><div><br></div><div>So why you don't write it in the commit log ? <br></div><div><br></div><div><div>Maybe it's just me, but I had trouble understanding this set. Explaining why you do a commit can be very helpful. Specially on set like this where you get the reason why only after the 4th or 5th commit.<br></div><div><br></div></div><div>Commit messages should explain 'why'. The 'what' should be in the diff/code/comment.<br></div><div><div><br></div></div><blockquote id="qt" type="cite"><div><br></div><div class="qt-gmail_quote"><div>Le 17 février 2020 14:39:08 GMT+02:00, Thomas Guillem <thomas@gllm.fr> a écrit :<br></div><blockquote class="qt-gmail_quote" style="margin-top:0pt;margin-right:0pt;margin-bottom:0pt;margin-left:0.8ex;border-left-color:rgb(204, 204, 204);border-left-style:solid;border-left-width:1px;padding-left:1ex;"><pre class="qt-k9mail"><div><br></div><div><br></div><div>On Sun, Feb 16, 2020, at 18:50, Rémi Denis-Courmont wrote:<br></div><blockquote class="qt-gmail_quote" style="margin-top:0pt;margin-right:0pt;margin-bottom:1ex;margin-left:0.8ex;border-left-color:rgb(114, 159, 207);border-left-style:solid;border-left-width:1px;padding-left:1ex;"><div><hr> src/Makefile.am | 3 ++<br></div><div> src/posix/wait.c | 128 +++++++++++++++++++++++++++++++++++++++++++++++<br></div><div> 2 files changed, 131 insertions(+)<br></div><div> create mode 100644 src/posix/wait.c<br></div><div><br></div><div>diff --git a/src/Makefile.am b/src/Makefile.am<br></div><div>index f2adf61095..0ad47fc105 100644<br></div><div>--- a/src/Makefile.am<br></div><div>+++ b/src/Makefile.am<br></div><div>@@ -482,6 +482,9 @@ libvlccore_la_SOURCES += \<br></div><div> posix/plugin.c \<br></div><div> posix/rand.c \<br></div><div> posix/timer.c<br></div><div>+if !HAVE_LINUX<br></div><div>+libvlccore_la_SOURCES += posix/wait.c<br></div><div>+endif<br></div><div> if !HAVE_ANDROID<br></div><div> libvlccore_la_SOURCES += posix/sort.c<br></div><div> if !HAVE_DARWIN<br></div><div>diff --git a/src/posix/wait.c b/src/posix/wait.c<br></div><div>new file mode 100644<br></div><div>index 0000000000..60f9c7626f<br></div><div>--- /dev/null<br></div><div>+++ b/src/posix/wait.c<br></div><div>@@ -0,0 +1,128 @@<br></div><div>+/*****************************************************************************<br></div><div>+ * wait.c : pthread back-end for vlc_atomic_wait<br></div><div>+ *****************************************************************************<br></div><div>+ * Copyright (C) 2019-2020 Rémi Denis-Courmont<br></div><div>+ *<br></div><div>+ * This program is free software; you can redistribute it and/or modify it<br></div><div>+ * under the terms of the GNU Lesser General Public License as published by<br></div><div>+ * the Free Software Foundation; either version 2.1 of the License, or<br></div><div>+ * (at your option) any later version.<br></div><div>+ *<br></div><div>+ * This program is distributed in the hope that it will be useful,<br></div><div>+ * but WITHOUT ANY WARRANTY; without even the implied warranty of<br></div><div>+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the<br></div><div>+ * GNU Lesser General Public License for more details.<br></div><div>+ *<br></div><div>+ * You should have received a copy of the GNU Lesser General Public License<br></div><div>+ * along with this program; if not, write to the Free Software Foundation,<br></div><div>+ * Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301, USA.<br></div><div>+ *****************************************************************************/<br></div><div>+<br></div><div>+#ifdef HAVE_CONFIG_H<br></div><div>+# include "config.h"<br></div><div>+#endif<br></div><div>+<br></div><div>+#include <vlc_common.h><br></div><div>+<br></div><div>+#include <stdalign.h><br></div><div>+#include <stdatomic.h><br></div><div>+#include <errno.h><br></div><div>+#include <time.h><br></div><div>+<br></div><div>+#include <sys/types.h><br></div><div>+#include <pthread.h><br></div><div>+<br></div><div>+#define WAIT_BUCKET_INIT \<br></div><div>+ { PTHREAD_MUTEX_INITIALIZER, PTHREAD_COND_INITIALIZER, 0 }<br></div><div>+#define WAIT_BUCKET_INIT_2 WAIT_BUCKET_INIT, WAIT_BUCKET_INIT<br></div><div>+#define WAIT_BUCKET_INIT_4 WAIT_BUCKET_INIT_2, WAIT_BUCKET_INIT_2<br></div><div>+#define WAIT_BUCKET_INIT_8 WAIT_BUCKET_INIT_4, WAIT_BUCKET_INIT_4<br></div><div>+#define WAIT_BUCKET_INIT_16 WAIT_BUCKET_INIT_8, WAIT_BUCKET_INIT_8<br></div><div>+#define WAIT_BUCKET_INIT_32 WAIT_BUCKET_INIT_16, WAIT_BUCKET_INIT_16<br></div></blockquote><div><br></div><div><br></div><div>If I understand correctly, this bucket is here to minimize the number of spurious wake, but it doesn't prevent it from happening, which is fine ?<br></div><div><br></div><blockquote class="qt-gmail_quote" style="margin-top:0pt;margin-right:0pt;margin-bottom:1ex;margin-left:0.8ex;border-left-color:rgb(114, 159, 207);border-left-style:solid;border-left-width:1px;padding-left:1ex;"><div>+<br></div><div>+static struct wait_bucket<br></div><div>+{<br></div><div>+ pthread_mutex_t lock;<br></div><div>+ pthread_cond_t wait;<br></div><div>+ unsigned waiters;<br></div><div>+} wait_buckets[32] = { WAIT_BUCKET_INIT_32 };<br></div><div>+<br></div><div>+static struct wait_bucket *wait_bucket_get(atomic_uint *addr)<br></div><div>+{<br></div><div>+ uintptr_t u = (uintptr_t)addr;<br></div><div>+ size_t idx = (u / alignof (*addr)) % ARRAY_SIZE(wait_buckets);<br></div><div>+<br></div><div>+ return &wait_buckets[idx];<br></div><div>+}<br></div><div>+<br></div><div>+static struct wait_bucket *wait_bucket_enter(atomic_uint *addr)<br></div><div>+{<br></div><div>+ struct wait_bucket *bucket = wait_bucket_get(addr);<br></div><div>+<br></div><div>+ pthread_mutex_lock(&bucket->lock);<br></div><div>+ bucket->waiters++;<br></div><div>+ return bucket;<br></div><div>+}<br></div><div>+<br></div><div>+static void wait_bucket_leave(void *data)<br></div><div>+{<br></div><div>+ struct wait_bucket *bucket = data;<br></div><div>+<br></div><div>+ bucket->waiters--;<br></div><div>+ pthread_mutex_unlock(&bucket->lock);<br></div><div>+}<br></div><div>+<br></div><div>+void vlc_atomic_wait(void *addr, unsigned value)<br></div><div>+{<br></div><div>+ atomic_uint *futex = addr;<br></div><div>+ struct wait_bucket *bucket = wait_bucket_enter(futex);<br></div><div>+<br></div><div>+ pthread_cleanup_push(wait_bucket_leave, bucket);<br></div><div>+<br></div><div>+ if (value == atomic_load_explicit(futex, memory_order_relaxed))<br></div><div>+ pthread_cond_wait(&bucket->wait, &bucket->lock);<br></div><div>+<br></div><div>+ pthread_cleanup_pop(1);<br></div><div>+}<br></div><div>+<br></div><div>+bool vlc_atomic_timedwait(void *addr, unsigned value, vlc_tick_t delay)<br></div><div>+{<br></div><div>+ atomic_uint *futex = addr;<br></div><div>+ struct wait_bucket *bucket;<br></div><div>+ struct timespec ts;<br></div><div>+ lldiv_t d = lldiv(delay, CLOCK_FREQ);<br></div><div>+ int ret = 0;<br></div><div>+<br></div><div>+ clock_gettime(CLOCK_MONOTONIC, &ts);<br></div><div>+ ts.tv_sec += d.quot;<br></div><div>+ ts.tv_nsec += NS_FROM_VLC_TICK(d.rem);<br></div><div>+<br></div><div>+ if (ts.tv_nsec >= 1000000000) {<br></div><div>+ ts.tv_sec++;<br></div><div>+ ts.tv_nsec -= 1000000000;<br></div><div>+ }<br></div><div>+<br></div><div>+ bucket = wait_bucket_enter(futex);<br></div><div>+ pthread_cleanup_push(wait_bucket_leave, bucket);<br></div><div>+<br></div><div>+ if (value == atomic_load_explicit(futex, memory_order_relaxed))<br></div><div>+ ret = pthread_cond_timedwait(&bucket->wait, &bucket->lock, &ts);<br></div><div>+<br></div><div>+ pthread_cleanup_pop(1);<br></div><div>+ return ret == 0;<br></div><div>+}<br></div><div>+<br></div><div>+void vlc_atomic_notify_one(void *addr)<br></div><div>+{<br></div><div>+ vlc_atomic_notify_all(addr);<br></div><div>+}<br></div><div>+<br></div><div>+void vlc_atomic_notify_all(void *addr)<br></div><div>+{<br></div><div>+ struct wait_bucket *bucket = wait_bucket_get(addr);<br></div><div>+<br></div><div>+ pthread_mutex_lock(&bucket->lock);<br></div><div>+ if (bucket->waiters > 0)<br></div><div>+ pthread_cond_broadcast(&bucket->wait);<br></div><div>+ pthread_mutex_unlock(&bucket->lock);<br></div><div>+}<br></div></blockquote><div><br></div><div>OK for me, but it deserves a git message and/or few comments. It took me some time to understand what you wanted to do.<br></div><div><br></div><blockquote class="qt-gmail_quote" style="margin-top:0pt;margin-right:0pt;margin-bottom:1ex;margin-left:0.8ex;border-left-color:rgb(114, 159, 207);border-left-style:solid;border-left-width:1px;padding-left:1ex;"><div>-- <br></div><div>2.25.0<hr>vlc-devel mailing list<br></div><div>To unsubscribe or modify your subscription options:<br></div><div><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br></div></blockquote><div><hr>vlc-devel mailing list<br></div><div>To unsubscribe or modify your subscription options:<br></div><div><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br></div></pre></blockquote></div><div><br></div><div>-- <br></div><div>Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté. <br></div><div>_______________________________________________<br></div><div>vlc-devel mailing list<br></div><div>To unsubscribe or modify your subscription options:<br></div><div>https://mailman.videolan.org/listinfo/vlc-devel<br></div></blockquote><div><br></div></body></html>