Merge lp:~charlesk/indicator-datetime/lp-1306112-query-eds-less-frequently-if-load-is-heavy into lp:indicator-datetime/14.04

Proposed by Charles Kerr
Status: Merged
Approved by: Bill Filler
Approved revision: 344
Merged at revision: 339
Proposed branch: lp:~charlesk/indicator-datetime/lp-1306112-query-eds-less-frequently-if-load-is-heavy
Merge into: lp:indicator-datetime/14.04
Diff against target: 262 lines (+78/-96)
4 files modified
include/datetime/appointment.h (+0/-2)
src/appointment.cpp (+0/-1)
src/engine-eds.cpp (+78/-91)
tests/manual-test-snap.cpp (+0/-2)
To merge this branch: bzr merge lp:~charlesk/indicator-datetime/lp-1306112-query-eds-less-frequently-if-load-is-heavy
Reviewer Review Type Date Requested Status
Bill Filler (community) Approve
PS Jenkins bot (community) continuous-integration Approve
Renato Araujo Oliveira Filho Pending
Review via email: mp+215497@code.launchpad.net

Commit message

If there's a large batch of EDS events coming in, try to wait until the event storm has passed before running our requery.

Description of the change

If there's a large batch of EDS events coming in, try to wait until the event storm has passed before running our requery.

Previously we re-queried after an interval of 200 msec. This patch progressively widens that window until the event storm subsides, or until a minute has passed, whichever comes first.

Made in collaboration w/Renato.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
340. By Charles Kerr

remove some extraneous EDS events on startup

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
341. By Charles Kerr

remove unused field Appointment.is_event

342. By Charles Kerr

remove unused field Appointment.is_daily

343. By Charles Kerr

sync with lp:~renatofilho/qtorganizer5-eds/fix-1284375's method of storing alarm urls. Happily this supercedes our code that used to call e_cal_client_get_attachment_uris(), so this means fewer round trip calls to EDS

344. By Charles Kerr

copyediting: fix indentation/formatting

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Bill Filler (bfiller) wrote :

Did you perform an exploratory manual test run of the code change and any related functionality on device or emulator?
yes

Did you successfully run all tests found in your component's Test Plan (https://wiki.ubuntu.com/Process/Merges/TestPlan/<package-name>) on device or emulator?
yes

Did CI run pass? If not, please explain why.
yes

Have you checked that submitter has accurately filled out the submitter checklist and has taken no shortcut?
yes

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'include/datetime/appointment.h'
2--- include/datetime/appointment.h 2014-01-30 19:00:22 +0000
3+++ include/datetime/appointment.h 2014-04-15 16:12:38 +0000
4@@ -39,8 +39,6 @@
5 std::string summary;
6 std::string url;
7 std::string uid;
8- bool is_event = false;
9- bool is_daily = false;
10 bool has_alarms = false;
11 DateTime begin;
12 DateTime end;
13
14=== modified file 'src/appointment.cpp'
15--- src/appointment.cpp 2014-01-30 19:00:22 +0000
16+++ src/appointment.cpp 2014-04-15 16:12:38 +0000
17@@ -33,7 +33,6 @@
18 && (summary==that.summary)
19 && (url==that.url)
20 && (uid==that.uid)
21- && (is_event==that.is_event)
22 && (has_alarms==that.has_alarms)
23 && (begin==that.begin)
24 && (end==that.end);
25
26=== modified file 'src/engine-eds.cpp'
27--- src/engine-eds.cpp 2014-03-10 02:08:47 +0000
28+++ src/engine-eds.cpp 2014-04-15 16:12:38 +0000
29@@ -25,6 +25,7 @@
30 #include <libedataserver/libedataserver.h>
31
32 #include <algorithm> // std::sort()
33+#include <ctime> // time()
34 #include <map>
35 #include <set>
36
37@@ -144,16 +145,29 @@
38 {
39 auto self = static_cast<Impl*>(gself);
40 self->m_rebuild_tag = 0;
41+ self->m_rebuild_deadline = 0;
42 self->set_dirty_now();
43 return G_SOURCE_REMOVE;
44 }
45
46 void set_dirty_soon()
47 {
48- static const int ARBITRARY_BATCH_MSEC = 200;
49-
50- if (m_rebuild_tag == 0)
51- m_rebuild_tag = g_timeout_add(ARBITRARY_BATCH_MSEC, set_dirty_now_static, this);
52+ static constexpr int MIN_BATCH_SEC = 1;
53+ static constexpr int MAX_BATCH_SEC = 60;
54+ static_assert(MIN_BATCH_SEC <= MAX_BATCH_SEC, "bad boundaries");
55+
56+ const auto now = time(nullptr);
57+
58+ if (m_rebuild_deadline == 0) // first pass
59+ {
60+ m_rebuild_deadline = now + MAX_BATCH_SEC;
61+ m_rebuild_tag = g_timeout_add_seconds(MIN_BATCH_SEC, set_dirty_now_static, this);
62+ }
63+ else if (now < m_rebuild_deadline)
64+ {
65+ g_source_remove (m_rebuild_tag);
66+ m_rebuild_tag = g_timeout_add_seconds(MIN_BATCH_SEC, set_dirty_now_static, this);
67+ }
68 }
69
70 static void on_source_registry_ready(GObject* /*source*/, GAsyncResult* res, gpointer gself)
71@@ -272,6 +286,7 @@
72 if (e_cal_client_get_view_finish (E_CAL_CLIENT(client), res, &view, &error))
73 {
74 // add the view to our collection
75+ e_cal_client_view_set_flags(view, E_CAL_CLIENT_VIEW_FLAGS_NONE, NULL);
76 e_cal_client_view_start(view, &error);
77 g_debug("got a view for %s", e_cal_client_get_local_attachment_store(E_CAL_CLIENT(client)));
78 auto self = static_cast<Impl*>(gself);
79@@ -386,14 +401,6 @@
80 }
81 };
82
83- struct UrlSubtask
84- {
85- std::shared_ptr<Task> task;
86- Appointment appointment;
87- UrlSubtask(const std::shared_ptr<Task>& task_in, const Appointment& appointment_in):
88- task(task_in), appointment(appointment_in) {}
89- };
90-
91 static gboolean
92 my_get_appointments_foreach(ECalComponent* component,
93 time_t begin,
94@@ -405,89 +412,68 @@
95
96 if ((vtype == E_CAL_COMPONENT_EVENT) || (vtype == E_CAL_COMPONENT_TODO))
97 {
98- const gchar* uid = nullptr;
99- e_cal_component_get_uid(component, &uid);
100-
101- auto status = ICAL_STATUS_NONE;
102- e_cal_component_get_status(component, &status);
103-
104- if ((uid != nullptr) &&
105- (status != ICAL_STATUS_COMPLETED) &&
106- (status != ICAL_STATUS_CANCELLED))
107- {
108- Appointment appointment;
109-
110- /* Determine whether this is a recurring event.
111- NB: icalrecurrencetype supports complex recurrence patterns;
112- however, since design only allows daily recurrence,
113- that's all we support here. */
114- GSList * recur_list;
115- e_cal_component_get_rrule_list(component, &recur_list);
116- for (auto l=recur_list; l!=nullptr; l=l->next)
117- {
118- const auto recur = static_cast<struct icalrecurrencetype*>(l->data);
119- appointment.is_daily |= ((recur->freq == ICAL_DAILY_RECURRENCE)
120- && (recur->interval == 1));
121- }
122- e_cal_component_free_recur_list(recur_list);
123-
124- ECalComponentText text;
125- text.value = nullptr;
126- e_cal_component_get_summary(component, &text);
127- if (text.value)
128- appointment.summary = text.value;
129-
130- appointment.begin = DateTime(begin);
131- appointment.end = DateTime(end);
132- appointment.color = subtask->color;
133- appointment.is_event = vtype == E_CAL_COMPONENT_EVENT;
134- appointment.uid = uid;
135-
136- GList * alarm_uids = e_cal_component_get_alarm_uids(component);
137- appointment.has_alarms = alarm_uids != nullptr;
138- cal_obj_uid_list_free(alarm_uids);
139-
140- e_cal_client_get_attachment_uris(subtask->client,
141- uid,
142- nullptr,
143- subtask->task->p->m_cancellable,
144- on_appointment_uris_ready,
145- new UrlSubtask(subtask->task, appointment));
146+ const gchar* uid = nullptr;
147+ e_cal_component_get_uid(component, &uid);
148+
149+ auto status = ICAL_STATUS_NONE;
150+ e_cal_component_get_status(component, &status);
151+
152+ if ((uid != nullptr) &&
153+ (status != ICAL_STATUS_COMPLETED) &&
154+ (status != ICAL_STATUS_CANCELLED))
155+ {
156+ Appointment appointment;
157+
158+ ECalComponentText text;
159+ text.value = nullptr;
160+ e_cal_component_get_summary(component, &text);
161+ if (text.value)
162+ appointment.summary = text.value;
163+
164+ appointment.begin = DateTime(begin);
165+ appointment.end = DateTime(end);
166+ appointment.color = subtask->color;
167+ appointment.uid = uid;
168+
169+ // if the component has display alarms that have a url,
170+ // use the first one as our Appointment.url
171+ auto alarm_uids = e_cal_component_get_alarm_uids(component);
172+ appointment.has_alarms = alarm_uids != nullptr;
173+ for(auto walk=alarm_uids; appointment.url.empty() && walk!=nullptr; walk=walk->next)
174+ {
175+ auto alarm = e_cal_component_get_alarm(component, static_cast<const char*>(walk->data));
176+
177+ ECalComponentAlarmAction action;
178+ e_cal_component_alarm_get_action(alarm, &action);
179+ if (action == E_CAL_COMPONENT_ALARM_DISPLAY)
180+ {
181+ icalattach* attach = nullptr;
182+ e_cal_component_alarm_get_attach(alarm, &attach);
183+ if (attach != nullptr)
184+ {
185+ if (icalattach_get_is_url (attach))
186+ {
187+ const char* url = icalattach_get_url(attach);
188+ if (url != nullptr)
189+ appointment.url = url;
190+ }
191+
192+ icalattach_unref(attach);
193+ }
194+ }
195+
196+ e_cal_component_alarm_free(alarm);
197+ }
198+ cal_obj_uid_list_free(alarm_uids);
199+
200+ g_debug("adding appointment '%s' '%s'", appointment.summary.c_str(), appointment.url.c_str());
201+ subtask->task->appointments.push_back(appointment);
202 }
203 }
204-
205+
206 return G_SOURCE_CONTINUE;
207 }
208-
209- static void on_appointment_uris_ready(GObject* client, GAsyncResult* res, gpointer gsubtask)
210- {
211- auto subtask = static_cast<UrlSubtask*>(gsubtask);
212-
213- GSList * uris = nullptr;
214- GError * error = nullptr;
215- e_cal_client_get_attachment_uris_finish(E_CAL_CLIENT(client), res, &uris, &error);
216- if (error != nullptr)
217- {
218- if (!g_error_matches(error, G_IO_ERROR, G_IO_ERROR_CANCELLED) &&
219- !g_error_matches(error, E_CLIENT_ERROR, E_CLIENT_ERROR_NOT_SUPPORTED))
220- {
221- g_warning("Error getting appointment uris: %s", error->message);
222- }
223-
224- g_error_free(error);
225- }
226- else if (uris != nullptr)
227- {
228- subtask->appointment.url = (const char*) uris->data; // copy the first URL
229- g_debug("found url '%s' for appointment '%s'", subtask->appointment.url.c_str(), subtask->appointment.summary.c_str());
230- e_client_util_free_string_slist(uris);
231- }
232-
233- g_debug("adding appointment '%s' '%s'", subtask->appointment.summary.c_str(), subtask->appointment.url.c_str());
234- subtask->task->appointments.push_back(subtask->appointment);
235- delete subtask;
236- }
237-
238+
239 EdsEngine& m_owner;
240 core::Signal<> m_changed;
241 std::set<ESource*> m_sources;
242@@ -496,6 +482,7 @@
243 GCancellable* m_cancellable = nullptr;
244 ESourceRegistry* m_source_registry = nullptr;
245 guint m_rebuild_tag = 0;
246+ time_t m_rebuild_deadline = 0;
247 };
248
249 /***
250
251=== modified file 'tests/manual-test-snap.cpp'
252--- tests/manual-test-snap.cpp 2014-02-04 19:00:22 +0000
253+++ tests/manual-test-snap.cpp 2014-04-15 16:12:38 +0000
254@@ -36,8 +36,6 @@
255 a.summary = "Alarm";
256 a.url = "alarm:///hello-world";
257 a.uid = "D4B57D50247291478ED31DED17FF0A9838DED402";
258- a.is_event = false;
259- a.is_daily = false;
260 a.has_alarms = true;
261 auto begin = g_date_time_new_local(2014,12,25,0,0,0);
262 auto end = g_date_time_add_full(begin,0,0,1,0,0,-1);

Subscribers

People subscribed via source and target branches