Databinding: Guide users to not bind against DbSet.Local directly #8898
Comments
|
Leaving open to answer the WPF question. |
|
Note for triage: This issue had one question pending: I tried binding in a WPF to an observable collection that implements IListSource and GetList was called. Given that in 3.0 we want to have better WPF/WinForms support, I would like to discuss if we want to do this (if we decide not to do it, maybe we should close the issue?). On the other hand, this reminded me of #14231, which was recently reported and says DbSet.Local isn't good for iterating either... I can't remember what DbSet.Local is good for |
. Should we make changes?
|
We should look at this as part of the databinding experience for 3.0--see #14318 |
Part of #8898 Fixes #14231 Investigation into DbSet.Local shows that: * Iteration over tracked entities had a lot of LINQ code; fixed by un-LINQing * Multiple composed iterators cause slow-down; fixed by giving LocalView it's only IStateManager method * Filtering causes slow-down; fixed by splitting storage into multiple dictionaries. Makes lookup for entity instance slightly slower. * Calculate Count lazily This means that DbSet.Local iteration is now about 3x faster, although it depends a lot on what is being tracked. Getting an ObservableCollection and then iterating over that _once_ is slower than just iterating over the entries once. Iterating over the ObservableCollection multiple times is still faster than iterating over DbSet.Local multiple times. This seems like a reasonable trade-off. If the application does heavy iteration, then creating a new collection for that seems reasonable. With regard to #8898, the layering we have still seems correct, so moving forward with the breaking changes there.
Part of #8898 Fixes #14231 Investigation into DbSet.Local shows that: * Iteration over tracked entities had a lot of LINQ code; fixed by un-LINQing * Multiple composed iterators cause slow-down; fixed by giving LocalView it's only IStateManager method * Filtering causes slow-down; fixed by splitting storage into multiple dictionaries. Makes lookup for entity instance slightly slower. * Calculate Count lazily This means that DbSet.Local iteration is now about 3x faster, although it depends a lot on what is being tracked. Getting an ObservableCollection and then iterating over that _once_ is slower than just iterating over the entries once. Iterating over the ObservableCollection multiple times is still faster than iterating over DbSet.Local multiple times. This seems like a reasonable trade-off. If the application does heavy iteration, then creating a new collection for that seems reasonable. With regard to #8898, the layering we have still seems correct, so moving forward with the breaking changes there.
Part of #8898 Fixes #14231 Investigation into DbSet.Local shows that: * Iteration over tracked entities had a lot of LINQ code; fixed by un-LINQing * Multiple composed iterators cause slow-down; fixed by giving LocalView it's only IStateManager method * Filtering causes slow-down; fixed by splitting storage into multiple dictionaries. Makes lookup for entity instance slightly slower. * Calculate Count lazily This means that DbSet.Local iteration is now about 3x faster, although it depends a lot on what is being tracked. Getting an ObservableCollection and then iterating over that _once_ is slower than just iterating over the entries once. Iterating over the ObservableCollection multiple times is still faster than iterating over DbSet.Local multiple times. This seems like a reasonable trade-off. If the application does heavy iteration, then creating a new collection for that seems reasonable. With regard to #8898, the layering we have still seems correct, so moving forward with the breaking changes there.
Part of #8898 Fixes #14231 Investigation into DbSet.Local shows that: * Iteration over tracked entities had a lot of LINQ code; fixed by un-LINQing * Multiple composed iterators cause slow-down; fixed by giving LocalView it's only IStateManager method * Filtering causes slow-down; fixed by splitting storage into multiple dictionaries. Makes lookup for entity instance slightly slower. * Calculate Count lazily This means that DbSet.Local iteration is now about 3x faster, although it depends a lot on what is being tracked. Getting an ObservableCollection and then iterating over that _once_ is slower than just iterating over the entries once. Iterating over the ObservableCollection multiple times is still faster than iterating over DbSet.Local multiple times. This seems like a reasonable trade-off. If the application does heavy iteration, then creating a new collection for that seems reasonable. With regard to #8898, the layering we have still seems correct, so moving forward with the breaking changes there.
Part of #8898 Fixes #14231 Investigation into DbSet.Local shows that: * Iteration over tracked entities had a lot of LINQ code; fixed by un-LINQing * Multiple composed iterators cause slow-down; fixed by giving LocalView it's only IStateManager method * Filtering causes slow-down; fixed by splitting storage into multiple dictionaries. Makes lookup for entity instance slightly slower. * Calculate Count lazily This means that DbSet.Local iteration is now about 3x faster, although it depends a lot on what is being tracked. Getting an ObservableCollection and then iterating over that _once_ is slower than just iterating over the entries once. Iterating over the ObservableCollection multiple times is still faster than iterating over DbSet.Local multiple times. This seems like a reasonable trade-off. If the application does heavy iteration, then creating a new collection for that seems reasonable. With regard to #8898, the layering we have still seems correct, so moving forward with the breaking changes there.
|
I started to work on this, but it turns out that we already did this for 2.0. See #9003 The only reason this issue was still open was to check if WPF calls the method, which has now been done., so closing. |
|
@roji I think this is a consequence of /cc @AndriySvyryd |
|
@ajcvickers we did do #23971 to preserve the synchronization context when accepting changes - is there something here with Local.ToObservableCollection that goes beyond that (am not too familiar with that...)? |
|
@roji Yes, it gets called when materializing entities/doing fixup after queries. |
|
Ok. Just to make sure given the issue title: is this something we want to recommend/support? (I have no opinion here whatsoever, just asking) |
|
Let's discuss in triage. This issue is unrelated--just where the comment was made. |
AndriySvyryd
added
type-bug
and removed
closed-fixed
|
Triage: I'll take a look to see if it could make sense to do ConfigureAwait(true) for this path, like we did for SaveChanges. Otherwise we could consider some sort of per-query or context option to just do true everywhere, if we think that's important. |
|
EnableCollectionSynchronization helped me! sorry to stir up your issue.. ) |
|
Opened #27533 |
ajcvickers
added
closed-fixed
In EF Core
DbSet.Localis more efficient than in EF6 but doesn't have stable order, so it is not good for databinding. For databinding you are supposed to callToObservableCollection(for the WPF flavor) orToBindingList(for the WinForms flavor).This is already in the XML documentation but nothing prevents users from binding directly to
DbSet.Local.We should consider implementing
IListSource.GetList()to throw an informative exception like we do for query, but we also need to verify that doing so will help for WPF.The text was updated successfully, but these errors were encountered: