← Back to team overview

zeitgeist team mailing list archive

Re: [Merge] lp:~tdfischer/zeitgeist/event-caching into lp:zeitgeist

 

Review: Needs Fixing

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).
-- 
https://code.launchpad.net/~tdfischer/zeitgeist/event-caching/+merge/97450
Your team Zeitgeist Framework Team is subscribed to branch lp:zeitgeist.


References