From a209e54b8f75a07b2695c41053ae711a541d7ddf Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Sat, 18 Jul 2020 04:45:46 +0200 Subject: [PATCH] timsort: Add gtk_tim_sort_set_max_merge_size() Makes the SOrtListModel responsive when incrementally sorting. By making it configurable we can avoid losting performance in the non-incremental case. --- gtk/gtksortlistmodel.c | 16 +++++++++++++++ gtk/gtktimsort-impl.c | 22 ++++++++++---------- gtk/gtktimsort.c | 45 ++++++++++++++++++++++++++++++++--------- gtk/gtktimsortprivate.h | 8 ++++++++ 4 files changed, 70 insertions(+), 21 deletions(-) diff --git a/gtk/gtksortlistmodel.c b/gtk/gtksortlistmodel.c index 798ae24e56..5dfc6fb4cd 100644 --- a/gtk/gtksortlistmodel.c +++ b/gtk/gtksortlistmodel.c @@ -25,6 +25,21 @@ #include "gtkprivate.h" #include "gtktimsortprivate.h" +/* The maximum amount of items to merge for a single merge step + * + * Making this smaller will result in more steps, which has more overhead and slows + * down total sort time. + * Making it larger will result in fewer steps, which increases the time taken for + * a single step. + * + * As merges are the most expensive steps, this is essentially a tunable for the + * longest time spent in gtk_tim_sort_step(). + * + * Note that this should be reset to 0 when not doing incremental sorting to get + * rid of all the overhead. + */ +#define GTK_SORT_MAX_MERGE_SIZE (1024) + typedef struct _SortItem SortItem; struct _SortItem { @@ -234,6 +249,7 @@ gtk_sort_list_model_resort (GtkSortListModel *self, sizeof (SortItem), sort_func, self->sorter); + gtk_tim_sort_set_max_merge_size (&self->sort, GTK_SORT_MAX_MERGE_SIZE); gtk_tim_sort_set_runs (&self->sort, (gsize[2]) { already_sorted, 0 }); gtk_sort_list_model_start_sorting (self); diff --git a/gtk/gtktimsort-impl.c b/gtk/gtktimsort-impl.c index 3e048324e2..8878db6a41 100644 --- a/gtk/gtktimsort-impl.c +++ b/gtk/gtktimsort-impl.c @@ -828,13 +828,13 @@ gtk_tim_sort(merge_at) (GtkTimSort *self, /* Merge remaining runs, using tmp array with min(len1, len2) elements */ if (len1 <= len2) { - if (len1 > MAX_MERGE_PER_RUN) + if (len1 > self->max_merge_size) { - base1 = ELEM (self->run[i].base, self->run[i].len - MAX_MERGE_PER_RUN); - gtk_tim_sort(merge_lo) (self, base1, MAX_MERGE_PER_RUN, base2, len2); - self->run[i].len -= MAX_MERGE_PER_RUN; - self->run[i + 1].base = ELEM (self->run[i + 1].base, - MAX_MERGE_PER_RUN); - self->run[i + 1].len += MAX_MERGE_PER_RUN; + base1 = ELEM (self->run[i].base, self->run[i].len - self->max_merge_size); + gtk_tim_sort(merge_lo) (self, base1, self->max_merge_size, base2, len2); + self->run[i].len -= self->max_merge_size; + self->run[i + 1].base = ELEM (self->run[i + 1].base, - self->max_merge_size); + self->run[i + 1].len += self->max_merge_size; g_assert (ELEM (self->run[i].base, self->run[i].len) == self->run[i + 1].base); return; } @@ -845,12 +845,12 @@ gtk_tim_sort(merge_at) (GtkTimSort *self, } else { - if (len2 > MAX_MERGE_PER_RUN) + if (len2 > self->max_merge_size) { - gtk_tim_sort(merge_hi) (self, base1, len1, base2, MAX_MERGE_PER_RUN); - self->run[i].len += MAX_MERGE_PER_RUN; - self->run[i + 1].base = ELEM (self->run[i + 1].base, MAX_MERGE_PER_RUN); - self->run[i + 1].len -= MAX_MERGE_PER_RUN; + gtk_tim_sort(merge_hi) (self, base1, len1, base2, self->max_merge_size); + self->run[i].len += self->max_merge_size; + self->run[i + 1].base = ELEM (self->run[i + 1].base, self->max_merge_size); + self->run[i + 1].len -= self->max_merge_size; g_assert (ELEM (self->run[i].base, self->run[i].len) == self->run[i + 1].base); return; } diff --git a/gtk/gtktimsort.c b/gtk/gtktimsort.c index 38a46880e8..e76c915da0 100644 --- a/gtk/gtktimsort.c +++ b/gtk/gtktimsort.c @@ -25,16 +25,6 @@ */ #define MIN_MERGE 32 -/* - * Limit the amount of work done when merging. This ensures that the step - * function does not take too much time. Of coure, there's overhead associated - * with splitting a merge into miultliple merges. - * - * So lowering this number will make the average runtime of the step function - * faster but increase the total runtime. - */ -#define MAX_MERGE_PER_RUN 1024 - /* * When we get into galloping mode, we stay there until both runs win less * often than MIN_GALLOP consecutive times. @@ -84,6 +74,7 @@ gtk_tim_sort_init (GtkTimSort *self, self->data = data; self->min_gallop = MIN_GALLOP; + self->max_merge_size = G_MAXSIZE; self->min_run = compute_min_run (size); self->tmp = NULL; @@ -229,6 +220,40 @@ gtk_tim_sort_set_runs (GtkTimSort *self, gtk_tim_sort_push_run (self, self->base, runs[i]); } +/* + * gtk_tim_sort_set_max_merge_size: + * @self: a #GtkTimSort + * @max_merge_size: Maximum size of a merge step, 0 for unlimited + * + * Sets the maximum size of a merge step. Every time + * gtk_tim_sort_step() is called and a merge operation has to be + * done, the @max_merge_size will be used to limit the size of + * the merge. + * + * The benefit is that merges happen faster, and if you're using + * an incremental sorting algorithm in the main thread, this will + * limit the runtime. + * + * The disadvantage is that setting up merges is expensive and that + * various optimizations benefit from larger merges, so the total + * runtime of the sorting will increase with the number of merges. + * + * A good estimate is to set a @max_merge_size to 1024 for around + * 1ms runtimes, if your compare function is fast. + * + * By default, max_merge_size is set to unlimited. + **/ +void +gtk_tim_sort_set_max_merge_size (GtkTimSort *self, + gsize max_merge_size) +{ + g_return_if_fail (self != NULL); + + if (max_merge_size == 0) + max_merge_size = G_MAXSIZE; + self->max_merge_size = max_merge_size; +} + #if 1 #define WIDTH 4 #include "gtktimsort-impl.c" diff --git a/gtk/gtktimsortprivate.h b/gtk/gtktimsortprivate.h index f6ebd348e9..7472453556 100644 --- a/gtk/gtktimsortprivate.h +++ b/gtk/gtktimsortprivate.h @@ -55,6 +55,12 @@ struct _GtkTimSort gpointer base; gsize size; + /* + * The maximum size of a merge. It's guaranteed >0 and user-provided. + * See the comments for gtk_tim_sort_set_max_merge_size() for details. + */ + gsize max_merge_size; + /* * This controls when we get *into* galloping mode. It is initialized * to MIN_GALLOP. The mergeLo and mergeHi methods nudge it higher for @@ -99,6 +105,8 @@ void gtk_tim_sort_get_runs (GtkTimSort gsize runs[GTK_TIM_SORT_MAX_PENDING + 1]); void gtk_tim_sort_set_runs (GtkTimSort *self, gsize *runs); +void gtk_tim_sort_set_max_merge_size (GtkTimSort *self, + gsize max_merge_size); gboolean gtk_tim_sort_step (GtkTimSort *self);