After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 751160 - gtask does unnecessary work
gtask does unnecessary work
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: gio
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2015-06-18 13:46 UTC by Matthias Clasen
Modified: 2015-06-29 15:22 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Add async queue api (3.55 KB, patch)
2015-06-18 14:39 UTC, Matthias Clasen
none Details | Review
Add threadpool api (2.67 KB, patch)
2015-06-18 14:40 UTC, Matthias Clasen
none Details | Review
avoid resorting (1019 bytes, patch)
2015-06-18 14:41 UTC, Matthias Clasen
none Details | Review
cleanup (1015 bytes, patch)
2015-06-18 14:41 UTC, Matthias Clasen
none Details | Review
Add async queue api (5.16 KB, patch)
2015-06-22 15:45 UTC, Matthias Clasen
committed Details | Review
Add tests (2.30 KB, patch)
2015-06-22 15:46 UTC, Matthias Clasen
committed Details | Review
Add threadpool api (2.93 KB, patch)
2015-06-22 15:46 UTC, Matthias Clasen
committed Details | Review
avoid resorting (1.00 KB, patch)
2015-06-22 15:47 UTC, Matthias Clasen
committed Details | Review
cleanup (1015 bytes, patch)
2015-06-22 15:47 UTC, Matthias Clasen
committed Details | Review

Description Matthias Clasen 2015-06-18 13:46:23 UTC
When a task is cancelled, gtask resorts the entire list of unprocessed items, when all we really want to do is move the cancelled task to the head of the queue.

This is a problem in situation where the queue gets long, as can be seen in bug 751130
Comment 1 Matthias Clasen 2015-06-18 14:39:58 UTC
Created attachment 305593 [details] [review]
Add async queue api
Comment 2 Matthias Clasen 2015-06-18 14:40:22 UTC
Created attachment 305594 [details] [review]
Add threadpool api
Comment 3 Matthias Clasen 2015-06-18 14:41:19 UTC
Created attachment 305595 [details] [review]
avoid resorting
Comment 4 Matthias Clasen 2015-06-18 14:41:40 UTC
Created attachment 305596 [details] [review]
cleanup
Comment 5 Dan Winship 2015-06-20 14:59:15 UTC
Comment on attachment 305593 [details] [review]
Add async queue api

seems a bit weird to add _unlocked() versions without the corresponding un-unlocked ones?

also, maybe test cases
Comment 6 Dan Winship 2015-06-20 15:00:08 UTC
Comment on attachment 305594 [details] [review]
Add threadpool api

not sure about the name, but ok
Comment 7 Dan Winship 2015-06-20 15:00:34 UTC
Comment on attachment 305595 [details] [review]
avoid resorting

ok
Comment 8 Dan Winship 2015-06-20 15:01:16 UTC
Comment on attachment 305596 [details] [review]
cleanup

yup
Comment 9 Matthias Clasen 2015-06-22 15:45:39 UTC
Created attachment 305838 [details] [review]
Add async queue api
Comment 10 Matthias Clasen 2015-06-22 15:46:04 UTC
Created attachment 305839 [details] [review]
Add tests
Comment 11 Matthias Clasen 2015-06-22 15:46:45 UTC
Created attachment 305840 [details] [review]
Add threadpool api
Comment 12 Matthias Clasen 2015-06-22 15:47:18 UTC
Created attachment 305841 [details] [review]
avoid resorting
Comment 13 Matthias Clasen 2015-06-22 15:47:44 UTC
Created attachment 305842 [details] [review]
cleanup
Comment 14 Matthias Clasen 2015-06-25 10:42:22 UTC
Review of attachment 305840 [details] [review]:

::: docs/reference/glib/glib-sections.txt
@@ +845,1 @@
 g_async_queue_pop

This hunk should be in the async queue patch
Comment 15 Dan Winship 2015-06-29 15:11:28 UTC
Comment on attachment 305838 [details] [review]
Add async queue api

> g_async_queue_push_sorted_unlocked
>+g_async_queue_push_front_unlocked
>+g_async_queue_remove_unlocked
> g_async_queue_pop_unlocked

need to add the non-_unlocked versions too

>+ * g_async_queue_push_front:
>+ * @queue: a #GAsyncQueue
>+ * @data: @data to push into the @queue
>+ *
>+ * Pushes the @data into the @queue. @data must not be %NULL.
>+ * In contrast to g_async_queue_push_unlocked(), this function

"In contrast to g_async_queue_push()"
Comment 16 Dan Winship 2015-06-29 15:12:04 UTC
Comment on attachment 305839 [details] [review]
Add tests

>From e5893324b86878c4b580f66d91fd7f06b0666721 Mon Sep 17 00:00:00 2001
>From: Matthias Clasen <mclasen@redhat.com>
>Date: Mon, 22 Jun 2015 11:35:06 -0400
>Subject: [PATCH 2/5] Add tests for new GAsyncQueue api
>
>---
> glib/tests/asyncqueue.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 48 insertions(+)
>
>diff --git a/glib/tests/asyncqueue.c b/glib/tests/asyncqueue.c
>index 5ccbb23..4a81d23 100644
>--- a/glib/tests/asyncqueue.c
>+++ b/glib/tests/asyncqueue.c
>@@ -220,6 +220,52 @@ test_async_queue_timed (void)
>   g_async_queue_unref (q);
> }
> 
>+static void
>+test_async_queue_remove (void)
>+{
>+  GAsyncQueue *q;
>+
>+  q = g_async_queue_new ();
>+
>+  g_async_queue_push (q, GINT_TO_POINTER (10));
>+  g_async_queue_push (q, GINT_TO_POINTER (2));
>+  g_async_queue_push (q, GINT_TO_POINTER (7));
>+  g_async_queue_push (q, GINT_TO_POINTER (1));
>+
>+  g_async_queue_remove (q, GINT_TO_POINTER (7));
>+
>+  g_assert_cmpint (GPOINTER_TO_INT (g_async_queue_pop (q)), ==, 10);
>+  g_assert_cmpint (GPOINTER_TO_INT (g_async_queue_pop (q)), ==, 2);
>+  g_assert_cmpint (GPOINTER_TO_INT (g_async_queue_pop (q)), ==, 1);
>+
>+  g_assert (g_async_queue_try_pop (q) == NULL);
>+
>+  g_async_queue_unref (q);
>+}
>+
>+static void
>+test_async_queue_push_front (void)
>+{
>+  GAsyncQueue *q;
>+
>+  q = g_async_queue_new ();
>+
>+  g_async_queue_push (q, GINT_TO_POINTER (10));
>+  g_async_queue_push (q, GINT_TO_POINTER (2));
>+  g_async_queue_push (q, GINT_TO_POINTER (7));
>+
>+  g_async_queue_push_front (q, GINT_TO_POINTER (1));
>+
>+  g_assert_cmpint (GPOINTER_TO_INT (g_async_queue_pop (q)), ==, 1);
>+  g_assert_cmpint (GPOINTER_TO_INT (g_async_queue_pop (q)), ==, 10);
>+  g_assert_cmpint (GPOINTER_TO_INT (g_async_queue_pop (q)), ==, 2);
>+  g_assert_cmpint (GPOINTER_TO_INT (g_async_queue_pop (q)), ==, 7);
>+
>+  g_assert (g_async_queue_try_pop (q) == NULL);
>+
>+  g_async_queue_unref (q);
>+}
>+
> int
> main (int argc, char *argv[])
> {
>@@ -229,6 +275,8 @@ main (int argc, char *argv[])
>   g_test_add_func ("/asyncqueue/destroy", test_async_queue_destroy);
>   g_test_add_func ("/asyncqueue/threads", test_async_queue_threads);
>   g_test_add_func ("/asyncqueue/timed", test_async_queue_timed);
>+  g_test_add_func ("/asyncqueue/remove", test_async_queue_remove);
>+  g_test_add_func ("/asyncqueue/push_front", test_async_queue_push_front);
> 
>   return g_test_run ();
> }
>-- 
>2.4.3
>
Comment 17 Dan Winship 2015-06-29 15:12:51 UTC
Comment on attachment 305840 [details] [review]
Add threadpool api

good other than the part that was supposed to be in the other patch
Comment 18 Matthias Clasen 2015-06-29 15:21:53 UTC
Pushed with the reshuffled hunk, and the one doc reference fixup.