On 5/23/2019 1:51 AM, Jeff King wrote:
On Wed, May 22, 2019 at 09:23:39AM -0400, Jeff Hostetler wrote:
I was wondering about that too as the proper long term solution.
We would need (as the discussion suggests [1]) to properly
respect/represent the pthread_key_t argument.
For now, I've guarded my usage of pthread_getspecific() in the trace2
(similar to what index-pack does), so its not urgent that we update it.
And I'd rather we take this simple trace2 fix now and not try to combine
it with fixes for the pthread macros. Especially now as we're in the RC
cycle for 2.22.
Yeah, I think that makes sense.
I'll make a note to revisit the pthread code after 2.22.
For fun, here's a constant-time zero-allocation implementation that I
came up with. It passes t0211 with NO_PTHREADS, but I didn't test it
beyond that.
diff --git a/thread-utils.h b/thread-utils.h
index 4961487ed9..f466215742 100644
--- a/thread-utils.h
+++ b/thread-utils.h
@@ -18,7 +18,7 @@
#define pthread_t int
#define pthread_mutex_t int
#define pthread_cond_t int
-#define pthread_key_t int
+#define pthread_key_t git_pthread_key_t
#define pthread_mutex_init(mutex, attr) dummy_pthread_init(mutex)
#define pthread_mutex_lock(mutex)
@@ -31,16 +31,49 @@
#define pthread_cond_broadcast(cond)
#define pthread_cond_destroy(cond)
-#define pthread_key_create(key, attr) dummy_pthread_init(key)
-#define pthread_key_delete(key)
+#define pthread_key_create(key, destroy) git_pthread_key_create(key, destroy)
+#define pthread_key_delete(key) git_pthread_key_delete(key)
#define pthread_create(thread, attr, fn, data) \
dummy_pthread_create(thread, attr, fn, data)
#define pthread_join(thread, retval) \
dummy_pthread_join(thread, retval)
-#define pthread_setspecific(key, data)
-#define pthread_getspecific(key) NULL
+#define pthread_setspecific(key, data) git_pthread_setspecific(key, data)
+#define pthread_getspecific(key) git_pthread_getspecific(key)
+
+typedef struct {
+ void *data;
+ /* extra indirection because setspecific is passed key by value */
+ void **vdata;
+} git_pthread_key_t;
+
+static inline int git_pthread_key_create(git_pthread_key_t *key,
+ void (*destroy)(void *))
+{
+ key->data = NULL;
+ key->vdata = &key->data;
+ /* We don't use this; alternatively we could all via atexit(). */
+ if (destroy)
+ BUG("NO_PTHREADS does not support pthread key destructors");
+ return 0;
+}
+
+static inline int git_pthread_key_delete(git_pthread_key_t key)
+{
+ /* noop */
+ return 0;
+}
+
+static inline void git_pthread_setspecific(git_pthread_key_t key, void *data)
+{
+ *(key.vdata) = data;
+}
+
+static inline void *git_pthread_getspecific(git_pthread_key_t key)
+{
+ return key.data;
+}
int dummy_pthread_create(pthread_t *pthread, const void *attr,
void *(*fn)(void *), void *data);
Thanks! I'll play with this and submit something after 2.22 and
things slow down a bit.
Jeff
