[blam] ItemStore: do away with instance and locking

[Carlos Martín Nieto <cmartin src gnome org>]

commit c7f4fb3904be1248de6c77bfa6cc71c03dea692e
Author: Carlos Martín Nieto <cmn dwim me>
Date:   Tue Jun 18 01:21:44 2013 +0200

    ItemStore: do away with instance and locking
    
    Use a ConcurrentDictionary instead of locking when we want
    to perform an operation, and load the data explicitly on
    program start

 src/Application.cs |    1 +
 src/Channel.cs     |   27 +++++++--------
 src/ItemList.cs    |    3 +-
 src/ItemStore.cs   |   93 ++++++++++++++++++----------------------------------
 4 files changed, 46 insertions(+), 78 deletions(-)
---
diff --git a/src/Application.cs b/src/Application.cs
index 0423c38..5c3328d 100644
--- a/src/Application.cs
+++ b/src/Application.cs
@@ -155,6 +155,7 @@ namespace Imendio.Blam {
                 Directory.CreateDirectory(Defines.APP_HOMEDIR);
             }
 
+            ItemStore.Load();
             mCollection = ChannelCollection.LoadFromFile (Defines.APP_HOMEDIR + "/collection.xml");
 
             mCollection.ChannelRefreshStarted  += ChannelRefreshStartedCb;
diff --git a/src/Channel.cs b/src/Channel.cs
index 0b4cb5b..205af12 100644
--- a/src/Channel.cs
+++ b/src/Channel.cs
@@ -48,7 +48,6 @@ namespace Imendio.Blam {
                [XmlAttribute] public string http_username = "";
                [XmlAttribute] public string http_password = "";
 
-        ItemStore store = null;
         ArrayList item_list = null;
         Object obj = new Object();
 
@@ -100,7 +99,7 @@ namespace Imendio.Blam {
                 lock(obj){
                     foreach (string id in item_list) {
                         if(id != null){
-                            item = store.Get(id);
+                            item = ItemStore.Get(id);
                             if(item.Unread){
                                 ++unread;
                             }
@@ -121,7 +120,7 @@ namespace Imendio.Blam {
                         if(id == null){ /* TODO: Figure out why this happens */
                             continue;
                         }
-                        item = store.Get(id);
+                        item = ItemStore.Get(id);
                         if(item != null && item.Unread && !item.Old){
                             ++new_items;
                         }
@@ -167,7 +166,6 @@ namespace Imendio.Blam {
         {
             item_list = new ArrayList();
             mIter = new Gtk.TreeIter();
-            store = ItemStore.GetInstance();
         }
 
         public Channel (string name, string url) : this()
@@ -178,7 +176,7 @@ namespace Imendio.Blam {
 
         public Item GetItem (string id)
         {
-            return store.Get(id);
+            return ItemStore.Get(id);
         }
 
         public void Setup()
@@ -189,7 +187,7 @@ namespace Imendio.Blam {
                     if(id == null){
                         continue;
                     }
-                    Item item = store.Get(id);
+                    Item item = ItemStore.Get(id);
                     if(item == null){
                     } else {
                         nlist.Add(id);
@@ -202,10 +200,9 @@ namespace Imendio.Blam {
 
                public void RemoveItems()
                {
-                       ItemStore store = ItemStore.GetInstance();
                        lock(obj){
                                foreach(string id in item_list){
-                                       store.Remove(id);
+                                       ItemStore.Remove(id);
                                }
                        }
                }
@@ -221,7 +218,7 @@ namespace Imendio.Blam {
                                                System.Console.WriteLine("null id {0} on {1}", id, Url);
                                                continue;
                                        }
-                                       item = store.Get(id);
+                                       item = ItemStore.Get(id);
                                        if (item.Unread) {
                                                item.Unread = false;
                                                updated = true;
@@ -251,17 +248,17 @@ namespace Imendio.Blam {
                        lock (obj ){
                                var common = feed.Items.Where(i => item_list.Contains(i.Id));
                                foreach (var item in common)
-                                       store.Get(item.Id).Update(item);
+                                       ItemStore.Get(item.Id).Update(item);
 
                                // new items
                                foreach (var item in feed.Items.Except(common))
-                                       store.Add(new Item(item));
+                                       ItemStore.Add(new Item(item));
 
                                // items no longer in the feed are evicted
                                var ids = feed.Items.Select(i => i.Id);
                                var evict = item_list.Cast<string>().Except(ids);
                                foreach (var id in evict)
-                                       store.Remove(id);
+                                       ItemStore.Remove(id);
 
                                item_list = new ArrayList(ids.ToArray());
                        }
@@ -289,15 +286,15 @@ namespace Imendio.Blam {
         {
             if(item_list.Contains(item.Id)){
                 /* In this case we only need to update */
-                store.Get(item.Id).Update(item);
+                ItemStore.Get(item.Id).Update(item);
                 return;
             }
 
-            store.Add(item);
+                       ItemStore.Add(item);
             lock(obj){
                 item_list.Add(item.Id);
             }
-            store.Get(item.Id).PropertyChanged += ItemChanged;
+                       ItemStore.Get(item.Id).PropertyChanged += ItemChanged;
         }
 
                public bool GetHasKeyword (string keyword)
diff --git a/src/ItemList.cs b/src/ItemList.cs
index f38f50b..7111d5e 100644
--- a/src/ItemList.cs
+++ b/src/ItemList.cs
@@ -132,14 +132,13 @@ namespace Imendio.Blam {
                                return;
 
                        var list = Model as ListStore;
-                       var store = ItemStore.GetInstance();
 
                        list.Clear();
                        foreach(string id in channel.ItemList){
                                if (id == null)
                                        continue;
 
-                               list.AppendValues(store.Get(id));
+                               list.AppendValues(ItemStore.Get(id));
                        }
                }
 
diff --git a/src/ItemStore.cs b/src/ItemStore.cs
index ce35464..88dd0bc 100644
--- a/src/ItemStore.cs
+++ b/src/ItemStore.cs
@@ -1,21 +1,21 @@
 using System;
 using System.IO;
+using System.Linq;
 using System.Collections;
+using System.Collections.Generic;
+using System.Collections.Concurrent;
 using System.ServiceModel.Syndication;
 using System.Xml;
-using System.Xml.Serialization;
 
 namespace Imendio.Blam
 {
     public class ItemStore
     {
-        private Hashtable items;
-        static ItemStore instance = null;
+               static ConcurrentDictionary<string, Item> Items;
         static string itemfile = Defines.APP_HOMEDIR + "/" + Defines.APP_ITEMSTORE_FILE;
         static string itemfile_tmp = Defines.APP_HOMEDIR + "/" + Defines.APP_ITEMSTORE_FILE + ".tmp";
-        private object db_lock = new object();
 
-        public Item Get(string id)
+        public static Item Get(string id)
         {
             Item item;
 
@@ -24,64 +24,47 @@ namespace Imendio.Blam
                 return null;
             }
 
-            lock(db_lock){
-                item = items[id] as Item;
-            }
+                       if (!Items.TryGetValue(id, out item))
+                               return null;
 
-            return item;
+                       return item;
         }
 
-        public void Add(Item item)
+        public static void Add(Item item)
         {
             if(item.Id == null){
                 Console.Error.WriteLine("Tried to add item with null key");
                 return;
             }
 
-            lock(db_lock){
-                if(items.ContainsKey(item.Id)){
-                    (items[item.Id] as Item).Grab();
-                    return;
-                } else {
-                    item.Grab();
-                    items.Add(item.Id, item);
-                }
-            }
+                       Item inside;
+
+                       // Add the item if it's not there already and increase the refcount
+                       // of whatever is or was inside.
+                       inside = Items.GetOrAdd(item.Id, item);
+                       inside.Grab();
         }
 
-        public void Remove(Item item)
+        public static void Remove(Item item)
         {
             if(item == null){
                 return;
             }
 
-            lock(db_lock){
-                item.Release();
-                if(item.RefCount == 0){
-                    items.Remove(item.Id);
-                }
-            }
-        }
-
-        public void Remove(string id)
-        {
-            lock(db_lock){
-                Remove(items[id] as Item);
-            }
-        }
+                       // FIXME: pretty sure this is a race condition
+                       item.Release();
+                       if (item.RefCount == 0)
+                               Items.TryRemove(item.Id, out item);
+               }
 
-        public static ItemStore GetInstance()
+        public static void Remove(string id)
         {
-            if(instance == null){
-                Load();
-            }
-
-            return instance;
+                       Remove(Items[id]);
         }
 
-        private static void Load()
+               public static void Load()
         {
-            instance = new ItemStore();
+                       Items = new ConcurrentDictionary<string, Item>();
 
             XmlReader reader;
             try{
@@ -92,26 +75,19 @@ namespace Imendio.Blam
             }
 
             SyndicationFeed feed = SyndicationFeed.Load(reader);
-            foreach(SyndicationItem item in feed.Items){
-                instance.items.Add(item.Id, new Item(item));
-            }
+                       var kv = feed.Items.Select(i => new KeyValuePair<string, Item>(i.Id, new Item(i)));
+                       Items = new ConcurrentDictionary<string, Item>(kv);
         }
 
         public static void Save()
         {
-            if(instance == null){
-                return;
-            }
+                       var values = Items.Values;
+                       foreach (var item in values)
+                               item.WriteExtensions();
 
-            Item[] items;
+                       var items = new Item[Items.Count];
+                       values.CopyTo(items, 0);
 
-            lock(instance.db_lock){
-                foreach(Item item in instance.items.Values){
-                    item.WriteExtensions();
-                }
-                items = new Item[instance.items.Count];
-                instance.items.Values.CopyTo(items, 0);
-            }
             XmlWriter writer = XmlWriter.Create(itemfile_tmp);
             SyndicationFeed sf = new SyndicationFeed(items);
             Atom10FeedFormatter fmtr = sf.GetAtom10Formatter();
@@ -120,10 +96,5 @@ namespace Imendio.Blam
             File.Delete(itemfile);
             File.Move(itemfile_tmp, itemfile);
         }
-
-       private ItemStore ()
-       {
-            items = new Hashtable();
-       }
     }
 }