Hi,

we have some performance issues if an entity is modified. We discovered that the method 'NotifyCollectionChanged' in "Xtensive.Storage.EntitySetBase.cs" calls our code using the action 'Reset' (hard coded) even if the argument 'action' was 'Added'. This leads to a lot of full refreshes if we are initializing an entity.

Greeting Andres Rohr Diartis AG


Updated at 13.07.2010 15:12:46

Hi Alex,

I don't understand why this isn't easy. Possibly I didn't describe the problem exactly. I meant the method NotifyCollectionChanged with 2 arguments:

protected void NotifyCollectionChanged(NotifyCollectionChangedAction action, Entity item)
    {
      if (!Session.EntityEventBroker.HasSubscribers)
        return;
      var subscriptionInfo = GetSubscription(EntityEventBroker.CollectionChangedEventKey);
      if (subscriptionInfo.Second != null)
        ((NotifyCollectionChangedEventHandler) subscriptionInfo.Second)
          .Invoke(this, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset));
      NotifyPropertyChanged("Count");
    }

Wouldn't be the solution just to change it to something like that?

protected void NotifyCollectionChanged(NotifyCollectionChangedAction action, Entity item)
    {
      if (!Session.EntityEventBroker.HasSubscribers)
        return;
      var subscriptionInfo = GetSubscription(EntityEventBroker.CollectionChangedEventKey);
      if (subscriptionInfo.Second != null)
        ((NotifyCollectionChangedEventHandler) subscriptionInfo.Second)
          .Invoke(this, new NotifyCollectionChangedEventArgs([b]action, item[/b]));
      NotifyPropertyChanged("Count");
    }

Greetings


Updated at 14.07.2010 15:24:46

In the code of our system we are currently accepting CollectionChanged events that have -1 as an item index. This is a natural way for a Collection to propagate changes to the listener because a Collection isn't an ordered container and therefore has no indexes. For us the proposed solution above would be ok. But I don't know how other listeners would react on the 'Add' or 'Remove' action with an index of -1. Possibly a switch to turn this on or off would be a solution.


Updated at 15.07.2010 12:37:38

Yes, my test method shows that this call is a macro for wrapping the item into a ReadOnlyList which is put into 'args.newItems'. All properties with indices will be set to -1. This is the debug output:

  • args {System.Collections.Specialized.NotifyCollectionChangedEventArgs} System.Collections.Specialized.NotifyCollectionChangedEventArgs
  • base {System.Collections.Specialized.NotifyCollectionChangedEventArgs} System.EventArgs {System.Collections.Specialized.NotifyCollectionChangedEventArgs} _action Add System.Collections.Specialized.NotifyCollectionChangedAction

  • _newItems {System.Collections.ArrayList.ReadOnlyList} System.Collections.IList {System.Collections.ArrayList.ReadOnlyList}

  • [System.Collections.ArrayList.ReadOnlyList] {System.Collections.ArrayList.ReadOnlyList} System.Collections.ArrayList.ReadOnlyList

  • _list {object[1]} System.Collections.IList {object[]} Count 1 int IsFixedSize true bool IsReadOnly true bool IsSynchronized false bool
  • SyncRoot {object[1]} object {object[]} IsFixedSize true bool IsReadOnly true bool _newStartingIndex -1 int _oldItems null System.Collections.IList _oldStartingIndex -1 int Action Add System.Collections.Specialized.NotifyCollectionChangedAction
  • NewItems {System.Collections.ArrayList.ReadOnlyList} System.Collections.IList {System.Collections.ArrayList.ReadOnlyList} NewStartingIndex -1 int OldItems null System.Collections.IList OldStartingIndex -1 int

This thread was imported from our support forum. The original discussion may contain more detailed answer. Original topic by AndresRohr.

asked Jul 12 '10 at 14:10

Editor's gravatar image

Editor
46154156157


One Answer:

That's true - and AFAIK, this isn't easy to fix. Possible workarounds:

  • Try to pause or temporarily cancel WPF databinding. I'm not sure if this easy.

  • Another workaround for this can be implemented @ our own side: we can quickly add support for using (session.SuspendNotifications()) {...} blocks, if this will help.


Originally is was more architectural decision: by design, EntitySet<t> is unordered (like Dictionary or HashSet)- i.e. generally you can't rely on order there, although currently we ensure it is stable - at least in case with DisconnectedState. So speaking strictly, item index is undefined here. But as we originally seen (it's clear that's wrong), item-related notifications require item index.

So we'll fix this shortly.

Btw, there is a TODO related to this: implement EntitySetView / LiveQuery. There are mainly ideas, actual implemen tation is still under the question, but the idea behind is clear: we must provide a bindable collection, which allows to:

  • Expose either all entities, or just onces cached in DisconnectedState, or entities of a particular EntitySet<t>

  • Filter them by arbitrary predicate

  • Order them by arbitrary expression

  • Listen changes in Session and send appropriate notifications when it content is modified because of them.

As far as I can judge, this is nearly ideal solution for such cases. If you need this, please leave a note \ drop your own ideas.

answered Jul 12 '10 at 18:44

Alex%20Yakunin's gravatar image

Alex Yakunin
29714412

As far as I can judge, there is an event w/o index at all: http://msdn.microsoft.com/en-us/library/ms653207.aspx

(Jul 12 '10 at 18:44) Alex Yakunin Alex%20Yakunin's gravatar image

Clear - so it's fully legal then ;)

(Jul 12 '10 at 18:44) Alex Yakunin Alex%20Yakunin's gravatar image

There is really was an issue related to this: we were raising Reset event in any case, even when Add/Remove events should be raised. The behavior you've seen was not intentional.

The bug is fixed.

(Jul 12 '10 at 18:44) Alex Yakunin Alex%20Yakunin's gravatar image
Your answer
Please start posting your answer anonymously - your answer will be saved within the current session and published after you log in or create a new account. Please try to give a substantial answer, for discussions, please use comments and please do remember to vote (after you log in)!
toggle preview

Subscription:

Once you sign in you will be able to subscribe for any updates here

Tags:

×573

Asked: Jul 12 '10 at 14:10

Seen: 7,553 times

Last updated: Jul 12 '10 at 14:10

powered by OSQA