index-pack: work around thread-unsafe pread()
Multi-threaing of index-pack was disabled with c0f8654 (index-pack: Disable threading on cygwin - 2012-06-26), because pread() implementations for Cygwin and MSYS were not thread safe. Recent Cygwin does offer usable pread() and we enabled multi-threading with 103d530f (Cygwin 1.7 has thread-safe pread, 2013-07-19). Work around this problem on platforms with a thread-unsafe pread() emulation by opening one file handle per thread; it would prevent parallel pread() on different file handles from stepping on each other. Also remove NO_THREAD_SAFE_PREAD that was introduced in c0f8654 because it's no longer used anywhere. This workaround is unconditional, even for platforms with thread-safe pread() because the overhead is small (a couple file handles more) and not worth fragmenting the code. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Tested-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Nguyễn Thái Ngọc Duy committed
Mar 25, 2014 at 20:41 UTC
39539495acb24abfb4dee551e3e9f2e696be7abf
3 files changed
+17
-18
Makefile
-7
@@ -191,9 +191,6 @@ all::
191
# Define NO_STRUCT_ITIMERVAL if you don't have struct itimerval
192
# This also implies NO_SETITIMER
193
#
194
-# Define NO_THREAD_SAFE_PREAD if your pread() implementation is not
195
-# thread-safe. (e.g. compat/pread.c or cygwin)
196
-#
194
# Define NO_FAST_WORKING_DIRECTORY if accessing objects in pack files is
195
# generally faster on your platform than accessing the working directory.
196
#
@@ -1341,10 +1338,6 @@ endif
1338
ifdef NO_PREAD
1339
COMPAT_CFLAGS += -DNO_PREAD
1340
COMPAT_OBJS += compat/pread.o
1344
- NO_THREAD_SAFE_PREAD = YesPlease
1345
-endif
1346
-ifdef NO_THREAD_SAFE_PREAD
1347
- BASIC_CFLAGS += -DNO_THREAD_SAFE_PREAD
1341
endif
1342
ifdef NO_FAST_WORKING_DIRECTORY
1343
BASIC_CFLAGS += -DNO_FAST_WORKING_DIRECTORY
builtin/index-pack.c
+17
-10
@@ -40,17 +40,13 @@ struct base_data {
40
int ofs_first, ofs_last;
41
};
42
43
-#if !defined(NO_PTHREADS) && defined(NO_THREAD_SAFE_PREAD)
44
-/* pread() emulation is not thread-safe. Disable threading. */
45
-#define NO_PTHREADS
46
-#endif
47
-
43
struct thread_local {
44
#ifndef NO_PTHREADS
45
pthread_t thread;
46
#endif
47
struct base_data *base_cache;
48
size_t base_cache_used;
49
+ int pack_fd;
50
};
51
52
/*
@@ -91,7 +87,8 @@ static off_t consumed_bytes;
87
static unsigned deepest_delta;
88
static git_SHA_CTX input_ctx;
89
static uint32_t input_crc32;
94
-static int input_fd, output_fd, pack_fd;
90
+static int input_fd, output_fd;
91
+static const char *curr_pack;
92
93
#ifndef NO_PTHREADS
94
@@ -134,6 +131,7 @@ static inline void unlock_mutex(pthread_mutex_t *mutex)
131
*/
132
static void init_thread(void)
133
{
134
+ int i;
135
init_recursive_mutex(&read_mutex);
136
pthread_mutex_init(&counter_mutex, NULL);
137
pthread_mutex_init(&work_mutex, NULL);
@@ -141,11 +139,18 @@ static void init_thread(void)
139
pthread_mutex_init(&deepest_delta_mutex, NULL);
140
pthread_key_create(&key, NULL);
141
thread_data = xcalloc(nr_threads, sizeof(*thread_data));
142
+ for (i = 0; i < nr_threads; i++) {
143
+ thread_data[i].pack_fd = open(curr_pack, O_RDONLY);
144
+ if (thread_data[i].pack_fd == -1)
145
+ die_errno(_("unable to open %s"), curr_pack);
146
+ }
147
+
148
threads_active = 1;
149
}
150
151
static void cleanup_thread(void)
152
{
153
+ int i;
154
if (!threads_active)
155
return;
156
threads_active = 0;
@@ -154,6 +159,8 @@ static void cleanup_thread(void)
159
pthread_mutex_destroy(&work_mutex);
160
if (show_stat)
161
pthread_mutex_destroy(&deepest_delta_mutex);
162
+ for (i = 0; i < nr_threads; i++)
163
+ close(thread_data[i].pack_fd);
164
pthread_key_delete(key);
165
free(thread_data);
166
}
@@ -288,13 +295,13 @@ static const char *open_pack_file(const char *pack_name)
295
output_fd = open(pack_name, O_CREAT|O_EXCL|O_RDWR, 0600);
296
if (output_fd < 0)
297
die_errno(_("unable to create '%s'"), pack_name);
291
- pack_fd = output_fd;
298
+ nothread_data.pack_fd = output_fd;
299
} else {
300
input_fd = open(pack_name, O_RDONLY);
301
if (input_fd < 0)
302
die_errno(_("cannot open packfile '%s'"), pack_name);
303
output_fd = -1;
297
- pack_fd = input_fd;
304
+ nothread_data.pack_fd = input_fd;
305
}
306
git_SHA1_Init(&input_ctx);
307
return pack_name;
@@ -542,7 +549,7 @@ static void *unpack_data(struct object_entry *obj,
549
550
do {
551
ssize_t n = (len < 64*1024) ? len : 64*1024;
545
- n = pread(pack_fd, inbuf, n, from);
552
+ n = pread(get_thread_data()->pack_fd, inbuf, n, from);
553
if (n < 0)
554
die_errno(_("cannot pread pack file"));
555
if (!n)
@@ -1490,7 +1497,7 @@ static void show_pack_info(int stat_only)
1497
int cmd_index_pack(int argc, const char **argv, const char *prefix)
1498
{
1499
int i, fix_thin_pack = 0, verify = 0, stat_only = 0;
1493
- const char *curr_pack, *curr_index;
1500
+ const char *curr_index;
1501
const char *index_name = NULL, *pack_name = NULL;
1502
const char *keep_name = NULL, *keep_msg = NULL;
1503
char *index_name_buf = NULL, *keep_name_buf = NULL;
config.mak.uname
-1
@@ -158,7 +158,6 @@ ifeq ($(uname_O),Cygwin)
158
NO_SYMLINK_HEAD = YesPlease
159
NO_IPV6 = YesPlease
160
OLD_ICONV = UnfortunatelyYes
161
- NO_THREAD_SAFE_PREAD = YesPlease
161
# There are conflicting reports about this.
162
# On some boxes NO_MMAP is needed, and not so elsewhere.
163
# Try commenting this out if you suspect MMAP is more efficient