Skip to content

Commit

Permalink
Merge pull request #1851 from riganti/perf-htmlwriter-indexof
Browse files Browse the repository at this point in the history 
Improve performance of HtmlWriter and some smaller things
  • Loading branch information
@tomasherceg
tomasherceg authored Sep 7, 2024
2 parents b22dab3 + 5ed77a9 commit c0a911e
Show file tree
Hide file tree
Showing 6 changed files with 180 additions and 108 deletions.
6 changes: 3 additions & 3 deletions src/Framework/Framework/Binding/BindingHelper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,11 @@ public static string FormatKnockoutScript(this ParametrizedCode code, DotvvmBind
/// Gets Internal.PathFragmentProperty or DataContext.KnockoutExpression. Returns null if none of these is set.
/// </summary>
public static string? GetDataContextPathFragment(this DotvvmBindableObject currentControl) =>
(string?)currentControl.GetValue(Internal.PathFragmentProperty, inherit: false) ??
(currentControl.GetBinding(DotvvmBindableObject.DataContextProperty, inherit: false) is IValueBinding binding ?
currentControl.properties.TryGet(Internal.PathFragmentProperty, out var pathFragment) && pathFragment is string pathFragmentStr ? pathFragmentStr :
currentControl.properties.TryGet(DotvvmBindableObject.DataContextProperty, out var dataContext) && dataContext is IValueBinding binding ?
binding.GetProperty<SimplePathExpressionBindingProperty>()
.Code.FormatKnockoutScript(currentControl, binding) :
null);
null;


// PERF: maybe safe last GetValue's target/binding to ThreadLocal variable, so the path does not have to be traversed twice
Expand Down
61 changes: 46 additions & 15 deletions src/Framework/Framework/Compilation/Javascript/JavascriptTranslator.cs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System;
using System;
using System.Collections;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
Expand Down Expand Up @@ -146,12 +146,15 @@ public JsExpression CompileToJavascript(Expression binding, DataContextStack dat
public static ParametrizedCode AdjustKnockoutScriptContext(ParametrizedCode expression, int dataContextLevel)
{
if (dataContextLevel == 0) return expression;
return expression.AssignParameters(o =>
o is ViewModelSymbolicParameter vm ? GetKnockoutViewModelParameter(vm.ParentIndex + dataContextLevel).ToParametrizedCode() :
o is ContextSymbolicParameter context ? GetKnockoutContextParameter(context.ParentIndex + dataContextLevel).ToParametrizedCode() :
o == CommandBindingExpression.OptionalKnockoutContextParameter ? GetKnockoutContextParameter(dataContextLevel).ToParametrizedCode() :
default
);

// separate method to avoid closure allocation if dataContextLevel == 0
return shift(expression, dataContextLevel);
static ParametrizedCode shift(ParametrizedCode expression, int dataContextLevel) =>
expression.AssignParameters(o =>
o is ViewModelSymbolicParameter vm ? GetKnockoutViewModelParameter(vm.ParentIndex + dataContextLevel, vm.ReturnObservable).ToParametrizedCode() :
o is ContextSymbolicParameter context ? GetKnockoutContextParameter(context.ParentIndex + dataContextLevel).ToParametrizedCode() :
o == CommandBindingExpression.OptionalKnockoutContextParameter ? GetKnockoutContextParameter(dataContextLevel).ToParametrizedCode() :
default);
}

/// <summary>
Expand All @@ -164,14 +167,42 @@ public static string FormatKnockoutScript(JsExpression expression, bool allowDat
/// </summary>
public static string FormatKnockoutScript(ParametrizedCode expression, bool allowDataGlobal = true, int dataContextLevel = 0)
{
// TODO(exyi): more symbolic parameters
var adjusted = AdjustKnockoutScriptContext(expression, dataContextLevel);
if (allowDataGlobal)
return adjusted.ToDefaultString();
else
return adjusted.ToString(o =>
o == KnockoutViewModelParameter ? CodeParameterAssignment.FromIdentifier("$data") :
default);
if (dataContextLevel == 0)
{
if (allowDataGlobal)
return expression.ToDefaultString();
else
return expression.ToString(static o =>
o == KnockoutViewModelParameter ? CodeParameterAssignment.FromIdentifier("$data") :
default);

}

// separate method to avoid closure allocation if dataContextLevel == 0
return shiftToString(expression, dataContextLevel);

static string shiftToString(ParametrizedCode expression, int dataContextLevel) =>
expression.ToString(o => {
if (o is ViewModelSymbolicParameter vm)
{
var p = GetKnockoutViewModelParameter(vm.ParentIndex + dataContextLevel, vm.ReturnObservable).DefaultAssignment;
return new(p.Code!.ToDefaultString(), p.Code.OperatorPrecedence);
}
else if (o is ContextSymbolicParameter context)
{
var p = GetKnockoutContextParameter(context.ParentIndex + dataContextLevel).DefaultAssignment;
return new(p.Code!.ToDefaultString(), p.Code.OperatorPrecedence);
}
else if (o == CommandBindingExpression.OptionalKnockoutContextParameter)
{
var p = GetKnockoutContextParameter(dataContextLevel).DefaultAssignment;
return new(p.Code!.ToDefaultString(), p.Code.OperatorPrecedence);
}
else
{
return default;
}
});
}

/// <summary>
Expand Down
29 changes: 17 additions & 12 deletions src/Framework/Framework/Compilation/Javascript/ParametrizedCode.cs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System;
using System;
using System.Linq;
using System.Collections.Generic;
using System.Text;
Expand Down Expand Up @@ -64,26 +64,31 @@ public string ToString(Func<CodeSymbolicParameter, CodeParameterAssignment> para
if (allIsDefault && this.evaluatedDefault != null)
return evaluatedDefault;

var sb = new StringBuilder(codes.Sum((p) => p.code.Length) + stringParts.Sum(p => p.Length));
var capacity = 0;
foreach (var c in codes) capacity += c.code.Length;
foreach (var s in stringParts) capacity += s.Length;
var sb = new StringBuilder(capacity);

sb.Append(stringParts[0]);
for (int i = 0; i < codes.Length;)
{
var isGlobalContext = codes[i].parameter.IsGlobalContext && parameters![i].IsSafeMemberAccess;
var needsParens = codes[i].parameter.Code!.OperatorPrecedence.NeedsParens(parameters![i].OperatorPrecedence);
var code = codes[i];
var isGlobalContext = code.parameter.IsGlobalContext && parameters![i].IsSafeMemberAccess;
var needsParens = code.parameter.Code!.OperatorPrecedence.NeedsParens(parameters![i].OperatorPrecedence);

if (isGlobalContext)
sb.Append(stringParts[++i], 1, stringParts[i].Length - 1); // skip `.`
sb.Append(stringParts[++i], startIndex: 1, count: stringParts[i].Length - 1); // skip `.`
else
{
if (needsParens)
sb.Append("(");
else if (JsFormattingVisitor.NeedSpaceBetween(sb, codes[i].code))
sb.Append(" ");
sb.Append(codes[i].code);
sb.Append('(');
else if (JsFormattingVisitor.NeedSpaceBetween(sb, code.code))
sb.Append(' ');
sb.Append(code.code);
i++;
if (needsParens) sb.Append(")");
if (needsParens) sb.Append(')');
else if (JsFormattingVisitor.NeedSpaceBetween(sb, stringParts[i]))
sb.Append(" ");
sb.Append(' ');
sb.Append(stringParts[i]);
}
}
Expand Down Expand Up @@ -177,7 +182,7 @@ public void CopyTo(Builder builder)
builder.Add(stringParts[i]);
builder.Add(parameters[i]);
}
builder.Add(stringParts.Last());
builder.Add(stringParts[stringParts.Length - 1]);
}
}

Expand Down
19 changes: 16 additions & 3 deletions src/Framework/Framework/Controls/DotvvmBindableObject.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -274,14 +274,27 @@ internal IEnumerable<IValueBinding> GetDataContextHierarchy()
/// <summary>
/// Gets the closest control binding target. Returns null if the control is not found.
/// </summary>
public DotvvmBindableObject? GetClosestControlBindingTarget() =>
GetClosestControlBindingTarget(out int numberOfDataContextChanges);
public DotvvmBindableObject? GetClosestControlBindingTarget()
{
var c = this;
while (c != null)
{
if (c.properties.TryGet(Internal.IsControlBindingTargetProperty, out var x) && (bool)x!)
{
return c;
}
c = c.Parent;
}
return null;
}

/// <summary>
/// Gets the closest control binding target and returns number of DataContext changes since the target. Returns null if the control is not found.
/// </summary>
public DotvvmBindableObject? GetClosestControlBindingTarget(out int numberOfDataContextChanges) =>
(Parent ?? this).GetClosestWithPropertyValue(out numberOfDataContextChanges, (control, _) => (bool)control.GetValue(Internal.IsControlBindingTargetProperty)!);
(Parent ?? this).GetClosestWithPropertyValue(
out numberOfDataContextChanges,
(control, _) => control.properties.TryGet(Internal.IsControlBindingTargetProperty, out var x) && (bool)x!);

/// <summary>
/// Gets the closest control binding target and returns number of DataContext changes since the target. Returns null if the control is not found.
Expand Down
127 changes: 62 additions & 65 deletions src/Framework/Framework/Controls/HtmlWriter.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public class HtmlWriter : IHtmlWriter
private readonly bool debug;
private readonly bool enableWarnings;

private List<(string name, string? val, string? separator, bool allowAppending)> attributes = new List<(string, string?, string? separator, bool allowAppending)>();
private readonly List<(string name, string? val, string? separator, bool allowAppending)> attributes = new List<(string, string?, string? separator, bool allowAppending)>();
private DotvvmBindableObject? errorContext;
private OrderedDictionary dataBindAttributes = new OrderedDictionary();
private Stack<string> openTags = new Stack<string>();
Expand Down Expand Up @@ -435,7 +435,6 @@ private int CountQuotesAndApos(string value)
return result;
}


private void WriteEncodedText(string input, bool escapeQuotes, bool escapeApos)
{
int index = 0;
Expand All @@ -449,7 +448,6 @@ private void WriteEncodedText(string input, bool escapeQuotes, bool escapeApos)
writer.Write(input);
return;
}

#if NoSpan
writer.Write(input.Substring(startIndex));
#else
Expand All @@ -464,81 +462,80 @@ private void WriteEncodedText(string input, bool escapeQuotes, bool escapeApos)
#else
writer.Write(input.AsSpan().Slice(startIndex, index - startIndex));
#endif
switch (input[index])
{
case '<':
writer.Write("&lt;");
break;
case '>':
writer.Write("&gt;");
break;
case '"':
writer.Write("&quot;");
break;
case '\'':
writer.Write("&#39;");
break;
case '&':
writer.Write("&amp;");
break;
default:
throw new Exception("Should not happen.");
}
var encoding = EncodingTable[input[index] - 34];
Debug.Assert(encoding != null);
writer.Write(encoding);

index++;
if (index == input.Length)
return;
}
}
}

static string?[] EncodingTable = new List<string?>(Enumerable.Repeat((string?)null, 63 - 34)) {
[34 - 34] = "&quot;", // "
[39 - 34] = "&#39;", // '
[60 - 34] = "&lt;", // <
[62 - 34] = "&gt;", // >
[38 - 34] = "&amp;" // &
}.ToArray();
private static char[] MinimalEscapeChars = new char[] { '<', '>', '&' };
private static char[] DoubleQEscapeChars = new char[] { '<', '>', '&', '"' };
private static char[] SingleQEscapeChars = new char[] { '<', '>', '&', '\'' };
private static char[] BothQEscapeChars = new char[] { '<', '>', '&', '"', '\'' };

private static int IndexOfHtmlEncodingChars(string input, int startIndex, bool escapeQuotes, bool escapeApos)
{
for (int i = startIndex; i < input.Length; i++)
char[] breakChars;
if (escapeQuotes)
{
if (escapeApos)
breakChars = BothQEscapeChars;
else
breakChars = DoubleQEscapeChars;
}
else if (escapeApos)
{
breakChars = SingleQEscapeChars;
}
else
{
char ch = input[i];
if (ch <= '>')
breakChars = MinimalEscapeChars;
}

int i = startIndex;
while (true)
{
var foundIndex = MemoryExtensions.IndexOfAny(input.AsSpan(start: i), breakChars);
if (foundIndex < 0)
return -1;

i += foundIndex;

if (input[i] == '&' && i + 1 < input.Length)
{
switch (ch)
{
case '<':
case '>':
return i;
case '"':
if (escapeQuotes)
return i;
break;
case '\'':
if (escapeApos)
return i;
break;
case '&':
// HTML spec permits ampersands, if they are not ambiguous:

// An ambiguous ampersand is a U+0026 AMPERSAND character (&) that is followed by one or more ASCII alphanumerics, followed by a U+003B SEMICOLON character (;), where these characters do not match any of the names given in the named character references section.

// so if the next character is not alphanumeric, we can leave it there
if (i + 1 == input.Length)
return i;
var nextChar = input[i + 1];
if (IsInRange(nextChar, 'a', 'z') ||
IsInRange(nextChar, 'A', 'Z') ||
IsInRange(nextChar, '0', '9') ||
nextChar == '#')
return i;
break;
}
// HTML spec permits ampersands, if they are not ambiguous:
// (and unnecessarily quoting them makes JS less readable)

// An ambiguous ampersand is a U+0026 AMPERSAND character (&) that is followed by one or more ASCII alphanumerics, followed by a U+003B SEMICOLON character (;), where these characters do not match any of the names given in the named character references section.

// so if the next character is not alphanumeric, we can leave it there
var nextChar = input[i + 1];
if (IsInRange(nextChar, 'a', 'z') |
IsInRange(nextChar, 'A', 'Z') |
IsInRange(nextChar, '0', '9') |
nextChar == '#')
return i;
}
else if (char.IsSurrogate(ch))
else
{
// surrogates are fine, but they must not code for ASCII characters

var value = Char.ConvertToUtf32(ch, input[i + 1]);
if (value < 256)
throw new InvalidOperationException("Encountered UTF16 surrogate coding for ASCII char, this is not allowed.");

i++;
// all other characters are escaped unconditionaly
return i;
}

i++;
}

return -1;
}

private void ThrowIfAttributesArePresent([CallerMemberName] string operation = "Write")
Expand Down
Loading

0 comments on commit c0a911e

Please sign in to comment.