Skip to content

Commit b7983a0

Browse files
committed
DotvvmPropertyId: encode in ID if we can use direct dict access or virtual 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.
1 parent 4745066 commit b7983a0

File tree

5 files changed

+106
-30
lines changed

5 files changed

+106
-30
lines changed

src/Framework/Framework/Binding/DotvvmCapabilityProperty.CodeGeneration.cs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -116,11 +116,7 @@ public static (LambdaExpression getter, LambdaExpression setter) CreatePropertyA
116116

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

125121
var valueParameter = Expression.Parameter(type, "value");
126122
var unwrappedType = type.UnwrapNullableType();

src/Framework/Framework/Binding/DotvvmProperty.cs

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ public bool IsOwnedByCapability(DotvvmCapabilityProperty capability) =>
207207
{
208208
for (var p = control.Parent; p is not null; p = p.Parent)
209209
{
210-
if (p.properties.TryGet(this, out var v))
210+
if (p.properties.TryGet(Id, out var v))
211211
return v;
212212
}
213213
return DefaultValue;
@@ -218,7 +218,7 @@ public bool IsOwnedByCapability(DotvvmCapabilityProperty capability) =>
218218
/// </summary>
219219
public virtual object? GetValue(DotvvmBindableObject control, bool inherit = true)
220220
{
221-
if (control.properties.TryGet(this, out var value))
221+
if (control.properties.TryGet(Id, out var value))
222222
{
223223
return value;
224224
}
@@ -229,20 +229,29 @@ public bool IsOwnedByCapability(DotvvmCapabilityProperty capability) =>
229229
return DefaultValue;
230230
}
231231

232+
private bool IsSetInHierarchy(DotvvmBindableObject control)
233+
{
234+
for (var p = control.Parent; p is not null; p = p.Parent)
235+
{
236+
if (p.properties.Contains(Id))
237+
return true;
238+
}
239+
return false;
240+
}
232241

233242
/// <summary>
234243
/// Gets whether the value of the property is set
235244
/// </summary>
236245
public virtual bool IsSet(DotvvmBindableObject control, bool inherit = true)
237246
{
238-
if (control.properties.Contains(this))
247+
if (control.properties.Contains(Id))
239248
{
240249
return true;
241250
}
242251

243-
if (IsValueInherited && inherit && control.Parent != null)
252+
if (IsValueInherited && inherit)
244253
{
245-
return IsSet(control.Parent);
254+
return IsSetInHierarchy(control);
246255
}
247256

248257
return false;
@@ -254,7 +263,7 @@ public virtual bool IsSet(DotvvmBindableObject control, bool inherit = true)
254263
/// </summary>
255264
public virtual void SetValue(DotvvmBindableObject control, object? value)
256265
{
257-
control.properties.Set(this, value);
266+
control.properties.Set(Id, value);
258267
}
259268

260269
/// <summary>

src/Framework/Framework/Binding/DotvvmPropertyIdAssignment.cs

Lines changed: 88 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
using DotVVM.Framework.Compilation.ControlTree;
1010
using DotVVM.Framework.Controls;
1111
using DotVVM.Framework.Controls.Infrastructure;
12+
using FastExpressionCompiler;
1213

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

35+
/// <summary> Returns true if the property does not have GetValue/SetValue overrides and is not inherited. That means it is sufficient </summary>
36+
public bool CanUseFastAccessors
37+
{
38+
get
39+
{
40+
// properties: we encode this information as the LSB bit of the member ID (i.e. odd/even numbers)
41+
// property groups: always true, i.e.
42+
const uint mask = (1u << 31) | (1u);
43+
const uint targetValue = 1u;
44+
return (Id & mask) != targetValue;
45+
}
46+
}
47+
3448
public bool IsZero => Id == 0;
3549

3650
public DotvvmProperty PropertyInstance => DotvvmPropertyIdAssignment.GetProperty(Id) ?? throw new Exception($"Property with ID {Id} not registered.");
@@ -105,16 +119,16 @@ static DotvvmPropertyIdAssignment()
105119
#region Optimized metadata accessors
106120
public static bool IsInherited(DotvvmPropertyId propertyId)
107121
{
108-
if (propertyId.IsPropertyGroup)
122+
if (propertyId.CanUseFastAccessors)
109123
return false;
124+
110125
return BitmapRead(controls[propertyId.TypeId].inheritedBitmap, propertyId.MemberId);
111126
}
112127

113128
public static bool UsesStandardAccessors(DotvvmPropertyId propertyId)
114129
{
115-
if (propertyId.IsPropertyGroup)
130+
if (propertyId.CanUseFastAccessors)
116131
{
117-
// property groups can't override GetValue, otherwise VirtualPropertyGroupDictionary wouldn't work either
118132
return true;
119133
}
120134
else
@@ -191,17 +205,15 @@ public static bool IsActive(DotvvmPropertyId propertyId)
191205

192206
public static object? GetValueRaw(DotvvmBindableObject obj, DotvvmPropertyId id, bool inherit = true)
193207
{
194-
if (id.IsPropertyGroup)
208+
if (id.CanUseFastAccessors)
195209
{
196-
// property groups can't override GetValue
197210
if (obj.properties.TryGet(id, out var value))
198211
return value;
199212

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

315+
var typeCanUseDirectAccess = TypeCanUseAnyDirectAccess(property.GetType());
316+
var canUseDirectAccess = !property.IsValueInherited && typeCanUseDirectAccess;
317+
303318
var typeId = RegisterType(property.DeclaringType);
304319
ref ControlTypeInfo control = ref controls[typeId];
305-
lock (control.locker)
320+
lock (control.locker) // single control registrations are sequential anyway
306321
{
307-
var id = ++control.counter;
322+
uint id;
323+
if (canUseDirectAccess)
324+
{
325+
control.counterStandard += 1;
326+
id = control.counterStandard * 2;
327+
}
328+
else
329+
{
330+
control.counterNonStandard += 1;
331+
id = control.counterNonStandard * 2 + 1;
332+
}
308333
if (id > ushort.MaxValue)
309-
throw new Exception("Too many properties registered for a single control type.");
334+
ThrowTooManyException(property);
335+
336+
// resize arrays (we hold a write lock, but others may be reading in parallel)
310337
if (id >= control.properties.Length)
311338
{
312339
VolatileResize(ref control.properties, control.properties.Length * 2);
340+
}
341+
if (id / 64 >= control.inheritedBitmap.Length)
342+
{
343+
Debug.Assert(control.inheritedBitmap.Length == control.standardBitmap.Length);
344+
Debug.Assert(control.inheritedBitmap.Length == control.activeBitmap.Length);
345+
313346
VolatileResize(ref control.inheritedBitmap, control.inheritedBitmap.Length * 2);
314347
VolatileResize(ref control.standardBitmap, control.standardBitmap.Length * 2);
315348
VolatileResize(ref control.activeBitmap, control.activeBitmap.Length * 2);
316349
}
317350

318351
if (property.IsValueInherited)
319352
BitmapSet(control.inheritedBitmap, (uint)id);
320-
if (property.GetType() == typeof(DotvvmProperty))
353+
if (typeCanUseDirectAccess)
321354
BitmapSet(control.standardBitmap, (uint)id);
322355
if (property is ActiveDotvvmProperty)
323356
BitmapSet(control.activeBitmap, (uint)id);
324357

325358
control.properties[id] = property;
326359
return new DotvvmPropertyId(typeId, (ushort)id);
327360
}
361+
362+
static void ThrowTooManyException(DotvvmProperty property) =>
363+
throw new Exception($"Too many properties registered for a single control type ({property.DeclaringType.ToCode()}). Trying to register property '{property.Name}: {property.PropertyType.ToCode()}'");
364+
}
365+
366+
private static readonly ConcurrentDictionary<Type, (bool getter, bool setter)> cacheTypeCanUseDirectAccess = new(concurrencyLevel: 1, capacity: 10);
367+
/// <summary> Does the property use the default GetValue/SetValue methods? </summary>
368+
public static (bool getter, bool setter) TypeCanUseDirectAccess(Type propertyType)
369+
{
370+
if (propertyType == typeof(DotvvmProperty) || propertyType == typeof(GroupedDotvvmProperty))
371+
return (true, true);
372+
373+
if (propertyType == typeof(DotvvmCapabilityProperty) || propertyType == typeof(DotvvmPropertyAlias) || propertyType == typeof(CompileTimeOnlyDotvvmProperty))
374+
return (false, false);
375+
376+
if (propertyType.IsGenericType)
377+
{
378+
propertyType = propertyType.GetGenericTypeDefinition();
379+
if (propertyType == typeof(DelegateActionProperty<>))
380+
return (true, true);
381+
}
382+
383+
return cacheTypeCanUseDirectAccess.GetOrAdd(propertyType, static t =>
384+
{
385+
var getter = t.GetMethod(nameof(DotvvmProperty.GetValue), new [] { typeof(DotvvmBindableObject), typeof(bool) })!.DeclaringType == typeof(DotvvmProperty);
386+
var setter = t.GetMethod(nameof(DotvvmProperty.SetValue), new [] { typeof(DotvvmBindableObject), typeof(object) })!.DeclaringType == typeof(DotvvmProperty);
387+
return (getter, setter);
388+
});
389+
}
390+
public static bool TypeCanUseAnyDirectAccess(Type propertyType)
391+
{
392+
var (getter, setter) = TypeCanUseDirectAccess(propertyType);
393+
return getter && setter;
328394
}
329395

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

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

424490
private struct ControlTypeInfo
425491
{
426-
public object locker;
427492
public DotvvmProperty?[] properties;
428-
public Type controlType;
429493
public ulong[] inheritedBitmap;
430494
public ulong[] standardBitmap;
431495
public ulong[] activeBitmap;
432-
public int counter;
496+
public object locker;
497+
public Type controlType;
498+
public uint counterStandard;
499+
public uint counterNonStandard;
433500
}
434501

435502
public static class GroupMembers
@@ -461,5 +528,9 @@ public static class TypeIds
461528
// public const short Internal = 4;
462529
}
463530

531+
public static class PropertyIds
532+
{
533+
public const uint DotvvmBindableObject_DataContext = TypeIds.DotvvmBindableObject << 16 | 1;
534+
}
464535
}
465536
}

src/Framework/Framework/Controls/DotvvmControlProperties.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ public readonly bool TryGet(DotvvmPropertyId p, out object? value)
180180
if (keys != null)
181181
{
182182
Debug.Assert(values is object[]);
183-
Debug.Assert(keys is DotvvmPropertyId[]);
183+
Debug.Assert(keys is not null);
184184
var index = PropertyImmutableHashtable.FindSlot(this.keys, this.hashSeed, p);
185185
if (index >= 0)
186186
{

src/Framework/Framework/DependencyInjection/DotVVMServiceCollectionExtensions.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ public static IServiceCollection RegisterDotVVMServices(IServiceCollection servi
120120
var requiredResourceControl = controlResolver.ResolveControl(new ResolvedTypeDescriptor(typeof(RequiredResource)));
121121
o.TreeVisitors.Add(() => new StyleTreeShufflingVisitor(controlResolver));
122122
o.TreeVisitors.Add(() => new ControlPrecompilationVisitor(s));
123-
o.TreeVisitors.Add(() => new LiteralOptimizationVisitor());
123+
// o.TreeVisitors.Add(() => new LiteralOptimizationVisitor());
124124
o.TreeVisitors.Add(() => new BindingRequiredResourceVisitor((ControlResolverMetadata)requiredResourceControl));
125125
var requiredGlobalizeControl = controlResolver.ResolveControl(new ResolvedTypeDescriptor(typeof(GlobalizeResource)));
126126
o.TreeVisitors.Add(() => new GlobalizeResourceVisitor((ControlResolverMetadata)requiredGlobalizeControl));

0 commit comments

Comments
 (0)