Missing code in the SqlCeDatastore_Select.cs

Sep 27, 2012 at 11:23 PM
Edited Sep 28, 2012 at 12:39 PM

Hello,

First let me say I'm a big fan of your ORM. I've worked a lot with it and on it in the last 4 months. It's really a great tool which saved me a lot of coding.

I was browsing the latest version of your code (to see how you implemented the CreateProxy principle since I took the same path) and noticed something suspicious in the above mentioned file in the Select method starting at line 311:

 

object rowPK = null;

bool fieldsSet;
object item = CreateEntityInstance(entityName, objectType, m_fields, results, out fieldsSet);

if (!fieldsSet)
{
	// fill in the entity field values
	PopulateFields(entityName, m_fields, results, item, fillReferences);
}
// autofill references if desired
if ((fillReferences) && (referenceFields.Length > 0))
{
	FillReferences(item, rowPK, referenceFields, false);
}

 

The rowPK is not set anymore before it's given to the FillReferences.

Since you moved the field loop out of the method to put it in PopulateFields, this value is not set anymore. You did set it in the PopulateFields but it never comes out of the method.

 

Also, is there any reason why you are mapping the CreateProxy method during the Select rather than in the AddType of the DataStore class?

Since it's where you do all the rest of the mapping I'd do it there, but I might have missed a purpose for doing otherwise...

 

Also, I wanted to know if you did try using the Type.MakeGenericType method to create typed lists of objects?

 

var genType = typeof(List<>).MakeGenericType(entityType);
var list = (IList)Activator.CreateInstance(genType);

 

At the moment you create a List<object>, add the items to it then convert it to an array. Wouldn't it be easier to work with the result if you just returned the typed list?

Generic Methods would then return List<T> and the others would return IList.

At least that's what I'd like to implement to save myself coding around the arrays, but I didn't went down that path yet. I'm working on improving performances and adding support for ManyToMany references (through a mapping table generated behind the scene).

 

You inspired me a lot!

Keep up the excellent work!

 

Regards.

Sep 28, 2012 at 12:39 PM

I forgot to say that I did win some performance using a Delegate to store the CreateProxy method rather than using the MethodInfo then calling MethodInfo.Invoke().

After tests over a table with 2,3M records, I won 35-40% of the execution time. But it is still 2 times slower than directly instantiating the object and assigning the properties.

For comparison I tested the other two methods:

  • Using Activator.CreateInstance() with parameters and having a constructor accepting the IDataReader and the Ordinals. Result: 30% slower than the delegate method.
  • Mapping the ConstructorInfo when building the EntityCollection and calling it with ConstructorInfo.Invoke() with parameters. Result: 20% slower.

So I'm making a win by using the Delegate approach and it has the advantages you mentioned of using a Static Private method.

 

Regards.

Coordinator
Sep 28, 2012 at 2:38 PM

All good questions and observations.  I'd bet that other than myself, you're probably the only one who's taken the time to browse the code with any depth.  Definitely the first to ask about it or provide a code review, so let me say that I appreciate that. So here are some thoughts:

  1. Good catch on the rowPK no longer getting populated. It actually should get removed from the _Select code file for clarity.  Right now it's always null when it goes into FillReferences, where it then gets populated (SQLStoreBase.cs line 843).  Originally I kept a copy as I was iterating the list of fields, as it's wasteful to go look something up if you've already looked at it.  The refactor should have removed that rowPK completely and I'll likely have that in my next change set.  Thanks.
  2. Mapping the CreateProxy method was done in Select purely as a convenience.  I was working with Select when I had the idea and just put it there.  I'd agree that for consistency putting it in AddType where the other mappings happen would probably aid to code clarity.
  3. No, I didn't try using Type.MakeGenericType.  It's definitely worth investigationg as it would simplify things.
  4. The Delegate for the proxy is a great idea.  Anything that gains perf is a plus in my book.  I'll look at doing that (unless you've already done it).

If you have a patch you'd like to submit, or if you'd like direct developer access to the code base, just let me know.  I'm happy to integrate any changes that improve the overall usability, performance or usefulness of the library.

 

Sep 28, 2012 at 3:52 PM
Edited Sep 28, 2012 at 3:58 PM

I've indeed worked in depth on your code (and loved every bit of it).

Though, I fear I won't be able to give you a patch since my version of your project has changed a lot from what it was 4 months ago. It's my fault for not checking your updates as they appeared. I added quite a few bits of codes since I needed specific methods and functionality which tend to make my life easier (my REST webservices have gone incredibly simple thanks to your ORM). Though, you did add some very neat features on your side, that I'll need to implement on mine.

Among other things I added support for MSSQL and I'm in the process of adding other databases. The thing is, we use this ORM on all our platforms (Mobile, Silverlight, Server) so I had to extend it a little. The main idea was to share the same class library among all applications for consistency (for Silverlight I'm just doing an ORM mockup in order to be able to re-use the classes).

 

On the other hand, I'll be glad to give you whatever snippets you might find interesting.

For the Delegate it simply boils down to (using your naming convention):

 

// inside DataStore.cs - AddType()
var methodInfo = entityType.GetMethod("ORM_CreateProxy", BindingFlags.NonPublic | BindingFlags.Static | BindingFlags.Instance);
if (methodInfo != null)
	map.CreateProxy = (EntityInfo.CreateProxyDelegate)Delegate.CreateDelegate(typeof(EntityInfo.CreateProxyDelegate), methodInfo);

// inside EntityInfo.cs - Note that I'm computing the ordinals on each select since I plan to add Table Joins for a specific type of queries.

public delegate object CreateProxyDelegate(System.Data.IDataReader reader, Dictionary<string, int> ordinals);
public CreateProxyDelegate CreateProxy { get; internal set; }

// inside the Select methods

item = entity.CreateProxy.Invoke(results, dicOrdinals);

 

I've done extensive testings of all possible methods and this one gave me the best results.

 

On the other hand, I had a very interesting problem when trying to solve the performance issue.

Running the following snippet outside the ORM classes gives me 50% speed gain and comes close to what SQL Management Studio does to render all the info.

 

var conn = new System.Data.SqlServerCe.SqlCeConnection(String.Format("Data Source={0};Persist Security Info=False;Max Database Size=500;", "demo.sdf"));
var items = new List();
using (var cmd = new System.Data.SqlServerCe.SqlCeCommand(entity.EntityName, conn))
{
	conn.Open();

	cmd.CommandType = CommandType.TableDirect;
	start = DateTime.Now;
	var r = cmd.ExecuteResultSet(System.Data.SqlServerCe.ResultSetOptions.None);
	System.Diagnostics.Debug.WriteLine(String.Format("Table direct in {0}ms", DateTime.Now.Subtract(start).TotalMilliseconds));

	var ordinals = new Dictionary();
	foreach (var field in entity.Fields)
	{
		ordinals.Add(field.FieldName, r.GetOrdinal(field.FieldName));
	}
	start = DateTime.Now;
	while (r.Read())
	{
		items.Add(entity.CreateProxy.Invoke(r, ordinals));
	}
	System.Diagnostics.Debug.WriteLine(String.Format("populated in {0} ({1})", DateTime.Now.Subtract(start).TotalMilliseconds, items.Count));
}

 

Though, if you look closely, it's the same exact code as in the ORM itself (just strip down the check for references and so on). I was unable to find a reason why it would be two times faster when ran outside the Select method.

I finally gave up trying because I never require to pull 2M records out my various DBs. Tests done on 200k records did show a performance loss of 20-25% which is acceptable considering the ridiculous short times involved and our field of applications.

Rendering all those records on screen takes a lot more time.

 

I need to check your additions before I would touch the code base.

ctacke wrote:
  1. Good catch on the rowPK no longer getting populated. It actually should get removed from the _Select code file for clarity.  Right now it's always null when it goes into FillReferences, where it then gets populated (SQLStoreBase.cs line 843).  Originally I kept a copy as I was iterating the list of fields, as it's wasteful to go look something up if you've already looked at it.  The refactor should have removed that rowPK completely and I'll likely have that in my next change set.  Thanks.

 

My bad... I missed that one. Not enough sleep I fear... it makes more sense to do it there indeed (good safeguard aswell).

 

Thanks for your answers!

Coordinator
Sep 28, 2012 at 6:02 PM

I've updated the code base to use Delegates for the creator proxy, plus added support in EntityGenerator to generate the proxy method in generated Entities as well.

I'd love to head about any extensions you've needed as well.  We use ORM for CF, Desktop and Mono for Android projects using SQLCE and SQLite.  I keep considering a desktop SQL implementation, but haven't yet had the requirement.

Sep 28, 2012 at 10:25 PM

Basically our requirements were/are:

  • Transactions for cascaded actions (insert, update, delete), which I added individually on all commands so I can run multiple transactions in parallel. This is needed for our webservices and windows services.
  • Support for MS-SQL/Firebird (we mostly use MS-SQL, firebird is not yet added). We plan to add Oracle support aswell.
  • Some functions for classic actions like InsertOrUpdate, BulkInsert/BulkInsertOrUpdate (to avoid writing the loop in the code, plus using the same connection in MS-SQL to speed things up), DropAndCreateTable, etc. The goal was to simplify the code of the webservices and the mobile applications for the synchronisation part.
  • Performance tweaking by making some changes in the code like the Delegate trick. I also modified all queries to avoid using the wildcard.
  • Support for multiple primary keys (will be needed for mapping tables)
  • Sorting (with multiple columns) of the Select queries to reduce the need of sorting in the applications
  • Update of the DB structure and tables without the need to drop them (adding missing columns on the fly with CreateOrUpdateDataStore). This is done at application start and ensures the DB is up to date with the application running on it.
  • Silverlight support for the classes (mockup of the attributes and their parameters types), so we can use the same libraries in all applications.
  • ManyToMany relations with automatic creation of the mapping table, since we use this a lot in our applications on the server side (like the mapping between users and roles). (in progress)
  • OneToOne relations for "container" objects. The goal here is to increase the efficiency by loading both tables at the same time with a join. (not done yet)
  • The ability to load a class tree with one big Left Join based query then building the tree out of all the lines. It's something we do repeatedly on MS-SQL and Firebird. (not done yet)

Some of these changes required heavy tweaking in the overall structure. Especially the Transactions part. Since I didn't want the programmers to handle the transactions themselves, I added the logic inside the different functions that you can call with a bool to activate transaction mode.

To be honest I lacked the time to test on Android / Windows Phone since we don't use them (yet) but we already use the rest for the last 3 months (on WM6.5, Silverlight, Desktop) in a PoC environment with very good results. I must say everyone is pretty happy with how it works and how it simplified our code.

Sep 30, 2012 at 9:54 PM

Can you please give me an example where you'd use the Dynamic Entity?

It sounds like an interesting addition, but I didn't see where I'd use it yet.

 

For the rest, I see you added events around all important actions, great addition!

Coordinator
Oct 1, 2012 at 10:09 PM

For thoughts on the DynamicEntity, take a look at my blog entry here.

The events are the prelude to Triggers.

 

Oct 2, 2012 at 6:14 AM
ctacke wrote:

For thoughts on the DynamicEntity, take a look at my blog entry here.

Clever solution!

Even if I don't have any requirement for that at the moment, I will build this in. It can always come in handy...

 

I had thought of the Events as triggers indeed, so I added them already. That's a great addition. I don't have any requirement for them at the moment, but I can easily see that coming in some cases here.