FetchTypedView re-entrancy

Posts   
 
    
obzekt
User
Posts: 60
Joined: 29-Apr-2004
# Posted on: 25-Jul-2018 17:27:44   

Hello. We use LLBL 5.1 with the adapter model on our application server, and we call the FetchTypedView method passing a static instance of an IEntityFields2 collection as its first parameter. This is a little optimization, to avoid creating a new collection on every service call, as it has dozens of fields and some logic about their inclusion.

We thought that parameter is only being read, but apparently it is also modified. That causes a race condition leading to excessive CPU utilization, and exceptions like this:


System.ArgumentException: An item with the same key has already been added.
   at System.ThrowHelper.ThrowArgumentException(ExceptionResource resource)
   at System.Collections.Generic.Dictionary`2.Insert(TKey key, TValue value, Boolean add)
   at SD.LLBLGen.Pro.ORMSupportClasses.EntityFieldsCore`1.set_Item(Int32 index, TField value)
   at SD.LLBLGen.Pro.ORMSupportClasses.FieldUtilities.FixFirstFieldForSourceDetermination(IList fieldsToFix, IRelationCollection relations)
   at SD.LLBLGen.Pro.ORMSupportClasses.DataAccessAdapterCore.PreprocessQueryElements(QueryParameters parameters)
   at SD.LLBLGen.Pro.ORMSupportClasses.DataAccessAdapterCore.FetchTypedList(DataTable dataTableToFill, QueryParameters parameters)
   at SD.LLBLGen.Pro.ORMSupportClasses.DataAccessAdapterBase.<>c__DisplayClass14_0.<FetchTypedList>b__0()
   at SD.LLBLGen.Pro.ORMSupportClasses.DataAccessAdapterCore.FetchTypedView(DataTable dataTableToFill, QueryParameters parameters)

We plan to fix this bug on our end by adding a [ThreadStatic] attribute on that IEntityFields2 instance, but I would like to know if this is intentional by LLBL. If yes, maybe there should be a note about it in the documentation.

Walaa avatar
Walaa
Support Team
Posts: 14946
Joined: 21-Aug-2005
# Posted on: 25-Jul-2018 20:36:45   

We thought that parameter is only being read, but apparently it is also modified.

How do you fetch the typedView, could you please provide the smallest reproducible code snippet?

Thank you.

obzekt
User
Posts: 60
Joined: 29-Apr-2004
# Posted on: 26-Jul-2018 08:38:34   

Here's some sample code. It is a WCF service method called from different threads.


private static IEntityFields2 _myFields  = new VAuditEventTypedView().GetFieldsInfo();

public VAuditEventTypedView GetAuditEvents(...)
{
    RelationPredicateBucket rpb = ...
    VAuditEventTypedView view = new VAuditEventTypedView();
    using (var adapter = new DataAccessAdapter())
        adapter.FetchTypedView(_myFields, view, rpb, false);

    return view;
}

Otis avatar
Otis
LLBLGen Pro Team
Posts: 39590
Joined: 17-Aug-2003
# Posted on: 26-Jul-2018 09:45:14   

It's by design, because the fields are used to build the query with. Everything that's used to build the query with isn't thread safe as it's augmented along the way to make query generation possible, and the philosophy is that the input to produce the query is created just for that purpose. This means that you can't use them in a cross-thread manner. In general this isn't an issue as the objects are created for just the purpose of creating the query, and then tossed away. Thread safety is something that's always 'false' unless it's specified as such.

Creating field objects is fast, it shouldn't be noticeable. If you want to re-use them, you can call EntityFields2.Clone() to create a deep clone of the fields object (it clones the expressions as well etc.)

However Clone() doesn't use a lock so in the end you still need to use a lock around the Clone() call. I haven't measured whether that's faster than re-creating it, but for a normal set of fields for e.g. an entity or typed list, I'd assume it's (a bit) slower.

So I'd first try to simply re-create the fields every time and measure whether that's significantly slower and if so, cache it and use a lock and Clone() it. Using ThreadSafe doesn't work in a web-oriented scenario as requests could hop threads.

As a db call is a magnitude slower than recreating a large set of fields, I wouldn't worry about recreating it. Only cache if profiling suggested it's a bottleneck and caching would fix things. (we already cache a great deal of that for you btw: the field info objects are already cached and re-used as it's static data hence recreating the fields is fast and likely not a bottleneck).

Frans Bouma | Lead developer LLBLGen Pro
obzekt
User
Posts: 60
Joined: 29-Apr-2004
# Posted on: 26-Jul-2018 10:08:28   

Thanks. In our case each service method is serviced by the same WCF thread from the pool. It doesn't change context (at least that's my understanding). So for now we added this [ThreadStatic] attribute, and we will monitor it. I assume that the fields collection can be safely re-used once FetchTypedView returns.

I agree with though that it's probably a meaningless optimization, considering the query will take much longer. It's just that old-time programmers try to squeeze performance as much as they can wink

The earlier sample was simplistic. In reality we have dynamic generation of the needed fields, and they are ~90 of them:


...
int j = 0;
for (int i = 0; i < (int)VAuditEventFieldIndex.AmountOfFields; i++)
    //Exclude the fields that create duplicate records
    if (!excludedFields.Contains((VAuditEventFieldIndex)i))
        fields[j++] = EntityFieldFactory.Create((VAuditEventFieldIndex)i);

Otis avatar
Otis
LLBLGen Pro Team
Posts: 39590
Joined: 17-Aug-2003
# Posted on: 26-Jul-2018 13:13:26   

yeah you can re-use the fields after a query, as long as they're not shared among threads in other queries. simple_smile

Frans Bouma | Lead developer LLBLGen Pro