diff --git a/src/Framework/Framework/Controls/DotvvmControlProperties.cs b/src/Framework/Framework/Controls/DotvvmControlProperties.cs index 81db78d3af..bf874241cc 100644 --- a/src/Framework/Framework/Controls/DotvvmControlProperties.cs +++ b/src/Framework/Framework/Controls/DotvvmControlProperties.cs @@ -45,12 +45,12 @@ private uint hashSeed } private bool ownsKeys { - readonly get => (flags >> 30) != 0; + readonly get => ((flags >> 30) & 1) != 0; set => flags = (flags & ~(1u << 30)) | ((uint)BoolToInt(value) << 30); } private bool ownsValues { - readonly get => (flags >> 31) != 0; + readonly get => ((flags >> 31) & 1) != 0; set => flags = (flags & ~(1u << 31)) | ((uint)BoolToInt(value) << 31); } @@ -260,6 +260,7 @@ public void Set(DotvvmPropertyId p, object? value) else { Debug.Assert(values is Dictionary); + OwnValues(); valuesAsDictionary[p] = value; } } @@ -320,10 +321,12 @@ public bool TryAdd(DotvvmPropertyId p, object? value) return Object.ReferenceEquals(existingValue, value); else { + OwnValues(); valuesAsDictionary.Add(p, value); return true; } #else + OwnValues(); return valuesAsDictionary.TryAdd(p, value) || Object.ReferenceEquals(valuesAsDictionary[p], value); #endif } @@ -408,23 +411,40 @@ internal void CloneInto(ref DotvvmControlProperties newDict) else if (this.keys == null) { var dictionary = this.valuesAsDictionary; - if (dictionary.Count > 30) + if (dictionary.Count > 8) { newDict = this; newDict.keys = null; - newDict.valuesAsDictionary = new Dictionary(dictionary); + Dictionary? newValues = null; foreach (var (key, value) in dictionary) if (CloneValue(value) is {} newValue) + { + if (newValues is null) + // ok, we have to copy it + newValues = new Dictionary(dictionary); newDict.valuesAsDictionary[key] = newValue; + } + + if (newValues is null) + { + newDict.valuesAsDictionary = dictionary; + newDict.ownsValues = false; + this.ownsValues = false; + } + else + { + newDict.valuesAsDictionary = newValues; + newDict.ownsValues = true; + } return; } - // move to immutable version if it's reasonably small. It will be probably cloned multiple times again + // move to immutable version if it's small. It will be probably cloned multiple times again SwitchToPerfectHashing(); } newDict = this; newDict.ownsKeys = false; - newDict.ownsValues = false; + this.ownsKeys = false; for (int i = 0; i < newDict.valuesAsArray.Length; i++) { if (CloneValue(newDict.valuesAsArray[i]) is {} newValue) @@ -439,6 +459,12 @@ internal void CloneInto(ref DotvvmControlProperties newDict) newDict.valuesAsArray[i] = newValue; } } + + if (newDict.values == this.values) + { + this.ownsValues = false; + newDict.ownsValues = false; + } } [MethodImpl(MethodImplOptions.AggressiveInlining)] diff --git a/src/Framework/Framework/DependencyInjection/DotVVMServiceCollectionExtensions.cs b/src/Framework/Framework/DependencyInjection/DotVVMServiceCollectionExtensions.cs index 4c2ef36466..a446f281e3 100644 --- a/src/Framework/Framework/DependencyInjection/DotVVMServiceCollectionExtensions.cs +++ b/src/Framework/Framework/DependencyInjection/DotVVMServiceCollectionExtensions.cs @@ -120,7 +120,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)); diff --git a/src/Tests/Runtime/DotvvmPropertyTests.cs b/src/Tests/Runtime/DotvvmPropertyTests.cs index 3f26b2a6c5..9e15c03556 100644 --- a/src/Tests/Runtime/DotvvmPropertyTests.cs +++ b/src/Tests/Runtime/DotvvmPropertyTests.cs @@ -303,6 +303,8 @@ public void DotvvmProperty_CorrectGetAndSet() foreach (var example in GetExampleValues(p.PropertyType)) { instance.SetValue(p, example); + Assert.AreEqual(example, instance.GetValue(p), $"GetValue is behaving weird {p}"); + Assert.AreEqual(example, instance.GetValueRaw(p.Id), $"GetValue(id) is behaving weird {p}"); Assert.AreEqual(example, p.PropertyInfo.GetValue(instance), $"Getter is broken in {p}"); } @@ -371,5 +373,58 @@ public void DotvvmProperty_ManyItemsSetter() Assert.AreEqual(1000, control1.Properties.Count); } + + [TestMethod] + [DataRow(false, false)] + [DataRow(false, true)] + [DataRow(true, false)] + [DataRow(true, true)] + public void DotvvmProperty_ControlClone(bool manyAttributes, bool nestedControl) + { + var control = new HtmlGenericControl("div"); + control.CssStyles.Set("color", "red"); + control.Attributes.Set("something", "value"); + + if (manyAttributes) + for (int i = 0; i < 60; i++) + control.Attributes.Set("data-" + i.ToString(), i.ToString()); + + if (nestedControl) + { + control.Properties.Add(Styles.ReplaceWithProperty, new HtmlGenericControl("span") { InnerText = "23" }); + } + + var clone = (HtmlGenericControl)control.CloneControl(); + + Assert.AreEqual(control.TagName, clone.TagName); + Assert.AreEqual(control.CssStyles["color"], "red"); + + // change original + Assert.IsFalse(clone.CssStyles.ContainsKey("abc")); + control.CssStyles.Set("color", "blue"); + control.CssStyles.Set("abc", "1"); + Assert.AreEqual("red", clone.CssStyles["color"]); + Assert.IsFalse(clone.CssStyles.ContainsKey("abc")); + + if (nestedControl) + { + var nestedClone = (HtmlGenericControl)clone.Properties[Styles.ReplaceWithProperty]!; + var nestedOriginal = (HtmlGenericControl)control.Properties[Styles.ReplaceWithProperty]!; + Assert.AreEqual("23", nestedClone.InnerText); + // change clone this time + nestedClone.InnerText = "24"; + Assert.AreEqual("23", nestedOriginal.InnerText); + Assert.AreEqual("24", nestedClone.InnerText); + } + + if (manyAttributes) + { + for (int i = 0; i < 60; i++) + { + Assert.AreEqual(i.ToString(), control.Attributes["data-" + i.ToString()]); + Assert.AreEqual(i.ToString(), clone.Attributes["data-" + i.ToString()]); + } + } + } } }