Skip to content

Commit d9fc474

Browse files
derrickstoleedscho
authored andcommitted
pack-objects: thread the path-based compression
Adapting the implementation of ll_find_deltas(), create a threaded version of the --path-walk compression step in 'git pack-objects'. This involves adding a 'regions' member to the thread_params struct, allowing each thread to own a section of paths. We can simplify the way jobs are split because there is no value in extending the batch based on name-hash the way sections of the object entry array are attempted to be grouped. We re-use the 'list_size' and 'remaining' items for the purpose of borrowing work in progress from other "victim" threads when a thread has finished its batch of work more quickly. Using the Git repository as a test repo, the p5313 performance test shows that the resulting size of the repo is the same, but the threaded implementation gives gains of varying degrees depending on the number of objects being packed. (This was tested on a 16-core machine.) Test HEAD~1 HEAD ------------------------------------------------------------- 5313.6: thin pack with --path-walk 0.01 0.01 +0.0% 5313.7: thin pack size with --path-walk 475 475 +0.0% 5313.12: big pack with --path-walk 1.99 1.87 -6.0% 5313.13: big pack size with --path-walk 14.4M 14.3M -0.4% 5313.18: repack with --path-walk 98.14 41.46 -57.8% 5313.19: repack size with --path-walk 197.2M 197.3M +0.0% Signed-off-by: Derrick Stolee <[email protected]>
1 parent 940c8a2 commit d9fc474

File tree

2 files changed

+164
-5
lines changed

2 files changed

+164
-5
lines changed

builtin/pack-objects.c

Lines changed: 160 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2909,6 +2909,7 @@ static void find_deltas(struct object_entry **list, unsigned *list_size,
29092909
struct thread_params {
29102910
pthread_t thread;
29112911
struct object_entry **list;
2912+
struct packing_region *regions;
29122913
unsigned list_size;
29132914
unsigned remaining;
29142915
int window;
@@ -3222,6 +3223,163 @@ static void find_deltas_by_region(struct object_entry *list,
32223223
stop_progress(&progress_state);
32233224
}
32243225

3226+
static void *threaded_find_deltas_by_path(void *arg)
3227+
{
3228+
struct thread_params *me = arg;
3229+
3230+
progress_lock();
3231+
while (me->remaining) {
3232+
while (me->remaining) {
3233+
progress_unlock();
3234+
find_deltas_for_region(to_pack.objects,
3235+
me->regions,
3236+
me->processed);
3237+
progress_lock();
3238+
me->remaining--;
3239+
me->regions++;
3240+
}
3241+
3242+
me->working = 0;
3243+
pthread_cond_signal(&progress_cond);
3244+
progress_unlock();
3245+
3246+
/*
3247+
* We must not set ->data_ready before we wait on the
3248+
* condition because the main thread may have set it to 1
3249+
* before we get here. In order to be sure that new
3250+
* work is available if we see 1 in ->data_ready, it
3251+
* was initialized to 0 before this thread was spawned
3252+
* and we reset it to 0 right away.
3253+
*/
3254+
pthread_mutex_lock(&me->mutex);
3255+
while (!me->data_ready)
3256+
pthread_cond_wait(&me->cond, &me->mutex);
3257+
me->data_ready = 0;
3258+
pthread_mutex_unlock(&me->mutex);
3259+
3260+
progress_lock();
3261+
}
3262+
progress_unlock();
3263+
/* leave ->working 1 so that this doesn't get more work assigned */
3264+
return NULL;
3265+
}
3266+
3267+
static void ll_find_deltas_by_region(struct object_entry *list,
3268+
struct packing_region *regions,
3269+
uint32_t start, uint32_t nr)
3270+
{
3271+
struct thread_params *p;
3272+
int i, ret, active_threads = 0;
3273+
unsigned int processed = 0;
3274+
uint32_t progress_nr;
3275+
init_threaded_search();
3276+
3277+
if (!nr)
3278+
return;
3279+
3280+
progress_nr = regions[nr - 1].start + regions[nr - 1].nr;
3281+
if (delta_search_threads <= 1) {
3282+
find_deltas_by_region(list, regions, start, nr);
3283+
cleanup_threaded_search();
3284+
return;
3285+
}
3286+
3287+
if (progress > pack_to_stdout)
3288+
fprintf_ln(stderr, _("Path-based delta compression using up to %d threads"),
3289+
delta_search_threads);
3290+
CALLOC_ARRAY(p, delta_search_threads);
3291+
3292+
if (progress)
3293+
progress_state = start_progress(_("Compressing objects by path"),
3294+
progress_nr);
3295+
/* Partition the work amongst work threads. */
3296+
for (i = 0; i < delta_search_threads; i++) {
3297+
unsigned sub_size = nr / (delta_search_threads - i);
3298+
3299+
p[i].window = window;
3300+
p[i].depth = depth;
3301+
p[i].processed = &processed;
3302+
p[i].working = 1;
3303+
p[i].data_ready = 0;
3304+
3305+
p[i].regions = regions;
3306+
p[i].list_size = sub_size;
3307+
p[i].remaining = sub_size;
3308+
3309+
regions += sub_size;
3310+
nr -= sub_size;
3311+
}
3312+
3313+
/* Start work threads. */
3314+
for (i = 0; i < delta_search_threads; i++) {
3315+
if (!p[i].list_size)
3316+
continue;
3317+
pthread_mutex_init(&p[i].mutex, NULL);
3318+
pthread_cond_init(&p[i].cond, NULL);
3319+
ret = pthread_create(&p[i].thread, NULL,
3320+
threaded_find_deltas_by_path, &p[i]);
3321+
if (ret)
3322+
die(_("unable to create thread: %s"), strerror(ret));
3323+
active_threads++;
3324+
}
3325+
3326+
/*
3327+
* Now let's wait for work completion. Each time a thread is done
3328+
* with its work, we steal half of the remaining work from the
3329+
* thread with the largest number of unprocessed objects and give
3330+
* it to that newly idle thread. This ensure good load balancing
3331+
* until the remaining object list segments are simply too short
3332+
* to be worth splitting anymore.
3333+
*/
3334+
while (active_threads) {
3335+
struct thread_params *target = NULL;
3336+
struct thread_params *victim = NULL;
3337+
unsigned sub_size = 0;
3338+
3339+
progress_lock();
3340+
for (;;) {
3341+
for (i = 0; !target && i < delta_search_threads; i++)
3342+
if (!p[i].working)
3343+
target = &p[i];
3344+
if (target)
3345+
break;
3346+
pthread_cond_wait(&progress_cond, &progress_mutex);
3347+
}
3348+
3349+
for (i = 0; i < delta_search_threads; i++)
3350+
if (p[i].remaining > 2*window &&
3351+
(!victim || victim->remaining < p[i].remaining))
3352+
victim = &p[i];
3353+
if (victim) {
3354+
sub_size = victim->remaining / 2;
3355+
target->regions = victim->regions + victim->remaining - sub_size;
3356+
victim->list_size -= sub_size;
3357+
victim->remaining -= sub_size;
3358+
}
3359+
target->list_size = sub_size;
3360+
target->remaining = sub_size;
3361+
target->working = 1;
3362+
progress_unlock();
3363+
3364+
pthread_mutex_lock(&target->mutex);
3365+
target->data_ready = 1;
3366+
pthread_cond_signal(&target->cond);
3367+
pthread_mutex_unlock(&target->mutex);
3368+
3369+
if (!sub_size) {
3370+
pthread_join(target->thread, NULL);
3371+
pthread_cond_destroy(&target->cond);
3372+
pthread_mutex_destroy(&target->mutex);
3373+
active_threads--;
3374+
}
3375+
}
3376+
cleanup_threaded_search();
3377+
free(p);
3378+
3379+
display_progress(progress_state, progress_nr);
3380+
stop_progress(&progress_state);
3381+
}
3382+
32253383
static void prepare_pack(int window, int depth)
32263384
{
32273385
struct object_entry **delta_list;
@@ -3247,8 +3405,8 @@ static void prepare_pack(int window, int depth)
32473405
return;
32483406

32493407
if (path_walk)
3250-
find_deltas_by_region(to_pack.objects, to_pack.regions,
3251-
0, to_pack.nr_regions);
3408+
ll_find_deltas_by_region(to_pack.objects, to_pack.regions,
3409+
0, to_pack.nr_regions);
32523410

32533411
ALLOC_ARRAY(delta_list, to_pack.nr_objects);
32543412
nr_deltas = n = 0;

t/perf/p5313-pack-objects.sh

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ test_perf 'thin pack with --path-walk' '
4141
'
4242

4343
test_size 'thin pack size with --path-walk' '
44-
wc -c <out
44+
test_file_size out
4545
'
4646

4747
test_perf 'big pack' '
@@ -65,7 +65,7 @@ test_perf 'big pack with --path-walk' '
6565
'
6666

6767
test_size 'big pack size with --path-walk' '
68-
wc -c <out
68+
test_file_size out
6969
'
7070

7171
test_perf 'repack' '
@@ -91,7 +91,8 @@ test_perf 'repack with --path-walk' '
9191
'
9292

9393
test_size 'repack size with --path-walk' '
94-
wc -c <.git/objects/pack/pack-*.pack
94+
pack=$(ls .git/objects/pack/pack-*.pack) &&
95+
test_file_size "$pack"
9596
'
9697

9798
test_done

0 commit comments

Comments
 (0)