 |
|
 |
Hi, Thank you for great article and the source code. It has solved many problems in my projects.
Just a suggestion, It will be more nicer if you add Serializable attribute to the class.
Thanks a lot!!!
|
|
|
|
 |
|
 |
It is very useful article.
Wonde Tadesse
MCTS
|
|
|
|
 |
|
|
 |
|
 |
Excellent work!
Thanks for sharing.
|
|
|
|
 |
|
|
 |
|
|
 |
|
 |
Hello 4565433,
Thanks a lot for you comments and the link.
I had no idea about what's coming new from Microsoft. I'm really happy to know.
I just took a quick look now because I'm very busy. Based on me quick look, I'm not sure that I can use it in the actual implementation of the ObservableCollection. I think you gave me the info mainly as a very interesting think to read and learn... for that thank you a lot. I'm sure I will invest the requested hours very soon because it seems a very nice thing to know.
About Sasha implementation, I read his sample and downloaded Cinch v2 to have a better idea. To me, it sounds more a synchronized extension to the actual ObservableCollection. It is very nice and probably solve problems in most cases. But I think my solution is a little bit more complete/flexible and offer a little bit more advantages. I needed twice in the application I'm working on now, where Sasha or Paul solution would have not worked with the same performance (I was using Paul solution, that is similar to Sasha solution).
Thanks a lot,
Eric
|
|
|
|
 |
|
 |
This new async pattern (with Task) has thread affinity with the calling thread.
When you get time check out the videos, they explain it very well.
|
|
|
|
 |
|
 |
I checked a video but probably not the right one. (It was a kind of sells pitch from an interviewer and Mads Torgersen )
I will check now !
Thanks a lot !!!
Eric
|
|
|
|
 |
|
 |
Wow and mmmm
I feel strange... like a mixed bag of feelings...
I would like to say many things...
1 - I have to say something that probably does not help to see it very positively. I'm totally frustrated and very extremely shock that Microsoft does not enables me to update the UI from more than more thread. I had that limitation in COM with appartment and it still exists 15 years after. C# seems to evolve quickly but majors concerns are still alive !!! (my opinion).
2 - I wonder if some C# features become alive due to the limitation of single UI thread updating. I'm not sure but I think that perhaps few things that become alive now and in the near future will potentially die while we will be able (if it happen) to update the UI from any threads. In fact every things that depends on the ThreadContext. I really question myself on the priority that Microsoft have ? Fix UI thread update of find any twisted way to workaround it ?
3 - C# Async ... It seems nice but not so simple at first sight. The video is very good but personnaly i'm missing information about ThreadContext and who will run the callback code. It looks like that the UI will run it if it was the UI that initiate it... But how it could know it has to run on the UI thread. What happen if it is a worker thread who start it? I'm a bit mixed up. In fact... I'm totally mixed up and I don't understand at all how it could work (how the magic could happen). But I should say that my knowledge is limited and I have to understand 99% before I say that I understand (it is not the fact right now).
4 Due to point 3 (my not understanding), I can't realize where to use that in my MT ObservableCollection. I can't see some nice applications mainly while waiting on web. I can think that I could use it to call my lenghty job but still .... OUPS THE LIGHT LIGHT UP NOW ! I can create a task for each item that would feed my ObservableCollection (one task per Item to add in my collection). But still, I can't see how to use it for my MT OC. But instead, I can modify my code that call the OC to take advantage of the "C# Async" new features and use a regular OC. But doing so will put greater (not that much but) complexity in my code that otherwise I wouldn't have by using a MT OC that hide that complexity for me.
Hey... my answer is not simple, kind of like the new "C# Async" feature . I'm agree they did a very good job to make it simple to use but I also think that we have to invest few (many) hours to undertsand exactly what happen behinds to use it properly.
Thanks again for bringing me that to my attention. I have no time to test that now (that is only beta and I'm very busy) but I will definitively use it in the future while any need will requested it... and I'm very happy to know it (will) exists.
Thanks a lot
Eric
modified on Tuesday, May 3, 2011 11:24 AM
|
|
|
|
 |
|
 |
Doesn't the lock statement create a critical section that would be acknowledged by all threads within the application. The instance members of the class are not thread safe, so you have to encapsulate them in lock statements. What is the issue that you had with the ObservableCollection across threads?
http://msdn.microsoft.com/en-us/library/aa664735%28v=vs.71%29.aspx[^]
|
|
|
|
 |
|
 |
Hello Jeffrey,
Thanks for your feedback.
About locks. I made some mistakes. I corrected here and will forward corrections as soon as I'm sure about my modifications.
I followed: http://blogs.msdn.com/b/cburrows/archive/2010/03/30/events-get-a-little-overhaul-in-c-4-afterward-effective-events.aspx
I added a local copy of event before working with them.
Changes:
// ******************************************************************
protected void NotifyPropertyChanged(String propertyName)
{
PropertyChangedEventHandler propertyChanged = PropertyChanged;
if (propertyChanged != null)
{
propertyChanged(this, new PropertyChangedEventArgs(propertyName));
}
}
// ******************************************************************
protected void NotifyPropertyChanged(Expression> propAccess)
{
PropertyChangedEventHandler propertyChanged = PropertyChanged;
if (propertyChanged != null)
{
var asMember = propAccess.Body as MemberExpression;
if (asMember == null)
return;
propertyChanged(this, new PropertyChangedEventArgs(asMember.Member.Name));
}
}
// ******************************************************************
protected void NotifyPropertyChanged(Object obj, PropertyChangedEventArgs e)
{
PropertyChangedEventHandler propertyChanged = PropertyChanged;
if (propertyChanged != null)
{
propertyChanged(obj, e);
}
}
// ******************************************************************
protected void NotifyCollectionChanged(object sender, NotifyCollectionChangedEventArgs e)
{
NotifyCollectionChangedEventHandler collectionChanged = CollectionChanged;
if (collectionChanged != null)
{
collectionChanged(sender, e);
}
}
Here
Not for DispatchPriorityCall because of the nature of it (should not change or only on initialization).
Also, I can't see the reason to protect it.
If you see other places or have other way to do it that are better, please let me know.
About what problem I met with ObservableCollection. ObservableCollection are mainly used in WPF to notify binded UI components of any changes.
Those notification are changes that UI components uses to update their appearance. Changes to UI component could only occur in UI thread.
To improve performance of lenghty job. You should do those job on a worker thread. But sometimes UI updates are very lenghty too.
In order to decouple the worker thread from the UI thread I send notification on the UI thread through BeginInvoke (async) instead Invoke (sync).
Is that appears to you correct ?
|
|
|
|
 |
|
 |
using System;
using System.Collections.ObjectModel;
using System.Linq;
using System.Windows.Threading;
using System.Collections.Specialized;
using System.Collections.Generic;
namespace Cinch
{
public class DispatcherNotifiedObservableCollection<T> : ObservableCollection<T>
{
#region Ctors
public DispatcherNotifiedObservableCollection()
: base()
{
}
public DispatcherNotifiedObservableCollection(List<T> list)
: base(list)
{
}
public DispatcherNotifiedObservableCollection(IEnumerable<T> collection)
: base(collection)
{
}
#endregion
#region Overrides
public override event NotifyCollectionChangedEventHandler CollectionChanged;
protected override void OnCollectionChanged(NotifyCollectionChangedEventArgs e)
{
var eh = CollectionChanged;
if (eh != null)
{
Dispatcher dispatcher =
(from NotifyCollectionChangedEventHandler nh in eh.GetInvocationList()
let dpo = nh.Target as DispatcherObject
where dpo != null
select dpo.Dispatcher).FirstOrDefault();
if (dispatcher != null && dispatcher.CheckAccess() == false)
{
dispatcher.Invoke(DispatcherPriority.DataBind,
(Action)(() => OnCollectionChanged(e)));
}
else
{
foreach (NotifyCollectionChangedEventHandler nh
in eh.GetInvocationList())
nh.Invoke(this, e);
}
}
}
#endregion
}
}
Just in case you are interested
Sacha Barber
- Microsoft Visual C# MVP 2008-2011
- Codeproject MVP 2008-2011
Open Source ProjectsCinch SL/WPF MVVM
Your best friend is you.
I'm my best friend too. We share the same views, and hardly ever argue
My Blog : sachabarber.net
|
|
|
|
 |
|
 |
Thanks Sasha. It is interesting.
To me it sounds like another way of writing the Paul Ortiz solution (minus its little bug) or vice versa.
Thanks,
Eric
|
|
|
|
 |
|
 |
To be honest I did not look at what he does, was just posting for interests sake.
Sacha Barber
- Microsoft Visual C# MVP 2008-2011
- Codeproject MVP 2008-2011
Open Source ProjectsCinch SL/WPF MVVM
Your best friend is you.
I'm my best friend too. We share the same views, and hardly ever argue
My Blog : sachabarber.net
|
|
|
|
 |
|
 |
Thanks Sasha,
Just to let you know, I already used few of your nice code samples (I remember somthing about enum) + others.
... And read something about you once in a while (I think about everywhere).
Your work on Cinch seems very nice but do you know what will be the standardized MVVM framework ?
To be honnest, I did not ckeck any MVVM framework. I prefer to wait and learn the standardized one.
I used some Model-View-View-Model classes to help my view talk to my model but without specific framework.
Does the investment of taking a look at MVVM now would pay me or I can wait for Microsoft ? Which one is the best ?
Éric
|
|
|
|
 |
|
 |
I obviously think mine is the best, I think Cinch and PRISM 4 together are a very good combo, PRISM is not a MVVM framework but does lots of other good stuff like regions, and since its now MEF based works very well with CinchV2.
There is one from microsoft, but its quite minimal, my favourites are
Cinch v2
Caliburn
NRoute
All available at codeplex
Sacha Barber
- Microsoft Visual C# MVP 2008-2011
- Codeproject MVP 2008-2011
Open Source ProjectsCinch SL/WPF MVVM
Your best friend is you.
I'm my best friend too. We share the same views, and hardly ever argue
My Blog : sachabarber.net
|
|
|
|
 |
|
 |
FYI, Sacha - although that can work OK much of the time, it's actually HIGHLY unsafe. It all depends upon how you use the collection and timing, so it's quite scary (you could get different results on different machines, or with different CPU loads). I was basically doing it exactly this same way, and - although at first it seemed to work pretty well - I ran into nasty issues where the UI would sometimes glitch a bit - showing a blank line or repeated line in the data (bound to a ListView in my case, and when I say repeated I don't mean adjacent, so not so obvious). The problem with this approach is really quite easy to understand - while the UI is processing the change notification, the collection can be modified by a background thread at the same time, producing who-knows-what results in the UI control - it all depends upon MS's code, which of course doesn't do any locking on the collection and can't be modified to do so. Although in my particular situation, I'm often pumping a lot of data into the collection, I saw errors when doing as few operations as just 2 Adds on the background thread - but, with just enough time between them to allow the UI thread to get in and be in the middle of updating when the 2nd Add was done, with nasty results!
To be safe, ALL changes to the collection need to occur on the UI thread (in which case, ironically, the change notification can be left alone, as it will also then occur on the UI thread). For my situation, I ended up modeling a solution on this one: http://www.planetgeek.ch/2011/05/17/threadsafe-observablecollection/comment-page-1/#comment-1693 [^]
Basically, override all modification methods and do Invokes to the UI thread if necessary, but with the major caveat of NOT using BeginInvoke (synchronous Invoke instead), since that causes serious problems due to the resulting queuing meaning that indexes into the collection get out-of-sync (as I posted on that page).
It's kinda shocking just how many attempts have been made at addressing this particular issue around the internet, and how many of them actually have various problems - they mostly tend to work for the specific situation the author was dealing with, but are unsafe and can easily fall apart with slightly different usage. Nasty!
|
|
|
|
 |
|
 |
Yeah I can see what you are saying. I think I will leave the article up though, as people will see comments like yours and realise the horrors of attempting to create a thread safe collection.
Thanks
Sacha Barber
- Microsoft Visual C# MVP 2008-2011
- Codeproject MVP 2008-2011
Open Source ProjectsCinch SL/WPF MVVM
Your best friend is you.
I'm my best friend too. We share the same views, and hardly ever argue
My Blog : sachabarber.net
|
|
|
|
 |
|
 |
Hello Ken,
I red you comment and wonder if you are talking about my code or the one given by Sasha.
Did you had any problem with my code ?
You mentionned that every modification need to happen to the UI Thread and it is true. But, if I haven't made any mistake, the code provided here should do so. I wonder how it is possible to get duplicate or missing items. But the main usage I target for this collection was:
- Writes on the worker thread
- Read on the UI (update UI from collection events).
About "out of sync", it is normal that you could be out of sync between the UI thread and any other one. All other thread see the real collection state while the UI thread see real collection state minus any pending modification in the UI queue (which is normal and expected). That's why we decouple view of the collection... in order to not wait for the UI Thread.
To write the best code, you would have to implement the same mechanism as a database with all the problems that come with it (highly complex - need transaction - update could fail if not transactionned - ...). Also, I'm not sure but I think that the best way would be to use Event Kernel Object (Operating system event mechanism) to refresh UI but it would imply to re-think a good part of WPF (events with information on modification is by default a threading problem). In fact Micrsoft did real crap when they create ObservableCollection. But now the interface is deployed and we have to live with it. ObservableCollection2 should only advise of modifications happen and use a queue of modifications. It would be a lot nicer but you would have the same effect of out of sync, which is normal.ObservableCollection2 is virtual in my head and would request deep changes on WPF to implement. But it would be better to do that the sooner as possible.
Let me know if you got problem with my implementation please !
Eric
modified 31 Oct '11.
|
|
|
|
 |
|
 |
I specifically addressed Sasha regarding his posted code, not yours. I just wanted to help by letting people know the dangers of that method, since I had problems with it myself.
I looked at your sources, and you indeed seem to be making all changes on the UI thread, so that's good. However, you are using BeginInvokes, which is dangerous because of the queuing. So, it's probably OK as you say if the worker thread only does writes, but could easily fail if it ever tries to read back or remove any recently added items. That being the case, I guess I don't really understand all of the locks that you have in there... they don't seem necessary to me. The code on the page I posted the link to works just fine (with my posted changes) and is very simple compared to yours. In either case, BeginInvoke can be used for better performance if the background thread only writes, or Invoke otherwise. It appears you were trying to make the collection safe for changes by multiple threads - perhaps before marshalling all changes to the UI thread? But, since all changes are made on the UI thread, there is no reason to lock, and if the worker thread makes changes, the locks aren't really going to help much (there will be plenty of problems).
The real mess with threading and collections is that there is NO SUCH THING as a "thread safe collection". Few programmers seem to understand that properly. It's all about how the code using the collection is written, which is why some people argue against adding thread safety to collections at all - just putting locks in the code that uses them. For example, any code can request the count of a collection, then iterate through the items using that count, or otherwise be written such that it is vulnerable to changes, and no amount of locking inside the collection class will ever prevent problems when multiple threads are involved. This is the case with WPF binding to ObservableCollection{T} without doing any locking while updating the UI - which is why the only real "fix" is to make sure that ALL modifications to the collection occur on the UI thread. It's very far from the ideal solution for using multiple threads with a collection, but there is no alternative in this specific case.
|
|
|
|
 |
|
 |
Yes all locks are there for usage by multiple threads, either only workers thread or for example: one UI (read) and one worker (write). Yes we need to use lock when used when there is only one writer (I will not explain here, but It is far from being the first time I have to say that or explain that, but be sure it is a most and there is no way to do it differently).
About deletion, it should not cause any problem because every action is done in the UI thread then coherence for the UI thread should be kept fine.
But there is one thing you should take great care:
if you want to read the collection in any way from any worker thread, then you will have to use an extern (your own) lock for any usage of indexes or iteration.
Hope it helps.
P.S. Having a an "Observable" interface thinked with Multithreaded in mind would have greatly simplify MT implementation. I'm expecting Microsoft to come out with a better solution one day, but who knows when...
|
|
|
|
 |