Merge lp:~vikoadi/slingshot/async_update_menu into lp:~elementary-pantheon/slingshot/trunk

[Viko Adi Rahmawan]

* asynchronousely update_app_system on AppSystem backend and Synapse plugin registration

[lp:~vikoadi/slingshot/async_update_menu]

570. By Viko Adi Rahmawan on 2015-08-23

tidy up

571. By Viko Adi Rahmawan on 2015-08-24

dont animate if previous stack is search_view

572. By Viko Adi Rahmawan on 2015-08-24

remove spinner after itsn't used anymore

573. By Viko Adi Rahmawan on 2015-08-25

make synapse backend initialized asynchronously

[xapantu (xapantu)]

That's cool :)

However, as we discussed a few minutes ago, there is 2 main issues:

* Please add some Mutex, this way it is not thread safe, in AppSystem *and* in SynapseSearch.
* In the async function, if the thread creation fails, the function never finish, not sure it is a good thing, the yield should probably be before the catch.

(also some code style issues written in the diff comments)

review: Needs Fixing

[lp:~vikoadi/slingshot/async_update_menu]

574. By Viko Adi Rahmawan on 2015-09-03

lock shared variable

575. By Viko Adi Rahmawan on 2015-09-12

dont create a new thread, update on each category addition

576. By Viko Adi Rahmawan on 2015-09-12

dont screw to much code

[Viko Adi Rahmawan (vikoadi)]

I'm trying a completely different take, now we dont use different thread instead we update icon on using Idle cycle. Icon will be added on each every finished category.
this one should be much more simpler yet we show icon quicker although showing first popover will be a little bit slower (need to wait for first category to be completely created)

[xapantu (xapantu)]

Hum I just got this after a lot of launch:

xapantu@ntu /t/a/build> ./slingshot-launcher
[INFO 00:20:35.638194] Application.vala:155: Slingshot version: 0.8.1.1
[INFO 00:20:35.638579] Application.vala:157: Kernel version: 4.1.4-1-ARCH
[WARNING 00:20:35.786079] App.vala:192: Could not load icon. Falling back to method 2
[WARNING 00:20:35.786172] App.vala:225: Could not load icon. Falling back to method 4
[22:20:37.467840 Warning] App.vala:192: Could not load icon. Falling back to method 2
[22:20:37.467907 Warning] App.vala:225: Could not load icon. Falling back to method 4
*** Error in `./slingshot-launcher': free(): invalid next size (fast): 0x0000000001715a40 ***
======= Backtrace: =========
/usr/lib/libc.so.6(+0x72055)[0x7f9924dfd055]
/usr/lib/libc.so.6(+0x779a6)[0x7f9924e029a6]
/usr/lib/libc.so.6(+0x7818e)[0x7f9924e0318e]
/usr/lib/libglib-2.0.so.0(+0x37e5c)[0x7f9925464e5c]
/usr/lib/libglib-2.0.so.0(+0x3847f)[0x7f992546547f]
/usr/lib/libglib-2.0.so.0(+0x385b6)[0x7f99254655b6]
/usr/lib/libglib-2.0.so.0(g_string_chunk_insert_const+0x95)[0x7f9925498d75]
/usr/lib/libzeitgeist-2.0.so.0(zeitgeist_subject_set_interpretation+0x1c)[0x7f9925968fbc]
/usr/lib/libzeitgeist-2.0.so.0(zeitgeist_db_reader_get_subject_from_row+0x14b)[0x7f992597385b]
/usr/lib/libzeitgeist-2.0.so.0(zeitgeist_db_reader_get_events+0x1a8)[0x7f9925973bb8]
/usr/lib/libzeitgeist-2.0.so.0(zeitgeist_db_reader_find_events+0xe0)[0x7f9925976c00]
/usr/lib/libzeitgeist-2.0.so.0(+0x1b326)[0x7f9925958326]
/usr/lib/libzeitgeist-2.0.so.0(+0x1c9e3)[0x7f99259599e3]
/usr/lib/libglib-2.0.so.0(+0x70fb8)[0x7f992549dfb8]
/usr/lib/libglib-2.0.so.0(+0x70625)[0x7f992549d625]
/usr/lib/libpthread.so.0(+0x74a4)[0x7f99245424a4]
/usr/lib/libc.so.6(clone+0x6d)[0x7f9924e7413d]
======= Memory map: ========
00400000-004bc000 r-xp 00000000 00:22 182906 /tmp/async_update_menu/build/slingshot-launcher
006bc000-006be000 rw-p 000bc000 00:22 182906 /tmp/async_update_menu/build/slingshot-launcher
013eb000-03f6f000 rw-p 00000000 00:00 0 [heap]
7f98e8000000-7f98e8023000 rw-p 00000000 00:00 0
7f98e8023000-7f98ec000000 ---p 00000000 00:00 0
7f98f0000000-7f98f0076000 rw-p 00000000 00:00 0
7f98f0076000-7f98f4000000 ---p 00000000 00:00 0
7f98f4000000-7f98f404f000 rw-p 00000000 00:00 0
7f98f404f000-7f98f8000000 ---p 00000000 00:00 0
7f98f8000000-7f98f8022000 rw-p 00000000 00:00 0
7f98f8022000-7f98fc000000 ---p 00000000 00:00 0
7f98fd5e7000-7f98fe5e7000 rw-s 00000000 00:05 17825805 /SYSV00000000 (deleted)
7f98fe5e7000-7f98fe5fd000 r-xp 00000000 08:04 8129989 /usr/lib/libgcc_s.so.1
7f98fe5fd000-7f98fe7fc000 ---p 00016000 08:04 8129989 /usr/lib/libgcc_s.so.1
7f98fe7fc000-7f98fe7fd000 rw-p 00015000 08:04 8129989 /usr/lib/libgcc_s.so.1
7f98fe7fd000-7f98fe7fe000 ---p 00000000 00:00 0
7f98fe7fe000-7f98feffe000 rw-p 00000000 00:00 0 [stack:19193]
7f98feffe000-7f98fefff000 ---p 00000000 00:00 0
7f98fefff000-7f98ff7ff000 rw-p 00000000 00:00 0 [stack:19192]
7f98ff7ff000-7f98ff800000 ---p 00000000 00:00 0
7f98ff80000...

[xapantu (xapantu)]

See inline comments.

Ah, and do you notice performance improvements ? It's hard to mesure, but it looks like to be roughly the same here (however I have a rather fast computer and not so many apps).

[xapantu (xapantu)]

review: Needs Fixing

[xapantu (xapantu)]

About the crash, I will test more later, that may not be related to your branch, as it does not appear very often. The core dump is not usable I am afraid:

(gdb) bt
#0 0x00007f9924dbe5f8 in raise () from /usr/lib/libc.so.6
#1 0x00007f9924dbfa7a in abort () from /usr/lib/libc.so.6
#2 0x00007f9924dfd05a in __libc_message () from /usr/lib/libc.so.6
#3 0x00007f9924e029a6 in malloc_printerr () from /usr/lib/libc.so.6
#4 0x00007f9924e0318e in _int_free () from /usr/lib/libc.so.6
#5 0x00007f9925464e5c in ?? () from /usr/lib/libglib-2.0.so.0
#6 0x00007f992546547f in ?? () from /usr/lib/libglib-2.0.so.0
#7 0x00007f99254655b6 in ?? () from /usr/lib/libglib-2.0.so.0
#8 0x00007f9925498d75 in g_string_chunk_insert_const () from /usr/lib/libglib-2.0.so.0
#9 0x00007f9925968fbc in zeitgeist_subject_set_interpretation () from /usr/lib/libzeitgeist-2.0.so.0
#10 0x00007f992597385b in zeitgeist_db_reader_get_subject_from_row () from /usr/lib/libzeitgeist-2.0.so.0
#11 0x00007f9925973bb8 in zeitgeist_db_reader_get_events () from /usr/lib/libzeitgeist-2.0.so.0
#12 0x00007f9925976c00 in zeitgeist_db_reader_find_events () from /usr/lib/libzeitgeist-2.0.so.0
#13 0x00007f9925958326 in ?? () from /usr/lib/libzeitgeist-2.0.so.0
#14 0x00007f99259599e3 in ?? () from /usr/lib/libzeitgeist-2.0.so.0
#15 0x00007f992549dfb8 in ?? () from /usr/lib/libglib-2.0.so.0
#16 0x00007f992549d625 in ?? () from /usr/lib/libglib-2.0.so.0
#17 0x00007f99245424a4 in start_thread () from /usr/lib/libpthread.so.0
#18 0x00007f9924e7413d in clone () from /usr/lib/libc.so.6
(gdb) quit

[lp:~vikoadi/slingshot/async_update_menu]

577. By Viko Adi Rahmawan on 2015-09-16

use cancellation on appsystem update

578. By Viko Adi Rahmawan on 2015-09-16

rename to async

579. By Viko Adi Rahmawan on 2015-09-16

try to behave with zero apps

580. By Viko Adi Rahmawan on 2015-09-16

dont use implisit begin

581. By Viko Adi Rahmawan on 2015-09-16

merge trunk

[Viko Adi Rahmawan (vikoadi)]

now use cancellation to cancel previous update_appsystem.
this thing should not fasten anything, it just show popup earlier and then categories/apps are added incrementally to gridview. it is visible on my low performance computer

[lp:~vikoadi/slingshot/async_update_menu]

582. By Viko Adi Rahmawan on 2015-09-16

dont wait for data-sink

[xapantu (xapantu)]

Minor code-style issues that could be fixed, but perfect otherwise. Works fine in the indicator version too (however, in this one, we should have an api in wingpanel to decide when heavy loading can be done (to balance everything, if every indicator loads at startup, it will just be too heavy).

review: Approve

[lp:~vikoadi/slingshot/async_update_menu]

583. By Viko Adi Rahmawan on 2015-09-16

dont wait for refersh_popularty

584. By Viko Adi Rahmawan on 2015-09-24

tidy up

[Viko Adi Rahmawan (vikoadi)]

good to hear that it works in wingpanel too

[Cody Garver (codygarver)]

Conflicts with trunk

review: Needs Fixing

[Zisu Andrei (matzipan)]

It would be ashame to see all this work wasted. Merge trunk?

Unmerged revisions

584. By Viko Adi Rahmawan on 2015-09-24

tidy up

583. By Viko Adi Rahmawan on 2015-09-16

dont wait for refersh_popularty

582. By Viko Adi Rahmawan on 2015-09-16

dont wait for data-sink

581. By Viko Adi Rahmawan on 2015-09-16

merge trunk

580. By Viko Adi Rahmawan on 2015-09-16

dont use implisit begin

579. By Viko Adi Rahmawan on 2015-09-16

try to behave with zero apps

578. By Viko Adi Rahmawan on 2015-09-16

rename to async

577. By Viko Adi Rahmawan on 2015-09-16

use cancellation on appsystem update

576. By Viko Adi Rahmawan on 2015-09-12

dont screw to much code

575. By Viko Adi Rahmawan on 2015-09-12

dont create a new thread, update on each category addition

=== modified file 'src/Backend/AppSystem.vala'
--- src/Backend/AppSystem.vala	2014-08-16 15:13:42 +0000
+++ src/Backend/AppSystem.vala	2015-09-24 06:06:49 +0000
@@ -24,6 +24,8 @@
     private Gee.ArrayList<GMenu.TreeDirectory> categories = null;
     private Gee.HashMap<string, Gee.ArrayList<App>> apps = null;
     private GMenu.Tree apps_menu = null;
+    private Cancellable update_category_cancellable = null;
+    private Cancellable update_apps_cancellable = null;
 
 #if HAVE_ZEITGEIST
     private RelevancyService rl_service;
@@ -38,42 +40,61 @@
 #endif
 
         apps_menu = new GMenu.Tree ("pantheon-applications.menu", GMenu.TreeFlags.INCLUDE_EXCLUDED | GMenu.TreeFlags.SORT_DISPLAY_NAME);
-        apps_menu.changed.connect (update_app_system);
+        apps_menu.changed.connect (()=>{update_app_system_async.begin ();});
         
         apps = new Gee.HashMap<string, Gee.ArrayList<App>> ();
         categories = new Gee.ArrayList<GMenu.TreeDirectory> ();
+        update_category_cancellable = new Cancellable ();
+        update_apps_cancellable = new Cancellable ();
 
-        update_app_system ();
+        update_app_system_async.begin ();
     }
 
-    private void update_app_system () {
+    private async void update_app_system_async () {
         debug ("Updating Applications menu tree...");
 #if HAVE_ZEITGEIST
+        Idle.add (update_app_system_async.callback);
+        yield;
         rl_service.refresh_popularity ();
 #endif
+
         try {
+            Idle.add (update_app_system_async.callback);
+            yield;
             apps_menu.load_sync ();
         } catch (Error e) {
             warning (e.message);
         }
 
-        update_categories_index ();
-        update_apps ();
-
-        changed ();
-    }
-
-    private void update_categories_index () {
+        update_category_cancellable.cancel ();
         categories.clear ();
-
-        var iter = apps_menu.get_root_directory ().iter ();
+        update_category_cancellable.reset ();
+        yield update_categories_index_async (update_category_cancellable);
+
+        update_apps_cancellable.cancel ();
+        apps.clear ();
+        update_apps_cancellable.reset ();
+        yield update_apps_async (update_apps_cancellable);
+    }
+
+    private async void update_categories_index_async (Cancellable cancellable) {
+
+        GMenu.TreeIter iter;
+        iter = apps_menu.get_root_directory ().iter ();
+
         GMenu.TreeItemType type;
         while ((type = iter.next ()) != GMenu.TreeItemType.INVALID) {
+            uint handle = Idle.add (update_categories_index_async.callback);
+            var c_handler = cancellable.connect (() => {Source.remove (handle);});
+            yield;
+
             if (type == GMenu.TreeItemType.DIRECTORY) {
                 var dir = iter.get_directory ();
                 if (!dir.get_is_nodisplay ())
                     categories.add (dir);
             }
+
+            cancellable.disconnect (c_handler);
         }
     }
 
@@ -85,10 +106,18 @@
     }
 #endif
 
-    private void update_apps () {
-        apps.clear ();
-        foreach (var cat in categories)
-            apps.set (cat.get_name (), get_apps_by_category (cat));
+    private async void update_apps_async (Cancellable cancellable) {
+        foreach (var cat in categories) {
+            GMenu.TreeDirectory category = cat;
+            uint handle = Idle.add (update_apps_async.callback);
+            var c_handler = cancellable.connect (() => {Source.remove (handle);});
+            yield;
+
+            apps.set (category.get_name (), get_apps_by_category (category));
+            changed ();
+
+            cancellable.disconnect(c_handler);
+        }
     }
 
     public Gee.ArrayList<GMenu.TreeDirectory> get_categories () {
@@ -133,7 +162,6 @@
                     && (GCC_PANEL_CATEGORY in app.categories
                     || SWITCHBOARD_PLUG_CATEGORY in app.categories))
                     continue;
-                
 
                 if (!(app.exec in sorted_apps_execs)) {
                     sorted_apps.insert_sorted_with_data (app, Utils.sort_apps_by_name);
@@ -144,4 +172,4 @@
 
         return sorted_apps;
     }
-}
 \ No newline at end of file
+}
 
=== modified file 'src/Backend/SynapseSearch.vala'
--- src/Backend/SynapseSearch.vala	2015-06-03 19:10:24 +0000
+++ src/Backend/SynapseSearch.vala	2015-09-24 06:06:49 +0000
@@ -36,15 +36,23 @@
         public SynapseSearch () {
 
             if (sink == null) {
-                sink = new Synapse.DataSink ();
-                foreach (var plugin in plugins) {
-                    sink.register_static_plugin (plugin);
-                }
+                register_static_plugins_async.begin ();
 
                 favicon_cache = new Gee.HashMap<string,Gdk.Pixbuf> ();
             }
         }
 
+        private async void register_static_plugins_async () {
+            Idle.add (register_static_plugins_async.callback);
+            yield;
+            sink = new Synapse.DataSink ();
+            foreach (var plugin in plugins) {
+                Idle.add_full (Priority.LOW, register_static_plugins_async.callback);
+                yield;
+                sink.register_static_plugin (plugin);
+            }
+        }
+
         public async Gee.List<Synapse.Match>? search (string text, Synapse.SearchProvider? provider = null) {
 
             if (current_search != null)
 
=== modified file 'src/SlingshotView.vala'
--- src/SlingshotView.vala	2015-02-01 15:13:15 +0000
+++ src/SlingshotView.vala	2015-09-24 06:06:49 +0000
@@ -87,16 +87,14 @@
             this.set_type_hint (Gdk.WindowTypeHint.MENU);
             this.focus_on_map = true;
 
-            // Have the window in the right place
-            read_settings (true);
 
             Slingshot.icon_theme = Gtk.IconTheme.get_default ();
-
+            categories = new Gee.ArrayList<GMenu.TreeDirectory> ();
+            apps = new Gee.HashMap<string, Gee.ArrayList<Backend.App>> ();
             app_system = new Backend.AppSystem ();
             synapse = new Backend.SynapseSearch ();
-
-            categories = app_system.get_categories ();
-            apps = app_system.get_apps ();
+            // Have the window in the right place
+            read_settings (true);
 
             primary_monitor = screen.get_primary_monitor ();
             Gdk.Rectangle geometry;
@@ -108,6 +106,7 @@
             setup_ui ();
 
             connect_signals ();
+
             debug ("Apps loaded");
         }
 
@@ -830,8 +829,6 @@
                 grid_view.append (app_entry);
                 app_entry.show_all ();
             }
-
-            stack.set_visible_child_name ("normal");
         }
 
         private void read_settings (bool first_start = false, bool check_columns = true, bool check_rows = true) {
 
=== modified file 'src/Widgets/CategoryView.vala'
--- src/Widgets/CategoryView.vala	2015-01-17 23:56:32 +0000
+++ src/Widgets/CategoryView.vala	2015-09-24 06:06:49 +0000
@@ -52,7 +52,6 @@
         add (container);
 
         connect_events ();
-        setup_sidebar ();
     }
 
     public void setup_sidebar () {
@@ -72,17 +71,6 @@
         }
         category_switcher.show_all ();
 
-        int minimum_width;
-        category_switcher.get_preferred_width (out minimum_width, null);
-
-        // Because of the different sizes of the column widget, we need to calculate if it will fit.
-        int removing_columns = (int)((double)minimum_width / (double)Pixels.ITEM_SIZE);
-        if (minimum_width % Pixels.ITEM_SIZE != 0)
-            removing_columns++;
-
-        int columns = view.columns - removing_columns;
-        app_view.resize (view.rows, columns);
-
         category_switcher.selected = old_selected;
     }