dee

Merge lp:~kamstrup/dee/less-gtype-dancing into lp:dee

  1. less-gtype-dancing
  2. Merge into trunk
Proposed by Mikkel Kamstrup Erlandsen
Status: Merged
Approved by: Michal Hruby
Approved revision: 353
Merged at revision: 348
Proposed branch: lp:~kamstrup/dee/less-gtype-dancing
Merge into: lp:dee
Diff against target: 302 lines (+83/-34)
7 files modified
src/dee-model.c (+6/-13)
src/dee-model.h (+4/-1)
src/dee-sequence-model.c (+37/-15)
src/dee-serializable-model.c (+28/-3)
vapi/Dee-1.0-custom.vala (+6/-0)
vapi/Dee-1.0.metadata (+1/-1)
vapi/dee-1.0.vapi (+1/-1)
To merge this branch: bzr merge lp:~kamstrup/dee/less-gtype-dancing
Reviewer Review Type Date Requested Status
Michal Hruby (community) Approve
Mikkel Kamstrup Erlandsen Pending
Review via email: mp+93980@code.launchpad.net

This proposal supersedes a proposal from 2012-02-21.

Description of the change

A series of optimizations for DeeSequenceModel. Notably benchmarks report improvements:

append+prepend: ~10%
sorted: ~5%
read_string: ~25%
read_row: ~50%
clear: ~10%
walk_next: ~50%
walk_pos: ~15%
FilterModel.collate,collate_desc,sort_uint: ~5-10%

Probably many small gains here and there.

To post a comment you must log in.
Revision history for this message
Mikkel Kamstrup Erlandsen (kamstrup) wrote : Posted in a previous version of this proposal

I am not entirely sure what the impact of this is...

269 - [CCode (array_length = false, array_null_terminated = true)]
270 - public GLib.Variant[] get_row (Dee.ModelIter iter, [CCode (array_length = false)] out GLib.Variant[] out_row_members = null);
271 + [NoWrapper]
272 + public abstract GLib.Variant get_row (Dee.ModelIter iter, GLib.Variant out_row_members = null);

Revision history for this message
Michal Hruby (mhr3) wrote : Posted in a previous version of this proposal

Faster everything, yey!

This seems to have broken gir a little and vapigen now can't match the virtual get_row to the invoker method, could you move the whole get_row method to -custom.vala (with all the annotations there were + make it abstract).

review: Needs Fixing
Revision history for this message
Mikkel Kamstrup Erlandsen (kamstrup) wrote : Posted in a previous version of this proposal

> Faster everything, yey!
>
> This seems to have broken gir a little and vapigen now can't match the virtual
> get_row to the invoker method, could you move the whole get_row method to
> -custom.vala (with all the annotations there were + make it abstract).

Pushed as r352

Revision history for this message
Michal Hruby (mhr3) wrote :

Awesome! :)

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/dee-model.c'
2--- src/dee-model.c 2012-01-26 18:30:57 +0000
3+++ src/dee-model.c 2012-02-21 12:37:14 +0000
4@@ -1436,26 +1436,19 @@
5 * or a newly allocated array otherwise which you must free
6 * with g_free(). The variants in the array will have a strong
7 * reference and needs to be freed with g_variant_unref().
8- **/
9+ */
10 GVariant**
11 dee_model_get_row (DeeModel *self,
12 DeeModelIter *iter,
13 GVariant **out_row_members)
14 {
15- guint col, n_cols;
16+ DeeModelIface *iface;
17
18 g_return_val_if_fail (DEE_IS_MODEL (self), NULL);
19- g_return_val_if_fail (iter != NULL, NULL);
20-
21- n_cols = dee_model_get_n_columns (self);
22-
23- if (out_row_members == NULL)
24- out_row_members = g_new0 (GVariant*, n_cols + 1);
25-
26- for (col = 0; col < n_cols; col++)
27- out_row_members[col] = dee_model_get_value (self, iter, col);
28-
29- return out_row_members;
30+
31+ iface = DEE_MODEL_GET_IFACE (self);
32+
33+ return (* iface->get_row) (self, iter, out_row_members);
34 }
35
36 /**
37
38=== modified file 'src/dee-model.h'
39--- src/dee-model.h 2012-01-26 18:30:57 +0000
40+++ src/dee-model.h 2012-02-21 12:37:14 +0000
41@@ -215,6 +215,10 @@
42 DeeModelIter *iter,
43 DeeModelTag *tag,
44 gpointer value);
45+
46+ GVariant** (*get_row) (DeeModel *self,
47+ DeeModelIter *iter,
48+ GVariant **out_row_members);
49
50 /*< private >*/
51 void (*_dee_model_1) (void);
52@@ -224,7 +228,6 @@
53 void (*_dee_model_5) (void);
54 void (*_dee_model_6) (void);
55 void (*_dee_model_7) (void);
56- void (*_dee_model_8) (void);
57 };
58
59 GType dee_model_iter_get_type (void);
60
61=== modified file 'src/dee-sequence-model.c'
62--- src/dee-sequence-model.c 2012-01-10 07:43:54 +0000
63+++ src/dee-sequence-model.c 2012-02-21 12:37:14 +0000
64@@ -128,6 +128,10 @@
65 DeeModelIter *iter,
66 guint column);
67
68+static GVariant** dee_sequence_model_get_row (DeeModel *self,
69+ DeeModelIter *iter,
70+ GVariant **out_row_members);
71+
72 static DeeModelIter* dee_sequence_model_get_first_iter (DeeModel *self);
73
74 static DeeModelIter* dee_sequence_model_get_last_iter (DeeModel *self);
75@@ -295,6 +299,7 @@
76 iface->set_row = dee_sequence_model_set_row;
77 iface->set_value = dee_sequence_model_set_value;
78 iface->get_value = dee_sequence_model_get_value;
79+ iface->get_row = dee_sequence_model_get_row;
80 iface->get_first_iter = dee_sequence_model_get_first_iter;
81 iface->get_last_iter = dee_sequence_model_get_last_iter;
82 iface->get_iter_at_row = dee_sequence_model_get_iter_at_row;
83@@ -469,9 +474,7 @@
84
85 g_return_if_fail (DEE_IS_SEQUENCE_MODEL (_self));
86 g_return_if_fail (iter != NULL);
87-
88- if (iter_ == dee_model_get_last_iter (self))
89- return;
90+ g_return_if_fail (!g_sequence_iter_is_end (iter));
91
92 if (iter)
93 {
94@@ -500,6 +503,7 @@
95 g_return_if_fail (DEE_IS_SEQUENCE_MODEL (_self));
96 g_return_if_fail (iter != NULL);
97 g_return_if_fail (value != NULL);
98+ g_return_if_fail (column < dee_model_get_n_columns (self));
99
100 priv = _self->priv;
101
102@@ -546,14 +550,9 @@
103 guint column,
104 GVariant *value)
105 {
106- DeeSequenceModel *_self = (DeeSequenceModel *)self;
107 gpointer *row;
108
109- g_return_if_fail (DEE_IS_SEQUENCE_MODEL (_self));
110- g_return_if_fail (iter != NULL);
111- g_return_if_fail (column < dee_model_get_n_columns (self));
112 g_return_if_fail (g_variant_type_equal (g_variant_get_type (value), G_VARIANT_TYPE (dee_model_get_column_schema (self, column))));
113- g_return_if_fail (value != NULL);
114
115 row = g_sequence_get ((GSequenceIter *) iter);
116
117@@ -576,13 +575,8 @@
118 DeeModelIter *iter,
119 guint column)
120 {
121- DeeSequenceModel *_self = (DeeSequenceModel *)self;
122 gpointer *row;
123
124- g_return_val_if_fail (DEE_IS_SEQUENCE_MODEL (_self), NULL);
125- g_return_val_if_fail (iter != NULL, NULL);
126- g_return_val_if_fail (column < dee_model_get_n_columns (self), NULL);
127-
128 row = g_sequence_get ((GSequenceIter *) iter);
129 if (G_UNLIKELY (row == NULL))
130 {
131@@ -600,6 +594,10 @@
132 DeeModelIter *iter,
133 guint column)
134 {
135+ g_return_val_if_fail (DEE_IS_SEQUENCE_MODEL (self), NULL);
136+ g_return_val_if_fail (iter != NULL, NULL);
137+ g_return_val_if_fail (column < dee_model_get_n_columns (self), NULL);
138+
139 GVariant *val = dee_sequence_model_peek_value (self, iter, column);
140
141 if (G_UNLIKELY (val == NULL))
142@@ -613,6 +611,30 @@
143 return g_variant_ref (val);
144 }
145
146+static GVariant**
147+dee_sequence_model_get_row (DeeModel *self,
148+ DeeModelIter *iter,
149+ GVariant **out_row_members)
150+{
151+ guint col, n_cols;
152+
153+ g_return_val_if_fail (DEE_IS_SEQUENCE_MODEL (self), NULL);
154+
155+ n_cols = dee_model_get_n_columns (self);
156+
157+ if (out_row_members == NULL)
158+ out_row_members = g_new0 (GVariant*, n_cols + 1);
159+
160+ /* We use peek_value() here because it saves us from some expensive checks
161+ * compared to get_value(), that we can guarantee from this call site anyway
162+ */
163+ for (col = 0; col < n_cols; col++)
164+ out_row_members[col] = g_variant_ref (
165+ dee_sequence_model_peek_value (self, iter, col));
166+
167+ return out_row_members;
168+}
169+
170 static DeeModelIter*
171 dee_sequence_model_get_first_iter (DeeModel *self)
172 {
173@@ -794,7 +816,7 @@
174 {
175 g_return_val_if_fail (DEE_IS_SEQUENCE_MODEL (self), NULL);
176 g_return_val_if_fail (iter, NULL);
177- g_return_val_if_fail (!dee_model_is_last (self, iter), NULL);
178+ g_return_val_if_fail (!g_sequence_iter_is_end ((GSequenceIter*) iter), NULL);
179
180 return (DeeModelIter *) g_sequence_iter_next ((GSequenceIter *)iter);
181 }
182@@ -805,7 +827,7 @@
183 {
184 g_return_val_if_fail (DEE_IS_SEQUENCE_MODEL (self), NULL);
185 g_return_val_if_fail (iter, NULL);
186- g_return_val_if_fail (!dee_model_is_first (self, iter), NULL);
187+ g_return_val_if_fail (!g_sequence_iter_is_begin ((GSequenceIter*) iter), NULL);
188
189 return (DeeModelIter *) g_sequence_iter_prev ((GSequenceIter *)iter);
190 }
191
192=== modified file 'src/dee-serializable-model.c'
193--- src/dee-serializable-model.c 2012-01-09 11:17:10 +0000
194+++ src/dee-serializable-model.c 2012-02-21 12:37:14 +0000
195@@ -144,6 +144,10 @@
196 DeeModelIter *iter,
197 guint column);
198
199+static GVariant** dee_serializable_model_get_row (DeeModel *self,
200+ DeeModelIter *iter,
201+ GVariant **out_row_members);
202+
203 static DeeModelIter* dee_serializable_model_get_first_iter (DeeModel *self);
204
205 static DeeModelIter* dee_serializable_model_get_last_iter (DeeModel *self);
206@@ -456,16 +460,16 @@
207 static void
208 dee_serializable_model_clear (DeeModel *self)
209 {
210- DeeModelIter *iter;
211+ DeeModelIter *iter, *end;
212
213 g_return_if_fail (DEE_IS_SERIALIZABLE_MODEL (self));
214
215 iter = dee_model_get_first_iter (self);
216+ end = dee_model_get_last_iter (self);
217
218- while (!dee_model_is_last (self, iter))
219+ while (iter != end)
220 {
221 dee_model_remove (self, iter);
222-
223 iter = dee_model_get_first_iter (self);
224 }
225 }
226@@ -614,6 +618,26 @@
227 return NULL;
228 }
229
230+static GVariant**
231+dee_serializable_model_get_row (DeeModel *self,
232+ DeeModelIter *iter,
233+ GVariant **out_row_members)
234+{
235+ guint col, n_cols;
236+
237+ g_return_val_if_fail (DEE_IS_SERIALIZABLE_MODEL (self), NULL);
238+
239+ n_cols = dee_model_get_n_columns (self);
240+
241+ if (out_row_members == NULL)
242+ out_row_members = g_new0 (GVariant*, n_cols + 1);
243+
244+ for (col = 0; col < n_cols; col++)
245+ out_row_members[col] = dee_model_get_value (self, iter, col);
246+
247+ return out_row_members;
248+}
249+
250 static gboolean
251 dee_serializable_model_get_bool (DeeModel *self,
252 DeeModelIter *iter,
253@@ -1111,6 +1135,7 @@
254 iface->set_value = dee_serializable_model_set_value;
255 iface->set_row = dee_serializable_model_set_row;
256 iface->get_value = dee_serializable_model_get_value;
257+ iface->get_row = dee_serializable_model_get_row;
258 iface->get_first_iter = dee_serializable_model_get_first_iter;
259 iface->get_last_iter = dee_serializable_model_get_last_iter;
260 iface->get_iter_at_row = dee_serializable_model_get_iter_at_row;
261
262=== modified file 'vapi/Dee-1.0-custom.vala'
263--- vapi/Dee-1.0-custom.vala 2012-02-19 17:19:33 +0000
264+++ vapi/Dee-1.0-custom.vala 2012-02-21 12:37:14 +0000
265@@ -1,5 +1,11 @@
266 namespace Dee {
267
268+ [CCode (type_id = "dee_model_get_type ()")]
269+ public interface Model {
270+ [CCode (array_length = false, array_null_terminated = true)]
271+ public abstract GLib.Variant[] get_row (Dee.ModelIter iter, [CCode (array_length = false)] out GLib.Variant[] out_row_members = null);
272+ }
273+
274 public struct Filter {
275 [CCode (cname = "destroy")]
276 public GLib.DestroyNotify destroy_notify;
277
278=== modified file 'vapi/Dee-1.0.metadata'
279--- vapi/Dee-1.0.metadata 2012-01-26 10:51:42 +0000
280+++ vapi/Dee-1.0.metadata 2012-02-21 12:37:14 +0000
281@@ -6,7 +6,7 @@
282 Model
283 .append skip=false
284 .get skip=false
285- .get_row.out_row_members default=null
286+ .get_row skip
287 .insert skip=false
288 .insert_before skip=false
289 .insert_sorted skip=false
290
291=== modified file 'vapi/dee-1.0.vapi'
292--- vapi/dee-1.0.vapi 2012-02-19 17:19:33 +0000
293+++ vapi/dee-1.0.vapi 2012-02-21 12:37:14 +0000
294@@ -217,7 +217,7 @@
295 public abstract uint get_n_rows ();
296 public abstract uint get_position (Dee.ModelIter iter);
297 [CCode (array_length = false, array_null_terminated = true)]
298- public GLib.Variant[] get_row (Dee.ModelIter iter, [CCode (array_length = false)] out GLib.Variant[] out_row_members = null);
299+ public abstract GLib.Variant[] get_row (Dee.ModelIter iter, [CCode (array_length = false)] out GLib.Variant[] out_row_members = null);
300 [CCode (array_length_pos = 0.1, array_length_type = "guint")]
301 public abstract unowned string[] get_schema ();
302 public abstract unowned string get_string (Dee.ModelIter iter, uint column);

Subscribers

People subscribed via source and target branches