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

Proposed by Viko Adi Rahmawan on 2015-08-23
Status: Rejected
Rejected by: Daniel Fore on 2017-04-20
Proposed branch: lp:~vikoadi/slingshot/async_update_menu
Merge into: lp:~elementary-pantheon/slingshot/trunk
Diff against target: 227 lines (+63/-42)
4 files modified
src/Backend/AppSystem.vala (+46/-18)
src/Backend/SynapseSearch.vala (+12/-4)
src/SlingshotView.vala (+5/-8)
src/Widgets/CategoryView.vala (+0/-12)
To merge this branch: bzr merge lp:~vikoadi/slingshot/async_update_menu
Reviewer Review Type Date Requested Status
Cody Garver Needs Fixing on 2016-05-02
xapantu (community) 2015-08-23 Approve on 2015-09-23
Review via email: mp+268855@code.launchpad.net

Description of the change

* asynchronousely update_app_system on AppSystem backend and Synapse plugin registration

To post a comment you must log in.
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) wrote :

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
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) wrote :

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) wrote :
Download full text (33.4 KiB)

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) wrote :

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) wrote :

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

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) wrote :

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

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

dont wait for data-sink

xapantu (xapantu) wrote :

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
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) wrote :

good to hear that it works in wingpanel too

Cody Garver (codygarver) wrote :

Conflicts with trunk

review: Needs Fixing
Zisu Andrei (matzipan) wrote :

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/Backend/AppSystem.vala'
2--- src/Backend/AppSystem.vala 2014-08-16 15:13:42 +0000
3+++ src/Backend/AppSystem.vala 2015-09-24 06:06:49 +0000
4@@ -24,6 +24,8 @@
5 private Gee.ArrayList<GMenu.TreeDirectory> categories = null;
6 private Gee.HashMap<string, Gee.ArrayList<App>> apps = null;
7 private GMenu.Tree apps_menu = null;
8+ private Cancellable update_category_cancellable = null;
9+ private Cancellable update_apps_cancellable = null;
10
11 #if HAVE_ZEITGEIST
12 private RelevancyService rl_service;
13@@ -38,42 +40,61 @@
14 #endif
15
16 apps_menu = new GMenu.Tree ("pantheon-applications.menu", GMenu.TreeFlags.INCLUDE_EXCLUDED | GMenu.TreeFlags.SORT_DISPLAY_NAME);
17- apps_menu.changed.connect (update_app_system);
18+ apps_menu.changed.connect (()=>{update_app_system_async.begin ();});
19
20 apps = new Gee.HashMap<string, Gee.ArrayList<App>> ();
21 categories = new Gee.ArrayList<GMenu.TreeDirectory> ();
22+ update_category_cancellable = new Cancellable ();
23+ update_apps_cancellable = new Cancellable ();
24
25- update_app_system ();
26+ update_app_system_async.begin ();
27 }
28
29- private void update_app_system () {
30+ private async void update_app_system_async () {
31 debug ("Updating Applications menu tree...");
32 #if HAVE_ZEITGEIST
33+ Idle.add (update_app_system_async.callback);
34+ yield;
35 rl_service.refresh_popularity ();
36 #endif
37+
38 try {
39+ Idle.add (update_app_system_async.callback);
40+ yield;
41 apps_menu.load_sync ();
42 } catch (Error e) {
43 warning (e.message);
44 }
45
46- update_categories_index ();
47- update_apps ();
48-
49- changed ();
50- }
51-
52- private void update_categories_index () {
53+ update_category_cancellable.cancel ();
54 categories.clear ();
55-
56- var iter = apps_menu.get_root_directory ().iter ();
57+ update_category_cancellable.reset ();
58+ yield update_categories_index_async (update_category_cancellable);
59+
60+ update_apps_cancellable.cancel ();
61+ apps.clear ();
62+ update_apps_cancellable.reset ();
63+ yield update_apps_async (update_apps_cancellable);
64+ }
65+
66+ private async void update_categories_index_async (Cancellable cancellable) {
67+
68+ GMenu.TreeIter iter;
69+ iter = apps_menu.get_root_directory ().iter ();
70+
71 GMenu.TreeItemType type;
72 while ((type = iter.next ()) != GMenu.TreeItemType.INVALID) {
73+ uint handle = Idle.add (update_categories_index_async.callback);
74+ var c_handler = cancellable.connect (() => {Source.remove (handle);});
75+ yield;
76+
77 if (type == GMenu.TreeItemType.DIRECTORY) {
78 var dir = iter.get_directory ();
79 if (!dir.get_is_nodisplay ())
80 categories.add (dir);
81 }
82+
83+ cancellable.disconnect (c_handler);
84 }
85 }
86
87@@ -85,10 +106,18 @@
88 }
89 #endif
90
91- private void update_apps () {
92- apps.clear ();
93- foreach (var cat in categories)
94- apps.set (cat.get_name (), get_apps_by_category (cat));
95+ private async void update_apps_async (Cancellable cancellable) {
96+ foreach (var cat in categories) {
97+ GMenu.TreeDirectory category = cat;
98+ uint handle = Idle.add (update_apps_async.callback);
99+ var c_handler = cancellable.connect (() => {Source.remove (handle);});
100+ yield;
101+
102+ apps.set (category.get_name (), get_apps_by_category (category));
103+ changed ();
104+
105+ cancellable.disconnect(c_handler);
106+ }
107 }
108
109 public Gee.ArrayList<GMenu.TreeDirectory> get_categories () {
110@@ -133,7 +162,6 @@
111 && (GCC_PANEL_CATEGORY in app.categories
112 || SWITCHBOARD_PLUG_CATEGORY in app.categories))
113 continue;
114-
115
116 if (!(app.exec in sorted_apps_execs)) {
117 sorted_apps.insert_sorted_with_data (app, Utils.sort_apps_by_name);
118@@ -144,4 +172,4 @@
119
120 return sorted_apps;
121 }
122-}
123\ No newline at end of file
124+}
125
126=== modified file 'src/Backend/SynapseSearch.vala'
127--- src/Backend/SynapseSearch.vala 2015-06-03 19:10:24 +0000
128+++ src/Backend/SynapseSearch.vala 2015-09-24 06:06:49 +0000
129@@ -36,15 +36,23 @@
130 public SynapseSearch () {
131
132 if (sink == null) {
133- sink = new Synapse.DataSink ();
134- foreach (var plugin in plugins) {
135- sink.register_static_plugin (plugin);
136- }
137+ register_static_plugins_async.begin ();
138
139 favicon_cache = new Gee.HashMap<string,Gdk.Pixbuf> ();
140 }
141 }
142
143+ private async void register_static_plugins_async () {
144+ Idle.add (register_static_plugins_async.callback);
145+ yield;
146+ sink = new Synapse.DataSink ();
147+ foreach (var plugin in plugins) {
148+ Idle.add_full (Priority.LOW, register_static_plugins_async.callback);
149+ yield;
150+ sink.register_static_plugin (plugin);
151+ }
152+ }
153+
154 public async Gee.List<Synapse.Match>? search (string text, Synapse.SearchProvider? provider = null) {
155
156 if (current_search != null)
157
158=== modified file 'src/SlingshotView.vala'
159--- src/SlingshotView.vala 2015-02-01 15:13:15 +0000
160+++ src/SlingshotView.vala 2015-09-24 06:06:49 +0000
161@@ -87,16 +87,14 @@
162 this.set_type_hint (Gdk.WindowTypeHint.MENU);
163 this.focus_on_map = true;
164
165- // Have the window in the right place
166- read_settings (true);
167
168 Slingshot.icon_theme = Gtk.IconTheme.get_default ();
169-
170+ categories = new Gee.ArrayList<GMenu.TreeDirectory> ();
171+ apps = new Gee.HashMap<string, Gee.ArrayList<Backend.App>> ();
172 app_system = new Backend.AppSystem ();
173 synapse = new Backend.SynapseSearch ();
174-
175- categories = app_system.get_categories ();
176- apps = app_system.get_apps ();
177+ // Have the window in the right place
178+ read_settings (true);
179
180 primary_monitor = screen.get_primary_monitor ();
181 Gdk.Rectangle geometry;
182@@ -108,6 +106,7 @@
183 setup_ui ();
184
185 connect_signals ();
186+
187 debug ("Apps loaded");
188 }
189
190@@ -830,8 +829,6 @@
191 grid_view.append (app_entry);
192 app_entry.show_all ();
193 }
194-
195- stack.set_visible_child_name ("normal");
196 }
197
198 private void read_settings (bool first_start = false, bool check_columns = true, bool check_rows = true) {
199
200=== modified file 'src/Widgets/CategoryView.vala'
201--- src/Widgets/CategoryView.vala 2015-01-17 23:56:32 +0000
202+++ src/Widgets/CategoryView.vala 2015-09-24 06:06:49 +0000
203@@ -52,7 +52,6 @@
204 add (container);
205
206 connect_events ();
207- setup_sidebar ();
208 }
209
210 public void setup_sidebar () {
211@@ -72,17 +71,6 @@
212 }
213 category_switcher.show_all ();
214
215- int minimum_width;
216- category_switcher.get_preferred_width (out minimum_width, null);
217-
218- // Because of the different sizes of the column widget, we need to calculate if it will fit.
219- int removing_columns = (int)((double)minimum_width / (double)Pixels.ITEM_SIZE);
220- if (minimum_width % Pixels.ITEM_SIZE != 0)
221- removing_columns++;
222-
223- int columns = view.columns - removing_columns;
224- app_view.resize (view.rows, columns);
225-
226 category_switcher.selected = old_selected;
227 }
228

Subscribers

People subscribed via source and target branches