Skip to content

Optimize FieldAccessor #335

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
merged 4 commits into from
Jan 15, 2025
Merged

Optimize FieldAccessor #335

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
dadc8df
Optimize `FieldAccessor`
SergeiPavlov Jan 14, 2025
8752275
Remove DefaultValue prop
SergeiPavlov Jan 14, 2025
34bf075
override
SergeiPavlov Jan 14, 2025
8a9c1e5
Fix EnumFieldAccessor
SergeiPavlov Jan 14, 2025
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
41 changes: 12 additions & 29 deletions Orm/Xtensive.Orm/Orm/Internals/FieldAccessor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,40 +4,23 @@
// Created by: Alex Yakunin
// Created: 2010.02.19

using System;
using System.Diagnostics;
using Xtensive.Core;
using Xtensive.Orm.Model;

namespace Xtensive.Orm.Internals
{
internal abstract class FieldAccessor
{
private FieldInfo fld;

public FieldInfo Field {
get { return fld; }
set {
if (fld !=null)
throw Exceptions.AlreadyInitialized("Field");
fld = value;
}
}

public object DefaultUntypedValue { get; private set; }

public abstract bool AreSameValues(object oldValue, object newValue);
namespace Xtensive.Orm.Internals;

public abstract void SetUntypedValue(Persistent obj, object value);

public abstract object GetUntypedValue(Persistent obj);
internal abstract class FieldAccessor(object defaultUntypedValue)
{
protected ColNum FieldIndex;

public object DefaultUntypedValue { get; } = defaultUntypedValue;

// Constructors

protected FieldAccessor(object defaultUntypedValue)
{
DefaultUntypedValue = defaultUntypedValue;
}
public virtual void SetFieldInfo(FieldInfo value)
{
FieldIndex = FieldIndex == 0 ? value.MappingInfo.Offset : throw Exceptions.AlreadyInitialized("Field");
}

public abstract bool AreSameValues(object oldValue, object newValue);
public abstract void SetUntypedValue(Persistent obj, object value);
public abstract object GetUntypedValue(Persistent obj);
}
4 changes: 2 additions & 2 deletions Orm/Xtensive.Orm/Orm/Internals/FieldAccessorProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ private static FieldAccessor CreateFieldAccessor(Type accessorType, FieldInfo fi
accessorType.CachedMakeGenericType(field.ValueType),
BindingFlags.CreateInstance | BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance,
null, Array.Empty<object>(), null);
accessor.Field = field;
accessor.SetFieldInfo(field);
Copy link

Choose a reason for hiding this comment

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

Thinking out loud: maybe we can convert all these accessors to structs and get rid of virtual call dispatching as well? E.g. make them generic arguments of FieldAccessor
Could it be possible?

And maybe we can replace SetFieldInfo with single ctor argument?

Copy link
Collaborator Author

@SergeiPavlov SergeiPavlov Jan 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This operation .SetFieldInfo() invoked only once, during Domain building
Little profit of optimizing it.

And I don't see easy way to use Accesor.GetValue()/SetValue() without virtual functions.
It would be to emulate vtbl by some other means

Copy link

Choose a reason for hiding this comment

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

I mean not only this operation is worth optimizing. But others as well. These accessors seem to be good candidates for compiler optimizations with devirtualizing. There is not much of shared code and actually there is little sense to use inheritance.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I had an idea to inherit *Accessor from FieldInfo
because they are always together and mutually referenced
that will save yet one pointer dereference during access

return accessor;
}
}
}
}
25 changes: 13 additions & 12 deletions Orm/Xtensive.Orm/Orm/Internals/FieldAccessors/CachingFieldAccessor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,21 @@
// Created by: Dmitri Maximov
// Created: 2009.07.08

using System;
using Xtensive.Core;
using Xtensive.Orm.Model;

namespace Xtensive.Orm.Internals.FieldAccessors
namespace Xtensive.Orm.Internals.FieldAccessors;

internal abstract class CachingFieldAccessor<T> : FieldAccessor<T>
{
internal abstract class CachingFieldAccessor<T> : FieldAccessor<T>
{
public static Func<Persistent, FieldInfo, IFieldValueAdapter> Constructor;
public static Func<Persistent, FieldInfo, IFieldValueAdapter> Constructor;

protected FieldInfo Field;

public override T GetValue(Persistent obj)
{
var field = Field;
var valueAdapter = obj.GetFieldValueAdapter(field, Constructor);
return (T) valueAdapter;
}
public override void SetFieldInfo(FieldInfo value) {
Field = Field is null ? value : throw Exceptions.AlreadyInitialized("Field");
base.SetFieldInfo(value);
}
}

public override T GetValue(Persistent obj) => (T) obj.GetFieldValueAdapter(Field, Constructor);
}
11 changes: 2 additions & 9 deletions Orm/Xtensive.Orm/Orm/Internals/FieldAccessors/DefaultFieldAccessor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
// Created by: Alexey Gamzov
// Created: 2008.05.26

using System;
using Xtensive.Reflection;

namespace Xtensive.Orm.Internals.FieldAccessors
Expand Down Expand Up @@ -32,17 +31,11 @@ public override bool AreSameValues(object oldValue, object newValue)
}

/// <inheritdoc/>
public override T GetValue(Persistent obj)
{
var fieldIndex = Field.MappingInfo.Offset;
var tuple = obj.Tuple;
var value = tuple.GetValueOrDefault<T>(fieldIndex);
return value;
}
public override T GetValue(Persistent obj) => obj.Tuple.GetValueOrDefault<T>(FieldIndex);

/// <inheritdoc/>
/// <exception cref="InvalidOperationException">Invalid arguments.</exception>
public override void SetValue(Persistent obj, T value) =>
obj.Tuple.SetValue(Field.MappingInfo.Offset, value);
obj.Tuple.SetValue(FieldIndex, value);
}
}
54 changes: 24 additions & 30 deletions Orm/Xtensive.Orm/Orm/Internals/FieldAccessors/EntityFieldAccessor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,54 +4,48 @@
// Created by: Alexey Gamzov
// Created: 2008.05.26

using System;

using Xtensive.Core;
using Xtensive.Orm.Model;
using Xtensive.Tuples;

namespace Xtensive.Orm.Internals.FieldAccessors
{
internal class EntityFieldAccessor<T> : FieldAccessor<T>
{
/// <inheritdoc/>
public override bool AreSameValues(object oldValue, object newValue)
private FieldInfo field;

public override void SetFieldInfo(FieldInfo value)
{
return ReferenceEquals(oldValue, newValue);
field = field is null ? value : throw Exceptions.AlreadyInitialized("Field");
base.SetFieldInfo(value);
}

/// <inheritdoc/>
public override bool AreSameValues(object oldValue, object newValue) => ReferenceEquals(oldValue, newValue);
/// <inheritdoc/>
/// <exception cref="InvalidOperationException">Invalid arguments.</exception>
public override void SetValue(Persistent obj, T value)
{
var entity = value as Entity;
var field = Field;
var tuple = obj.Tuple;
if (value is Entity entity) {
if (entity.Session != obj.Session)
throw new InvalidOperationException(string.Format(Strings.ExEntityXIsBoundToAnotherSession, entity.Key));

if (!ReferenceEquals(value, null) && entity==null)
throw new InvalidOperationException(string.Format(
Strings.ExValueShouldBeXDescendant, WellKnownOrmTypes.Entity));

if (entity!=null && entity.Session!=obj.Session)
throw new InvalidOperationException(string.Format(
Strings.ExEntityXIsBoundToAnotherSession, entity.Key));

var mappingInfo = field.MappingInfo;
int fieldIndex = mappingInfo.Offset;
if (entity==null) {
int nextFieldIndex = fieldIndex + mappingInfo.Length;
for (int i = fieldIndex; i < nextFieldIndex; i++)
obj.Tuple.SetValue(i, null);
entity.Key.Value.CopyTo(tuple, 0, FieldIndex, field.MappingInfo.Length);
}
else {
entity.Key.Value.CopyTo(obj.Tuple, 0, fieldIndex, mappingInfo.Length);
if (!ReferenceEquals(value, null))
throw new InvalidOperationException(string.Format(Strings.ExValueShouldBeXDescendant, WellKnownOrmTypes.Entity));

for (int i = FieldIndex, nextFieldIndex = FieldIndex + field.MappingInfo.Length; i < nextFieldIndex; i++)
tuple.SetValue(i, null);
}
}

/// <inheritdoc/>
public override T GetValue(Persistent obj)
{
var field = Field;
Key key = obj.GetReferenceKey(field);
if (key is null)
return default(T);
return (T) (object) obj.Session.Query.SingleOrDefault(key);
}
public override T GetValue(Persistent obj) =>
obj.GetReferenceKey(field) is { } key
? (T) (object) obj.Session.Query.SingleOrDefault(key)
: default;
}
}
30 changes: 15 additions & 15 deletions Orm/Xtensive.Orm/Orm/Internals/FieldAccessors/EnumFieldAccessor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
// Created by: Alexey Gamzov
// Created: 2008.06.07

using System;
using Xtensive.Orm.Model;
using Xtensive.Reflection;
using Xtensive.Tuples;

Expand All @@ -18,6 +18,14 @@ internal sealed class EnumFieldAccessor<T> : FieldAccessor<T>
? (type.IsNullable() ? null : Enum.GetValues(type).GetValue(0))
: default(T);

private Type columnValueType;

public override void SetFieldInfo(FieldInfo value)
{
columnValueType = value.Column.ValueType;
base.SetFieldInfo(value);
}

/// <inheritdoc/>
public override bool AreSameValues(object oldValue, object newValue)
{
Expand All @@ -27,25 +35,17 @@ public override bool AreSameValues(object oldValue, object newValue)
/// <inheritdoc/>
public override T GetValue(Persistent obj)
{
var field = Field;
int fieldIndex = field.MappingInfo.Offset;
var tuple = obj.Tuple;

TupleFieldState state;
var value = tuple.GetValue(fieldIndex, out state);
if (!state.HasValue())
return (T) @default;
if (type.IsEnum)
return (T) Enum.ToObject(type, value);
return (T) Enum.ToObject(Nullable.GetUnderlyingType(type), value);
var value = obj.Tuple.GetValue(FieldIndex, out var state);
return (T) (!state.HasValue() ? @default
: type.IsEnum ? Enum.ToObject(type, value)
: Enum.ToObject(Nullable.GetUnderlyingType(type), value));
}

/// <inheritdoc/>
public override void SetValue(Persistent obj, T value)
{
var field = Field;
// Biconverter<object, T> converter = GetConverter(field.ValueType);
obj.Tuple.SetValue(field.MappingInfo.Offset, value==null ? null : Convert.ChangeType(value, field.Column.ValueType));
obj.Tuple.SetValue(FieldIndex, value==null ? null : Convert.ChangeType(value, columnValueType));
}
}
}
}
48 changes: 17 additions & 31 deletions Orm/Xtensive.Orm/Orm/Internals/FieldAccessors/KeyFieldAccessor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,41 +4,27 @@
// Created by: Alex Yakunin
// Created: 2008.11.21

using System;
using Xtensive.Tuples;
using Tuple = Xtensive.Tuples.Tuple;

namespace Xtensive.Orm.Internals.FieldAccessors
{
internal class KeyFieldAccessor<T> : FieldAccessor<T>
{
private static readonly T @default = default;
namespace Xtensive.Orm.Internals.FieldAccessors;

/// <inheritdoc/>
public override bool AreSameValues(object oldValue, object newValue)
{
return object.Equals(oldValue, newValue);
}
internal class KeyFieldAccessor<T> : FieldAccessor<T>
{
/// <inheritdoc/>
public override bool AreSameValues(object oldValue, object newValue) => Equals(oldValue, newValue);

/// <inheritdoc/>
public override T GetValue(Persistent obj)
{
var field = Field;
int fieldIndex = field.MappingInfo.Offset;
var tuple = obj.Tuple;
TupleFieldState state;
var value = tuple.GetValue<string>(fieldIndex, out state);
if (!state.IsAvailable())
return @default;
return (T) (object) Key.Parse(obj.Session.Domain, value);
}
/// <inheritdoc/>
public override T GetValue(Persistent obj)
{
var value = obj.Tuple.GetValue<string>(FieldIndex, out var state);
return !state.IsAvailable()
? default
: (T) (object) Key.Parse(obj.Session.Domain, value);
}

/// <inheritdoc/>
public override void SetValue(Persistent obj, T value)
{
var field = Field;
var key = (Key) (object) value;
obj.Tuple.SetValue(field.MappingInfo.Offset, key is null ? null : key.Format());
}
/// <inheritdoc/>
public override void SetValue(Persistent obj, T value)
{
obj.Tuple.SetValue(FieldIndex, ((Key) (object) value)?.Format());
}
}
21 changes: 7 additions & 14 deletions Orm/Xtensive.Orm/Orm/Internals/FieldAccessors/StructureFieldAccessor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,35 +4,28 @@
// Created by: Dmitri Maximov
// Created: 2008.05.30

using System;
using Xtensive.Core;

using Xtensive.Tuples;

namespace Xtensive.Orm.Internals.FieldAccessors
{
internal sealed class StructureFieldAccessor<T> : CachingFieldAccessor<T>
{
/// <inheritdoc/>
public override bool AreSameValues(object oldValue, object newValue)
{
return oldValue.Equals(newValue);
}
public override bool AreSameValues(object oldValue, object newValue) => oldValue.Equals(newValue);

/// <inheritdoc/>
public override void SetValue(Persistent obj, T value)
{
var field = Field;
ArgumentNullException.ThrowIfNull(value);
var valueType = value.GetType();
if (field.ValueType != valueType)
if (Field.ValueType != valueType)
throw new InvalidOperationException(String.Format(
Strings.ExResultTypeIncorrect, valueType.Name, field.ValueType.Name));
Strings.ExResultTypeIncorrect, valueType.Name, Field.ValueType.Name));

var structure = (Structure) (object) value;
var adapter = (IFieldValueAdapter)value;
if (adapter.Owner!=null)
adapter.Owner.EnsureIsFetched(adapter.Field);
structure.Tuple.CopyTo(obj.Tuple, 0, field.MappingInfo.Offset, field.MappingInfo.Length);
adapter.Owner?.EnsureIsFetched(adapter.Field);
structure.Tuple.CopyTo(obj.Tuple, 0, FieldIndex, Field.MappingInfo.Length);
}

// Type initializer
Expand All @@ -42,4 +35,4 @@ static StructureFieldAccessor()
Constructor = (obj, field) => Activator.CreateStructure(field.ValueType, obj, field);
}
}
}
}
Loading
Loading