summaryrefslogtreecommitdiff
path: root/packfile.c
diff options
context:
space:
mode:
authorMatheus Tavares <matheus.bernardino@usp.br>2020-01-15 23:39:53 -0300
committerJunio C Hamano <gitster@pobox.com>2020-01-17 13:52:14 -0800
commit31877c9aec21e0824fd4fcf415069cf8dfae4b72 (patch)
treeed78b3edfb08a2f65c47dede4d320af072b46c71 /packfile.c
parentb1fc9da1c84a94ef03eb07df361f3ec43006b39f (diff)
downloadgit-31877c9aec21e0824fd4fcf415069cf8dfae4b72.tar.gz
object-store: allow threaded access to object reading
Allow object reading to be performed by multiple threads protecting it with an internal lock, the obj_read_mutex. The lock usage can be toggled with enable_obj_read_lock() and disable_obj_read_lock(). Currently, the functions which can be safely called in parallel are: read_object_file_extended(), repo_read_object_file(), read_object_file(), read_object_with_reference(), read_object(), oid_object_info() and oid_object_info_extended(). It's also possible to use obj_read_lock() and obj_read_unlock() to protect other sections that cannot execute in parallel with object reading. Probably there are many spots in the functions listed above that could be executed unlocked (and thus, in parallel). But, for now, we are most interested in allowing parallel access to zlib inflation. This is one of the sections where object reading spends most of the time in (e.g. up to one-third of git-grep's execution time in the chromium repo corresponds to inflation) and it's already thread-safe. So, to take advantage of that, the obj_read_mutex is released when calling git_inflate() and re-acquired right after, for every calling spot in oid_object_info_extended()'s call chain. We may refine this lock to also exploit other possible parallel spots in the future, but for now, threaded zlib inflation should already give great speedups for threaded object reading callers. Note that add_delta_base_cache() was also modified to skip adding already present entries to the cache. This wasn't possible before, but it would be now, with the parallel inflation. Take for example the following situation, where two threads - A and B - are executing the code at unpack_entry(): 1. Thread A is performing the decompression of a base O (which is not yet in the cache) at PHASE II. Thread B is simultaneously trying to unpack O, but just starting at PHASE I. 2. Since O is not yet in the cache, B will go to PHASE II to also perform the decompression. 3. When they finish decompressing, one of them will get the object reading mutex and go to PHASE III while the other waits for the mutex. Let’s say A got the mutex first. 4. Thread A will add O to the cache, go throughout the rest of PHASE III and return. 5. Thread B gets the mutex, also add O to the cache (if the check wasn't there) and returns. Finally, it is also important to highlight that the object reading lock can only ensure thread-safety in the mentioned functions thanks to two complementary mechanisms: the use of 'struct raw_object_store's replace_mutex, which guards sections in the object reading machinery that would otherwise be thread-unsafe; and the 'struct pack_window's inuse_cnt, which protects window reading operations (such as the one performed during the inflation of a packed object), allowing them to execute without the acquisition of the obj_read_mutex. Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Diffstat (limited to 'packfile.c')
-rw-r--r--packfile.c32
1 files changed, 32 insertions, 0 deletions
diff --git a/packfile.c b/packfile.c
index 7e7c04e4d8..24a73fc33a 100644
--- a/packfile.c
+++ b/packfile.c
@@ -1086,7 +1086,23 @@ unsigned long get_size_from_delta(struct packed_git *p,
do {
in = use_pack(p, w_curs, curpos, &stream.avail_in);
stream.next_in = in;
+ /*
+ * Note: the window section returned by use_pack() must be
+ * available throughout git_inflate()'s unlocked execution. To
+ * ensure no other thread will modify the window in the
+ * meantime, we rely on the packed_window.inuse_cnt. This
+ * counter is incremented before window reading and checked
+ * before window disposal.
+ *
+ * Other worrying sections could be the call to close_pack_fd(),
+ * which can close packs even with in-use windows, and to
+ * reprepare_packed_git(). Regarding the former, mmap doc says:
+ * "closing the file descriptor does not unmap the region". And
+ * for the latter, it won't re-open already available packs.
+ */
+ obj_read_unlock();
st = git_inflate(&stream, Z_FINISH);
+ obj_read_lock();
curpos += stream.next_in - in;
} while ((st == Z_OK || st == Z_BUF_ERROR) &&
stream.total_out < sizeof(delta_head));
@@ -1445,6 +1461,14 @@ static void add_delta_base_cache(struct packed_git *p, off_t base_offset,
struct delta_base_cache_entry *ent = xmalloc(sizeof(*ent));
struct list_head *lru, *tmp;
+ /*
+ * Check required to avoid redundant entries when more than one thread
+ * is unpacking the same object, in unpack_entry() (since its phases I
+ * and III might run concurrently across multiple threads).
+ */
+ if (in_delta_base_cache(p, base_offset))
+ return;
+
delta_base_cached += base_size;
list_for_each_safe(lru, tmp, &delta_base_cache_lru) {
@@ -1574,7 +1598,15 @@ static void *unpack_compressed_entry(struct packed_git *p,
do {
in = use_pack(p, w_curs, curpos, &stream.avail_in);
stream.next_in = in;
+ /*
+ * Note: we must ensure the window section returned by
+ * use_pack() will be available throughout git_inflate()'s
+ * unlocked execution. Please refer to the comment at
+ * get_size_from_delta() to see how this is done.
+ */
+ obj_read_unlock();
st = git_inflate(&stream, Z_FINISH);
+ obj_read_lock();
if (!stream.avail_out)
break; /* the payload is larger than it should be */
curpos += stream.next_in - in;