Skip to content

ATS: performance updates #322

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
natemcmaster merged 3 commits into from
Jun 17, 2014
Merged

ATS: performance updates #322

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3c91225
Perf improvement and thread safety for TableEntity factory.
Jun 16, 2014
5e8cb77
Update TableFilter to use delegates instead of reflection. Add more t…
Jun 16, 2014
f74207f
Moved extensions to more visible namespaces. Formatting fixes. Rebase to
Jun 16, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 5 additions & 6 deletions src/Microsoft.Data.Entity.AzureTableStorage/Adapters/StateEntryTableEntityAdapter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,11 @@ public void ReadEntity(IDictionary<string, EntityProperty> properties, Operation
foreach (var property in properties)
{
var entityProp = entityType.TryGetPropertyByStorageName(property.Key);
if (entityProp == null
|| entityProp.IsClrProperty && MismatchedTypes(property.Value.PropertyType, entityProp.PropertyType))
if (entityProp != null
&& entityProp.IsClrProperty && EdmTypeMatchesClrType(property.Value.PropertyType, entityProp.PropertyType))
{
continue;
SetProperty(entityProp,property.Value.PropertyAsObject);
}
SetProperty(entityProp,property.Value.PropertyAsObject);
}
}

Expand Down Expand Up @@ -125,9 +124,9 @@ private void SetProperty(IProperty prop, object value)
}
}

private bool MismatchedTypes(EdmType propertyType, Type clrType)
private static bool EdmTypeMatchesClrType(EdmType edmType, Type clrType)
{
switch (propertyType)
switch (edmType)
{
case EdmType.String:
return typeof(string).IsAssignableFrom(clrType);
Expand Down
37 changes: 23 additions & 14 deletions src/Microsoft.Data.Entity.AzureTableStorage/Adapters/TableEntityAdapterFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;
using System.Reflection;
using System.Linq.Expressions;
using JetBrains.Annotations;
using Microsoft.Data.Entity.AzureTableStorage.Utilities;
using Microsoft.Data.Entity.ChangeTracking;
using Microsoft.Data.Entity.Utilities;
using Microsoft.WindowsAzure.Storage.Table;

namespace Microsoft.Data.Entity.AzureTableStorage.Adapters
Expand All @@ -17,21 +17,30 @@ public ITableEntity CreateFromStateEntry([NotNull] StateEntry entry)
{
Check.NotNull(entry, "entry");

var type = entry.Entity.GetType();
var ctor = GetOrMakeStateAdapterCtor(type);
return (ITableEntity) ctor.Invoke(new object[] { entry });
var entityType = entry.Entity.GetType();
var ctor = GetOrMakeCreator(entityType);
return ctor(entry);
}
private readonly IDictionary<Type, ConstructorInfo> _stateCtors = new Dictionary<Type, ConstructorInfo>();

private ConstructorInfo GetOrMakeStateAdapterCtor(Type objType)
private readonly ThreadSafeDictionaryCache<Type, Func<StateEntry, ITableEntity>> _instanceCreatorCache = new ThreadSafeDictionaryCache<Type, Func<StateEntry, ITableEntity>>();
private Func<StateEntry, ITableEntity> GetOrMakeCreator(Type objType)
{
if (_stateCtors.ContainsKey(objType))
{
return _stateCtors[objType];
}
var ctor = typeof(StateEntryTableEntityAdapter<>).MakeGenericType(objType).GetConstructor(new[] { typeof(StateEntry) });
_stateCtors[objType] = ctor;
return ctor;
return _instanceCreatorCache.GetOrAdd(objType, type =>
{
var paramExpression = new[]
{
Expression.Parameter(typeof(StateEntry), "entry")
};
var ctorExpression = Expression.New(
typeof(StateEntryTableEntityAdapter<>)
.MakeGenericType(type)
.GetConstructor(new[] { typeof(StateEntry) }),
paramExpression
);

var lambda = Expression.Lambda<Func<StateEntry, ITableEntity>>(ctorExpression, paramExpression);
return lambda.Compile();
});

@ajcvickers ajcvickers Jun 17, 2014

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This good because it creates a compiled delegate to create instances. I'm fine with it staying like this for now, and possibly for always. However, it is possible that for project-n the delegate created will be interpreted rather than truly compiled. This is one of the reasons we are avoiding creating "compiled" delegates where possible. Take a look at SimpleEntityKeyFactory and related classes. In these classes we avoid having to compile any dynamic code by creating a generic type for the factory using Reflection, creating one instance of this factory, then caching the factory instance such that it can be used over and over to create SimpleEntityKey instances without Reflection and without using any "compiled" delegates. All that being said, I am fine with leaving this code as is for now, especially since it is for ATS which does not currently intersect with project-n.

@divega divega Jun 17, 2014

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I talked to Arthur about this. I think it is worth having a TODO to consider doing what he described later.

@natemcmaster natemcmaster Jun 17, 2014

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. For now, I'm intentionally not caching instances to avoid storing an adapter for every entity in the table. This would be faster on the CPU in some cases, but worse for memory in others. These issues can be mitigated by limiting the cache size or using a different method of generic type construction. I'll add a TODO to investigate these options and their performance gains.

}
}
}
13 changes: 10 additions & 3 deletions src/Microsoft.Data.Entity.AzureTableStorage/AtsBatchedDataStore.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,16 @@ protected AtsBatchedDataStore(AtsConnection connection, TableEntityAdapterFactor
{
}

public AtsBatchedDataStore([NotNull] DbContextConfiguration configuration, [NotNull] AtsConnection connection, [NotNull] AtsQueryFactory queryFactory, [NotNull] TableEntityAdapterFactory tableEntityFactory)
public AtsBatchedDataStore([NotNull] DbContextConfiguration configuration,
[NotNull] AtsConnection connection,
[NotNull] AtsQueryFactory queryFactory,
[NotNull] TableEntityAdapterFactory tableEntityFactory)
: base(configuration, connection, queryFactory, tableEntityFactory)
{
}

public override async Task<int> SaveChangesAsync(IReadOnlyList<StateEntry> stateEntries, CancellationToken cancellationToken = new CancellationToken())
public override async Task<int> SaveChangesAsync(IReadOnlyList<StateEntry> stateEntries,
CancellationToken cancellationToken = new CancellationToken())
{
cancellationToken.ThrowIfCancellationRequested();
var tableGroups = stateEntries.GroupBy(s => s.EntityType.StorageName);
Expand All @@ -53,7 +57,10 @@ public AtsBatchedDataStore([NotNull] DbContextConfiguration configuration, [NotN
foreach (var partitionGroup in partitionGroups)
{
var batch = new TableBatchOperation();
foreach (var operation in partitionGroup.Select(entry => GetOperation(entry, EntityFactory.CreateFromStateEntry(entry))).Where(operation => operation != null))
foreach (var operation in partitionGroup
.Select(entry => GetOperation(entry, EntityFactory.CreateFromStateEntry(entry)))
.Where(operation => operation != null)
)
{
// TODO allow user access to config options: Retry Policy, Secondary Storage, Timeout
batch.Add(operation);
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.Data.Entity.AzureTableStorage/AtsDataStore.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public override IEnumerable<TResult> Query<TResult>(QueryModel queryModel, State
Check.NotNull(stateManager, "stateManager");

var compilationContext = _queryFactory.MakeCompilationContext(Model);
var queryExecutor = compilationContext.CreateVisitor().CreateQueryExecutor<TResult>(queryModel);
var queryExecutor = compilationContext.CreateQueryModelVisitor().CreateQueryExecutor<TResult>(queryModel);
var queryContext = _queryFactory.MakeQueryContext(Model, Logger, stateManager, Connection);
return queryExecutor(queryContext, null);
}
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.Data.Entity.AzureTableStorage/AtsDataStoreSource.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

namespace Microsoft.Data.Entity.AzureTableStorage
{
internal class AtsDataStoreSource : DataStoreSource<
public class AtsDataStoreSource : DataStoreSource<
AtsDataStore,
AtsOptionsExtension,
AtsDataStoreCreator,
Expand Down
4 changes: 3 additions & 1 deletion src/Microsoft.Data.Entity.AzureTableStorage/AtsDatabaseExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@

using System;
using JetBrains.Annotations;
using Microsoft.Data.Entity.AzureTableStorage;
using Microsoft.Data.Entity.AzureTableStorage.Utilities;
using Microsoft.Data.Entity.Infrastructure;

namespace Microsoft.Data.Entity.AzureTableStorage
// ReSharper disable once CheckNamespace
namespace Microsoft.Data.Entity
{
public static class AtsDatabaseExtensions
{
Expand Down
1 change: 1 addition & 0 deletions src/Microsoft.Data.Entity.AzureTableStorage/AtsOptionsExtension.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using Microsoft.Data.Entity.Infrastructure;
using Microsoft.Framework.DependencyInjection;

namespace Microsoft.Data.Entity.AzureTableStorage
{
Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.Data.Entity.AzureTableStorage/EntityBuilderExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
using System.Linq.Expressions;
using JetBrains.Annotations;
using Microsoft.Data.Entity.AzureTableStorage.Utilities;
using Microsoft.Data.Entity.Metadata;

namespace Microsoft.Data.Entity.AzureTableStorage
// ReSharper disable once CheckNamespace
namespace Microsoft.Data.Entity.Metadata
{
public static class EntityBuilderExtensions
{
Expand Down
6 changes: 4 additions & 2 deletions src/Microsoft.Data.Entity.AzureTableStorage/EntityServicesBuilderExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using JetBrains.Annotations;
using Microsoft.Data.Entity;
using Microsoft.Data.Entity.AzureTableStorage;
using Microsoft.Data.Entity.AzureTableStorage.Adapters;
using Microsoft.Data.Entity.AzureTableStorage.Query;
using Microsoft.Data.Entity.AzureTableStorage.Utilities;
using Microsoft.Data.Entity.Storage;
using Microsoft.Framework.DependencyInjection;

namespace Microsoft.Data.Entity.AzureTableStorage
// ReSharper disable once CheckNamespace
namespace Microsoft.Framework.DependencyInjection
{
public static class EntityServicesBuilderExtensions
{
Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.Data.Entity.AzureTableStorage/Query/AtsQueryCompilationContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@ public class AtsQueryCompilationContext : QueryCompilationContext
public TableFilterFactory TableFilterFactory { get; private set; }

public AtsQueryCompilationContext([NotNull] IModel model, [NotNull] TableFilterFactory tableFilterFactory)
: base(model)
: base(model, new LinqOperatorProvider(), new ResultOperatorHandler())
{
Check.NotNull(tableFilterFactory, "tableFilterFactory");
TableFilterFactory = tableFilterFactory;
}

public override EntityQueryModelVisitor CreateVisitor()
public override EntityQueryModelVisitor CreateQueryModelVisitor()
{
return new AtsQueryModelVisitor(this);
}
Expand Down
Loading