Skip to content

Commit

Permalink
DotvvmPropertyId: encode in ID if we can use direct dict access or vi…
Browse files Browse the repository at this point in the history 
…rtual GetValue

This is now frequently needed when working only
with the ID instead of working with DotvvmProperty.
Without this information, we pay not only for virtual dispatch,
but also the (more expensive) cost of looking up the
DotvvmProperty instance.

Since most properties use plain DotvvmProperty
and don't need the polymorphism, we now avoid
this cost by using the last bit of the property ID
to indicate whether the property can use direct access.
  • Loading branch information
@exyi
exyi committed Dec 8, 2024
1 parent 594cd63 commit 212c13c
Show file tree
Hide file tree
Showing 5 changed files with 106 additions and 30 deletions.
6 changes: 1 addition & 5 deletions src/Framework/Framework/Binding/DotvvmCapabilityProperty.CodeGeneration.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,11 +116,7 @@ public static (LambdaExpression getter, LambdaExpression setter) CreatePropertyA

// if the property does not override GetValue/SetValue, we'll use
// control.properties dictionary directly to avoid virtual method calls
var canUseDirectAccess =
!property.IsValueInherited && (
property.GetType() == typeof(DotvvmProperty) ||
property.GetType().GetMethod(nameof(DotvvmProperty.GetValue), new [] { typeof(DotvvmBindableObject), typeof(bool) })!.DeclaringType == typeof(DotvvmProperty) &&
property.GetType().GetMethod(nameof(DotvvmProperty.SetValue), new [] { typeof(DotvvmBindableObject), typeof(object) })!.DeclaringType == typeof(DotvvmProperty));
var canUseDirectAccess = !property.IsValueInherited && DotvvmPropertyIdAssignment.TypeCanUseAnyDirectAccess(property.GetType());

var valueParameter = Expression.Parameter(type, "value");
var unwrappedType = type.UnwrapNullableType();
Expand Down
21 changes: 15 additions & 6 deletions src/Framework/Framework/Binding/DotvvmProperty.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ public bool IsOwnedByCapability(DotvvmCapabilityProperty capability) =>
{
for (var p = control.Parent; p is not null; p = p.Parent)
{
if (p.properties.TryGet(this, out var v))
if (p.properties.TryGet(Id, out var v))
return v;
}
return DefaultValue;
Expand All @@ -222,7 +222,7 @@ public bool IsOwnedByCapability(DotvvmCapabilityProperty capability) =>
/// </summary>
public virtual object? GetValue(DotvvmBindableObject control, bool inherit = true)
{
if (control.properties.TryGet(this, out var value))
if (control.properties.TryGet(Id, out var value))
{
return value;
}
Expand All @@ -233,20 +233,29 @@ public bool IsOwnedByCapability(DotvvmCapabilityProperty capability) =>
return DefaultValue;
}

private bool IsSetInHierarchy(DotvvmBindableObject control)
{
for (var p = control.Parent; p is not null; p = p.Parent)
{
if (p.properties.Contains(Id))
return true;
}
return false;
}

/// <summary>
/// Gets whether the value of the property is set
/// </summary>
public virtual bool IsSet(DotvvmBindableObject control, bool inherit = true)
{
if (control.properties.Contains(this))
if (control.properties.Contains(Id))
{
return true;
}

if (IsValueInherited && inherit && control.Parent != null)
if (IsValueInherited && inherit)
{
return IsSet(control.Parent);
return IsSetInHierarchy(control);
}

return false;
Expand All @@ -258,7 +267,7 @@ public virtual bool IsSet(DotvvmBindableObject control, bool inherit = true)
/// </summary>
public virtual void SetValue(DotvvmBindableObject control, object? value)
{
control.properties.Set(this, value);
control.properties.Set(Id, value);
}

/// <summary>
Expand Down
105 changes: 88 additions & 17 deletions src/Framework/Framework/Binding/DotvvmPropertyIdAssignment.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using DotVVM.Framework.Compilation.ControlTree;
using DotVVM.Framework.Controls;
using DotVVM.Framework.Controls.Infrastructure;
using FastExpressionCompiler;

namespace DotVVM.Framework.Binding
{
Expand All @@ -31,6 +32,19 @@ public DotvvmPropertyId(ushort typeOrGroupId, ushort memberId)
public ushort GroupId => (ushort)((Id >> 16) ^ 0x80_00);
public ushort MemberId => (ushort)(Id & 0xFFFF);

/// <summary> Returns true if the property does not have GetValue/SetValue overrides and is not inherited. That means it is sufficient </summary>
public bool CanUseFastAccessors
{
get
{
// properties: we encode this information as the LSB bit of the member ID (i.e. odd/even numbers)
// property groups: always true, i.e.
const uint mask = (1u << 31) | (1u);
const uint targetValue = 1u;
return (Id & mask) != targetValue;
}
}

public bool IsZero => Id == 0;

public DotvvmProperty PropertyInstance => DotvvmPropertyIdAssignment.GetProperty(Id) ?? throw new Exception($"Property with ID {Id} not registered.");
Expand Down Expand Up @@ -105,16 +119,16 @@ static DotvvmPropertyIdAssignment()
#region Optimized metadata accessors
public static bool IsInherited(DotvvmPropertyId propertyId)
{
if (propertyId.IsPropertyGroup)
if (propertyId.CanUseFastAccessors)
return false;

return BitmapRead(controls[propertyId.TypeId].inheritedBitmap, propertyId.MemberId);
}

public static bool UsesStandardAccessors(DotvvmPropertyId propertyId)
{
if (propertyId.IsPropertyGroup)
if (propertyId.CanUseFastAccessors)
{
// property groups can't override GetValue, otherwise VirtualPropertyGroupDictionary wouldn't work either
return true;
}
else
Expand Down Expand Up @@ -191,17 +205,15 @@ public static bool IsActive(DotvvmPropertyId propertyId)

public static object? GetValueRaw(DotvvmBindableObject obj, DotvvmPropertyId id, bool inherit = true)
{
if (id.IsPropertyGroup)
if (id.CanUseFastAccessors)
{
// property groups can't override GetValue
if (obj.properties.TryGet(id, out var value))
return value;

return propertyGroups[id.GroupId]!.DefaultValue;
}
else
{
// TODO: maybe try if using the std/inherit bitmaps would be faster
var property = controls[id.TypeId].properties[id.MemberId];
return property!.GetValue(obj, inherit);
}
Expand Down Expand Up @@ -300,31 +312,85 @@ public static DotvvmPropertyId RegisterProperty(DotvvmProperty property)
if (property.GetType() == typeof(GroupedDotvvmProperty))
throw new ArgumentException("RegisterProperty cannot be called with GroupedDotvvmProperty!");

var typeCanUseDirectAccess = TypeCanUseAnyDirectAccess(property.GetType());
var canUseDirectAccess = !property.IsValueInherited && typeCanUseDirectAccess;

var typeId = RegisterType(property.DeclaringType);
ref ControlTypeInfo control = ref controls[typeId];
lock (control.locker)
lock (control.locker) // single control registrations are sequential anyway
{
var id = ++control.counter;
uint id;
if (canUseDirectAccess)
{
control.counterStandard += 1;
id = control.counterStandard * 2;
}
else
{
control.counterNonStandard += 1;
id = control.counterNonStandard * 2 + 1;
}
if (id > ushort.MaxValue)
throw new Exception("Too many properties registered for a single control type.");
ThrowTooManyException(property);

// resize arrays (we hold a write lock, but others may be reading in parallel)
if (id >= control.properties.Length)
{
VolatileResize(ref control.properties, control.properties.Length * 2);
}
if (id / 64 >= control.inheritedBitmap.Length)
{
Debug.Assert(control.inheritedBitmap.Length == control.standardBitmap.Length);
Debug.Assert(control.inheritedBitmap.Length == control.activeBitmap.Length);

VolatileResize(ref control.inheritedBitmap, control.inheritedBitmap.Length * 2);
VolatileResize(ref control.standardBitmap, control.standardBitmap.Length * 2);
VolatileResize(ref control.activeBitmap, control.activeBitmap.Length * 2);
}

if (property.IsValueInherited)
BitmapSet(control.inheritedBitmap, (uint)id);
if (property.GetType() == typeof(DotvvmProperty))
if (typeCanUseDirectAccess)
BitmapSet(control.standardBitmap, (uint)id);
if (property is ActiveDotvvmProperty)
BitmapSet(control.activeBitmap, (uint)id);

control.properties[id] = property;
return new DotvvmPropertyId(typeId, (ushort)id);
}

static void ThrowTooManyException(DotvvmProperty property) =>
throw new Exception($"Too many properties registered for a single control type ({property.DeclaringType.ToCode()}). Trying to register property '{property.Name}: {property.PropertyType.ToCode()}'");
}

private static readonly ConcurrentDictionary<Type, (bool getter, bool setter)> cacheTypeCanUseDirectAccess = new(concurrencyLevel: 1, capacity: 10);
/// <summary> Does the property use the default GetValue/SetValue methods? </summary>
public static (bool getter, bool setter) TypeCanUseDirectAccess(Type propertyType)
{
if (propertyType == typeof(DotvvmProperty) || propertyType == typeof(GroupedDotvvmProperty))
return (true, true);

if (propertyType == typeof(DotvvmCapabilityProperty) || propertyType == typeof(DotvvmPropertyAlias) || propertyType == typeof(CompileTimeOnlyDotvvmProperty))
return (false, false);

if (propertyType.IsGenericType)
{
propertyType = propertyType.GetGenericTypeDefinition();
if (propertyType == typeof(DelegateActionProperty<>))
return (true, true);
}

return cacheTypeCanUseDirectAccess.GetOrAdd(propertyType, static t =>
{
var getter = t.GetMethod(nameof(DotvvmProperty.GetValue), new [] { typeof(DotvvmBindableObject), typeof(bool) })!.DeclaringType == typeof(DotvvmProperty);
var setter = t.GetMethod(nameof(DotvvmProperty.SetValue), new [] { typeof(DotvvmBindableObject), typeof(object) })!.DeclaringType == typeof(DotvvmProperty);
return (getter, setter);
});
}
public static bool TypeCanUseAnyDirectAccess(Type propertyType)
{
var (getter, setter) = TypeCanUseDirectAccess(propertyType);
return getter && setter;
}

public static ushort RegisterPropertyGroup(DotvvmPropertyGroup group)
Expand All @@ -351,10 +417,10 @@ public static ushort RegisterPropertyGroup(DotvvmPropertyGroup group)
/// <summary> Thread-safe to read from the array while we are resizing </summary>
private static void VolatileResize<T>(ref T[] array, int newSize)
{
var local = array;
Array.Resize(ref local, newSize);
Thread.MemoryBarrier(); // prevent reordering of the array assignment and array contents copy on weakly-ordered platforms
array = local;
var localRef = array;
var newArray = new T[newSize];
localRef.AsSpan().CopyTo(newArray.AsSpan(0, localRef.Length));
Volatile.Write(ref array, newArray);
}

#endregion Registration
Expand Down Expand Up @@ -423,13 +489,14 @@ static void BitmapSet(ulong[] bitmap, uint index)

private struct ControlTypeInfo
{
public object locker;
public DotvvmProperty?[] properties;
public Type controlType;
public ulong[] inheritedBitmap;
public ulong[] standardBitmap;
public ulong[] activeBitmap;
public int counter;
public object locker;
public Type controlType;
public uint counterStandard;
public uint counterNonStandard;
}

public static class GroupMembers
Expand Down Expand Up @@ -461,5 +528,9 @@ public static class TypeIds
// public const short Internal = 4;
}

public static class PropertyIds
{
public const uint DotvvmBindableObject_DataContext = TypeIds.DotvvmBindableObject << 16 | 1;
}
}
}
2 changes: 1 addition & 1 deletion src/Framework/Framework/Controls/DotvvmControlProperties.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ public readonly bool TryGet(DotvvmPropertyId p, out object? value)
if (keys != null)
{
Debug.Assert(values is object[]);
Debug.Assert(keys is DotvvmPropertyId[]);
Debug.Assert(keys is not null);
var index = PropertyImmutableHashtable.FindSlot(this.keys, this.hashSeed, p);
if (index >= 0)
{
Expand Down
2 changes: 1 addition & 1 deletion src/Framework/Framework/DependencyInjection/DotVVMServiceCollectionExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ public static IServiceCollection RegisterDotVVMServices(IServiceCollection servi
var requiredResourceControl = controlResolver.ResolveControl(new ResolvedTypeDescriptor(typeof(RequiredResource)));
o.TreeVisitors.Add(() => new StyleTreeShufflingVisitor(controlResolver));
o.TreeVisitors.Add(() => new ControlPrecompilationVisitor(s));
o.TreeVisitors.Add(() => new LiteralOptimizationVisitor());
// o.TreeVisitors.Add(() => new LiteralOptimizationVisitor());
o.TreeVisitors.Add(() => new BindingRequiredResourceVisitor((ControlResolverMetadata)requiredResourceControl));
var requiredGlobalizeControl = controlResolver.ResolveControl(new ResolvedTypeDescriptor(typeof(GlobalizeResource)));
o.TreeVisitors.Add(() => new GlobalizeResourceVisitor((ControlResolverMetadata)requiredGlobalizeControl));
Expand Down

0 comments on commit 212c13c

Please sign in to comment.