Рефакторинг словаря в ConcurrentDictionary - PullRequest
1 голос
/ 18 мая 2011

Я хочу сделать мой код многопоточным, поэтому мне нужно изменить словарь в ConcurrentDictionary. Я прочитал о ConcurrentDictionary, проверил некоторый пример, но все же мне нужно помочь:

Вот оригинальный код (для одного потока)

private IDictionary<string, IDictionary<string, Task>> _tasks;
public override IDictionary<string, IDictionary<string, Task>> Tasks
    {
        get
        {
            // return dictionary from cache unless too old
            // concurrency!! (null check)
            if (_tasks != null && (DateTime.Now - _lastTaskListRefreshDateTime < TimeSpan.FromSeconds(30)))
            {
                return _tasks;
            }

            // reload dictionary from database
            _tasks = new Dictionary<string, IDictionary<string, Task>>();

            // find returns an IEnumerable<Task>
            var tasks = Find<Task>(null, DependencyNode.TaskForCrawler).Cast<Task>();

            // build hierarchical dictionary from flat IEnumerable
            // concurrency!!
            foreach (var t in tasks)
            {

                if (_tasks.ContainsKey(t.Area.Key))
                {
                    if (_tasks[t.Area.Key] == null)
                    {
                        _tasks[t.Area.Key] = new Dictionary<string, Task>();
                    }

                    if (!_tasks[t.Area.Key].ContainsKey(t.Key))
                    {
                        _tasks[t.Area.Key].Add(t.Key, t);
                    }
                }
                else
                {
                    _tasks.Add(t.Area.Key, new Dictionary<string, Task> { { t.Key, t } });
                }
            }

            _lastTaskListRefreshDateTime = DateTime.Now;
            return _tasks;
        }

        set
        {
            _tasks = value;
        }
    }

Вот что я придумал:

private ConcurrentDictionary<string, ConcurrentDictionary<string, Task>> _tasks = new ConcurrentDictionary<string, ConcurrentDictionary<string, Task>>();
public override ConcurrentDictionary<string, ConcurrentDictionary<string, Task>> Tasks
{
        get
        {
            // use cache
            // concurrency?? (null check)
            if (!_tasks.IsEmpty && (DateTime.Now - _lastTaskListRefreshDateTime < TimeSpan.FromSeconds(30)))
            {
                return _tasks;
            }

            // reload
            var tasks = Find<Task>(null, DependencyNode.TaskForCrawler).Cast<Task>();

            foreach (var task in tasks)
            {
                var t = task; // inner scope for clousure
                var taskKey = t.Key;
                var areaKey = t.Area.Key;

                var newDict = new ConcurrentDictionary<string, Task>();
                newDict.TryAdd(taskKey, t);

                _tasks.AddOrUpdate(areaKey, newDict, (k, v) => {
                                                        // An dictionary element if key=areaKey already exists
                                                        // extend and return it.
                                                        v.TryAdd(taskKey, t);
                                                        return v;
                                                       });
            }

            _lastTaskListRefreshDateTime = DateTime.Now;
            return _tasks;
        }

Я не уверен, что это именно так, в частности, я вполне уверен, что проверка IsEmpty не является поточно-безопасной, поскольку _tasks может быть инициализирован между проверкой IsEmpty и частью && ... или return _tasks часть. Нужно ли блокировать эту проверку вручную? Нужен ли двойной замок (проверка нуля> блокировка> проверка нуля)?

Ответы [ 2 ]

1 голос
/ 19 мая 2011

ConcurrentDictionary только гарантирует, что чтение и запись в словарь не будут распространяться друг на друга, чего не делает класс Dictionary.Безопасность потока в ConcurrentDictionary не делает ваш поток кода безопасным, он только гарантирует, что его код является потокобезопасным.Поскольку это так, вам понадобится блокировка в вашем геттере.

1 голос
/ 18 мая 2011

Ваше беспокойство оправдано.Получатель свойства Tasks не является потокобезопасным.Здесь есть несколько вопросов.

Во-первых, как и вы, существует гонка между вызовом IsEmpty из одного потока и удалением элемента из другого потока.Получатель может вернуть пустой словарь.

Во-вторых, между чтением _lastTaskListRefreshDateTime в проверке if и назначением в конце получателя есть гонка.Даже если эти операции являются атомарными (что не может быть, по крайней мере, на 32-битных платформах, поскольку DateTime является 64-битной), все равно остается небольшая проблема с барьером памяти, поскольку в коде отсутствуют механизмы синхронизации, подобные volatile.

В-третьих, аналогично моему объяснению выше, существует другая проблема с барьером памяти со ссылкой _tasks.Один поток может вызвать установщик, в то время как другой вызывает получатель.Поскольку отсутствует барьер памяти, CLR или аппаратные средства могут оптимизировать операции чтения и записи таким образом, чтобы изменения, сделанные в установщике, не были видны получателю.Эта проблема не обязательно может вызвать какие-либо проблемы, но я уверен, что это поведение, которое не ожидалось.Без другого контекста для анализа я не могу сказать ни слова.

...