Merge lp:~tdfischer/zeitgeist/event-caching into lp:~zeitgeist/zeitgeist/bluebird

Proposed by Trever Fischer
Status: Needs review
Proposed branch: lp:~tdfischer/zeitgeist/event-caching
Merge into: lp:~zeitgeist/zeitgeist/bluebird
Diff against target: 313 lines (+198/-5)
7 files modified
.bzrignore (+1/-0)
extensions/fts++/Makefile.am (+1/-0)
src/Makefile.am (+1/-0)
src/db-reader.vala (+27/-5)
src/event-cache.vala (+77/-0)
test/direct/Makefile.am (+7/-0)
test/direct/event-cache-test.vala (+84/-0)
To merge this branch: bzr merge lp:~tdfischer/zeitgeist/event-caching
Reviewer Review Type Date Requested Status
Siegfried Gevatter Needs Fixing
Review via email: mp+97450@code.launchpad.net

Description of the change

Implements a LRU cache for events, to reduce hitting the database on every request.

To post a comment you must log in.
422. By Trever Fischer

Add files I missed

423. By Trever Fischer

Ignore the new test

424. By Trever Fischer

Remove the test binary that was accidentally committed

425. By Trever Fischer

Fix handling of null events, and use a pre-sized GenericArray to reduce CRITICAL messages

Revision history for this message
Siegfried Gevatter (rainct) wrote :

Hi Trever,

Nice work, thanks for working on this!

Some comments:

> max_cache_size = 1024;
We have a (so far unused) constant for this in src/utils.vala: Utils.CACHE_SIZE.

+ uncached_ids.resize(uncached_ids.length+1);
+ uncached_ids[uncached_ids.length-1] = event_ids[i];
You can use "+=" here (which looks nicer and does power-of-two allocations).

+ cache = new EventCache();
                              ^ space
+ for(int i = 0; i < event_ids.length; i++)
     ^ space
And more spaces missing in function declarations in event-cache.vala.

Also, see the comment:

// TODO: Consider if we still want the cache. [...]
// It'd also benchmark it again first, we may have better options
// to enhance the performance of SQLite now, and event processing
// will be faster now being C.

Some numbers about how this affects performance for different queries (cold query vs hot query vs mixed query with and without cache).

review: Needs Fixing
Revision history for this message
Trever Fischer (tdfischer) wrote :

Updated. I'll start working on providing some analysis.

426. By Trever Fischer

style++, and use automatic array power of two allocation

Unmerged revisions

426. By Trever Fischer

style++, and use automatic array power of two allocation

425. By Trever Fischer

Fix handling of null events, and use a pre-sized GenericArray to reduce CRITICAL messages

424. By Trever Fischer

Remove the test binary that was accidentally committed

423. By Trever Fischer

Ignore the new test

422. By Trever Fischer

Add files I missed

421. By Trever Fischer

Implement LRU event cache

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file '.bzrignore'
--- .bzrignore 2012-02-14 16:57:23 +0000
+++ .bzrignore 2012-03-30 14:51:19 +0000
@@ -68,3 +68,4 @@
68mimetype-test68mimetype-test
69marshalling-test69marshalling-test
70test/direct/datamodel-test70test/direct/datamodel-test
71test/direct/event-cache-test
7172
=== modified file 'extensions/fts++/Makefile.am'
--- extensions/fts++/Makefile.am 2012-02-12 20:17:52 +0000
+++ extensions/fts++/Makefile.am 2012-03-30 14:51:19 +0000
@@ -42,6 +42,7 @@
42 ontology-uris.vala \42 ontology-uris.vala \
43 mimetype.vala \43 mimetype.vala \
44 ext-dummies.vala \44 ext-dummies.vala \
45 event-cache.vala \
45 $(NULL)46 $(NULL)
4647
47libzeitgeist_internal_la_SOURCES = \48libzeitgeist_internal_la_SOURCES = \
4849
=== added symlink 'extensions/fts++/event-cache.vala'
=== target is u'../../src/event-cache.vala'
=== modified file 'src/Makefile.am'
--- src/Makefile.am 2012-02-15 19:55:59 +0000
+++ src/Makefile.am 2012-03-30 14:51:19 +0000
@@ -48,6 +48,7 @@
48 ontology.vala \48 ontology.vala \
49 ontology-uris.vala \49 ontology-uris.vala \
50 mimetype.vala \50 mimetype.vala \
51 event-cache.vala \
51 $(NULL)52 $(NULL)
5253
53zeitgeist_daemon_SOURCES = \54zeitgeist_daemon_SOURCES = \
5455
=== modified file 'src/db-reader.vala'
--- src/db-reader.vala 2012-03-19 19:56:38 +0000
+++ src/db-reader.vala 2012-03-30 14:51:19 +0000
@@ -45,6 +45,8 @@
45 protected TableLookup mimetypes_table;45 protected TableLookup mimetypes_table;
46 protected TableLookup actors_table;46 protected TableLookup actors_table;
4747
48 protected EventCache cache;
49
48 public DbReader () throws EngineError50 public DbReader () throws EngineError
49 {51 {
50 Object (database: new Zeitgeist.SQLite.Database.read_only ());52 Object (database: new Zeitgeist.SQLite.Database.read_only ());
@@ -59,6 +61,8 @@
59 manifestations_table = new TableLookup (database, "manifestation");61 manifestations_table = new TableLookup (database, "manifestation");
60 mimetypes_table = new TableLookup (database, "mimetype");62 mimetypes_table = new TableLookup (database, "mimetype");
61 actors_table = new TableLookup (database, "actor");63 actors_table = new TableLookup (database, "actor");
64
65 cache = new EventCache ();
62 }66 }
6367
64 protected Event get_event_from_row (Sqlite.Statement stmt, uint32 event_id)68 protected Event get_event_from_row (Sqlite.Statement stmt, uint32 event_id)
@@ -114,10 +118,26 @@
114 // to enhance the performance of SQLite now, and event processing118 // to enhance the performance of SQLite now, and event processing
115 // will be faster now being C.119 // will be faster now being C.
116120
121 var results = new GenericArray<Event?> ();
122 results.length = event_ids.length;
117 if (event_ids.length == 0)123 if (event_ids.length == 0)
118 return new GenericArray<Event?> ();124 return results;
119125
120 var sql_event_ids = database.get_sql_string_from_event_ids (event_ids);126 uint32[] uncached_ids = new uint32[0];
127 for (int i = 0; i < event_ids.length; i++)
128 {
129 Event? e = cache.get_event (event_ids[i]);
130 if (e != null) {
131 results.set(i, e);
132 } else {
133 uncached_ids += event_ids[i];
134 }
135 }
136
137 if (uncached_ids.length == 0)
138 return results;
139
140 var sql_event_ids = database.get_sql_string_from_event_ids (uncached_ids);
121 string sql = """141 string sql = """
122 SELECT * FROM event_view142 SELECT * FROM event_view
123 WHERE id IN (%s)143 WHERE id IN (%s)
@@ -149,12 +169,14 @@
149 }169 }
150170
151 // Sort events according to the sequence of event_ids171 // Sort events according to the sequence of event_ids
152 var results = new GenericArray<Event?> ();
153 results.length = event_ids.length;172 results.length = event_ids.length;
154 int i = 0;173 int i = 0;
155 foreach (var id in event_ids)174 foreach (var id in event_ids)
156 {175 {
157 results.set(i++, events.lookup (id));176 Event e = events.lookup (id);
177 if (e != null)
178 cache.cache_event (e);
179 results.set(i++, e);
158 }180 }
159181
160 return results;182 return results;
161183
=== added file 'src/event-cache.vala'
--- src/event-cache.vala 1970-01-01 00:00:00 +0000
+++ src/event-cache.vala 2012-03-30 14:51:19 +0000
@@ -0,0 +1,77 @@
1/* event-cache.vala
2 *
3 * Copyright © 2012 Collabora Ltd.
4 * By Trever Fischer <trever.fischer@collabora.co.uk>
5 *
6 * This program is free software: you can redistribute it and/or modify
7 * it under the terms of the GNU Lesser General Public License as published by
8 * the Free Software Foundation, either version 2.1 of the License, or
9 * (at your option) any later version.
10 *
11 * This program is distributed in the hope that it will be useful,
12 * but WITHOUT ANY WARRANTY; without even the implied warranty of
13 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
14 * GNU General Public License for more details.
15 *
16 * You should have received a copy of the GNU Lesser General Public License
17 * along with this program. If not, see <http://www.gnu.org/licenses/>.
18 *
19 */
20
21using Zeitgeist;
22
23namespace Zeitgeist
24{
25public class EventCache : Object
26{
27
28 private HashTable<uint32?, Event> cache_table;
29 private Queue<uint32> lru_queue;
30 private uint max_cache_size;
31
32 construct
33 {
34 cache_table = new HashTable<uint32?, Event> (int_hash, int_equal);
35 lru_queue = new Queue<uint32>();
36 max_cache_size = Utils.CACHE_SIZE;
37 }
38
39 public Event? get_event(uint32 id)
40 {
41 Event? e = cache_table.get (id);
42 if (e != null)
43 {
44 lru_queue.remove_all (id);
45 lru_queue.push_tail (id);
46 }
47 return e;
48 }
49
50 public void cache_event(Event e)
51 {
52 cache_table.set (e.id, e);
53 lru_queue.remove_all (e.id);
54 lru_queue.push_tail (e.id);
55
56 while (lru_queue.length > max_cache_size)
57 {
58 uint32 target = lru_queue.pop_head ();
59 cache_table.remove (target);
60 }
61 }
62
63 public uint size
64 {
65 get { return lru_queue.length; }
66 }
67
68 public uint max_size
69 {
70 get { return max_cache_size; }
71 set { max_cache_size = value; }
72 }
73}
74
75}
76
77// vim:expandtab:ts=4:sw=4
078
=== modified file 'test/direct/Makefile.am'
--- test/direct/Makefile.am 2012-02-15 18:55:42 +0000
+++ test/direct/Makefile.am 2012-03-30 14:51:19 +0000
@@ -11,6 +11,7 @@
1111
12TESTS = \12TESTS = \
13 datamodel-test \13 datamodel-test \
14 event-cache-test \
14 marshalling-test \15 marshalling-test \
15 mimetype-test \16 mimetype-test \
16 query-operators-test \17 query-operators-test \
@@ -36,8 +37,12 @@
36 $(top_srcdir)/src/ontology.vala \37 $(top_srcdir)/src/ontology.vala \
37 $(top_srcdir)/src/ontology-uris.vala \38 $(top_srcdir)/src/ontology-uris.vala \
38 $(top_srcdir)/src/mimetype.vala \39 $(top_srcdir)/src/mimetype.vala \
40 $(top_srcdir)/src/event-cache.vala \
39 $(NULL)41 $(NULL)
4042
43event-cache-test: event-cache-test.vala $(SRC_FILES)
44 $(VALA_V)$(VALAC) $(VALAFLAGS) -o $@ $^
45
41datamodel-test: datamodel-test.vala $(SRC_FILES)46datamodel-test: datamodel-test.vala $(SRC_FILES)
42 $(VALA_V)$(VALAC) $(VALAFLAGS) -o $@ $^47 $(VALA_V)$(VALAC) $(VALAFLAGS) -o $@ $^
4348
@@ -66,11 +71,13 @@
66 query-operators-test \71 query-operators-test \
67 table-lookup-test \72 table-lookup-test \
68 where-clause-test \73 where-clause-test \
74 event-cache-test \
69 $(NULL)75 $(NULL)
7076
71EXTRA_DIST = \77EXTRA_DIST = \
72 assertions.vapi \78 assertions.vapi \
73 datamodel-test.vala \79 datamodel-test.vala \
80 event-cache-test.vala \
74 marshalling-test.vala \81 marshalling-test.vala \
75 mimetype-test.vala \82 mimetype-test.vala \
76 query-operators-test.vala \83 query-operators-test.vala \
7784
=== added file 'test/direct/event-cache-test.vala'
--- test/direct/event-cache-test.vala 1970-01-01 00:00:00 +0000
+++ test/direct/event-cache-test.vala 2012-03-30 14:51:19 +0000
@@ -0,0 +1,84 @@
1/* event-cache-test.vala
2 *
3 * Copyright © 2012 Collabora Ltd.
4 * By Trever Fischer <trever.fischer@collabora.co.uk>
5 *
6 * This program is free software: you can redistribute it and/or modify
7 * it under the terms of the GNU Lesser General Public License as published by
8 * the Free Software Foundation, either version 2.1 of the License, or
9 * (at your option) any later version.
10 *
11 * This program is distributed in the hope that it will be useful,
12 * but WITHOUT ANY WARRANTY; without even the implied warranty of
13 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
14 * GNU General Public License for more details.
15 *
16 * You should have received a copy of the GNU Lesser General Public License
17 * along with this program. If not, see <http://www.gnu.org/licenses/>.
18 *
19 */
20
21using Zeitgeist;
22using Assertions;
23
24int main (string[] args)
25{
26 Test.init (ref args);
27 Log.set_always_fatal (LogLevelFlags.LEVEL_CRITICAL);
28
29 Test.add_func("/EventCache/basic", basic_test);
30 Test.add_func("/EventCache/size", size_test);
31 Test.add_func("/EventCache/expiration", expiration_test);
32
33 return Test.run ();
34}
35
36public void basic_test ()
37{
38 EventCache c = new EventCache ();
39 for (int i = 0; i<1024; i++)
40 {
41 Event e = new Event();
42 e.id = i;
43 c.cache_event (e);
44 }
45 assert (c.size == 1024);
46}
47
48public void size_test ()
49{
50 EventCache c = new EventCache ();
51 c.max_size = 5;
52 for (int i = 0; i<2048; i++)
53 {
54 Event e = new Event();
55 e.id = i;
56 c.cache_event (e);
57 }
58
59 assert (c.size == 5);
60}
61
62public void expiration_test ()
63{
64 EventCache c = new EventCache ();
65 c.max_size = 5;
66 for (int i = 0; i < 5; i++)
67 {
68 Event e = new Event();
69 e.id = i;
70 c.cache_event (e);
71 }
72
73 // Refresh the LRU stamp for #0, so #30 expires #1
74 c.get_event(0);
75 Event e = new Event();
76 e.id = 30;
77 c.cache_event (e);
78
79 assert(c.get_event(0) != null);
80 assert(c.get_event(1) == null);
81 assert(c.get_event(30) != null);
82}
83
84// vim:expandtab:ts=4:sw=4

Subscribers

People subscribed via source and target branches