Skip to content
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.

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

Allow end users to observe JSX token's value #331

Merged
merged 1 commit into from
Aug 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 2 additions & 3 deletions src/Esprima/JsxParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -580,8 +580,7 @@ private JsxIdentifier ParseJsxIdentifier()
ThrowUnexpectedToken(token);
}

var holder = (JsxToken.JsxHolder) token.Value!;
return Finalize(node, new JsxIdentifier(holder.Value));
return Finalize(node, new JsxIdentifier((string) token.Value!));
}

private JsxExpression ParseJsxElementName()
Expand Down Expand Up @@ -805,7 +804,7 @@ private ArrayList<JsxExpression> ParseJsxChildren()
if (token.Start < token.End)
{
var raw = GetTokenRaw(token);
var child = Finalize(node, new JsxText(((JsxToken.JsxHolder) token.Value!).Value, raw));
var child = Finalize(node, new JsxText((string) token.Value!, raw));
children.Add(child);
}

Expand Down
4 changes: 2 additions & 2 deletions src/Esprima/JsxToken.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ public enum JsxTokenType

public static class JsxToken
{
internal sealed record JsxHolder(JsxTokenType Type, string Value);
private sealed record JsxHolder(JsxTokenType Type, object? Value) : Token.ValueHolder(Value);

[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal static Token CreateIdentifier(string value, int start, int end, int lineNumber, int lineStart)
Expand All @@ -26,5 +26,5 @@ internal static Token CreateText(string value, int start, int end, int lineNumbe
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static JsxTokenType JsxTokenType(this Token token) => token.Type == TokenType.Extension && token.Value is JsxHolder holder ? holder.Type : Esprima.JsxTokenType.Unknown;
public static JsxTokenType JsxTokenType(this in Token token) => token.Type == TokenType.Extension && token._value is JsxHolder holder ? holder.Type : Esprima.JsxTokenType.Unknown;
}
29 changes: 20 additions & 9 deletions src/Esprima/Token.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Numerics;
using System.Configuration;
using System.Numerics;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using System.Text.RegularExpressions;
Expand Down Expand Up @@ -27,7 +28,9 @@ public enum TokenType : byte
[StructLayout(LayoutKind.Auto)]
public readonly record struct Token
{
private readonly object? _value;
internal abstract record ValueHolder(object? Value);

internal readonly object? _value;

internal Token(
TokenType type,
Expand Down Expand Up @@ -60,7 +63,7 @@ internal static Token CreateStringLiteral(string str, bool octal, int start, int
return new Token(TokenType.StringLiteral, str, start, end, lineNumber, lineStart, octal);
}

private sealed record RegexHolder(Regex? Value, RegexValue RegexValue);
private sealed record RegexHolder(object? Value, RegexValue RegexValue) : ValueHolder(Value);

[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal static Token CreateRegexLiteral(Regex? value, RegexValue regexValue, int start, int end, int lineNumber, int lineStart)
Expand Down Expand Up @@ -90,7 +93,7 @@ internal static Token CreatePunctuator(string str, int start, int end, int lineN
return new Token(TokenType.Punctuator, str, start, end, lineNumber, lineStart);
}

private sealed record TemplateHolder(string? Cooked, string RawTemplate, char NotEscapeSequenceHead, bool Head, bool Tail);
private sealed record TemplateHolder(object? Value, string RawTemplate, char NotEscapeSequenceHead, bool Head, bool Tail) : ValueHolder(Value);

[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal static Token CreateTemplate(
Expand All @@ -116,12 +119,20 @@ internal static Token CreateTemplate(
public readonly int LineNumber;
public readonly int LineStart;

public object? Value => Type switch
public object? Value
{
TokenType.Template => ((TemplateHolder) _value!).Cooked,
TokenType.RegularExpression => ((RegexHolder) _value!).Value,
_ => _value
};
[MethodImpl(MethodImplOptions.AggressiveInlining)]
get
{
// NOTE: This condition must not be inverted, otherwise the runtime (.NET 6) fail to inline the accessor correctly.
return Type is not (TokenType.RegularExpression or TokenType.Template or TokenType.Extension)
? _value
: GetValueFromHolder(in this);

[MethodImpl(MethodImplOptions.NoInlining)]
static object? GetValueFromHolder(in Token token) => ((ValueHolder) token._value!).Value;
}
Comment on lines +124 to +134
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Finally I've managed to find a combination which the runtime can inline without penalty.

If you change anything in this code (even if just invert the condition), inlining goes south and the acceptance test starts to fail. Mind-boggling...

However, this looks to work reliably and according to the benchmarks, we even have a little bit of improvement in execution time:

Esprima.Benchmark.FileParsingBenchmark

Diff Method FileName Mean Error Gen 0 Gen 1 Gen 2 Allocated
Old ParseProgram angular-1.2.5 17.628 ms 2.5634 ms 718.7500 343.7500 - 4,315 KB
New 17.475 ms (-1%) 1.1170 ms 718.7500 (0%) 312.5000 (-9%) - 4,315 KB (0%)
Old ParseProgram backbone-1.1.0 2.408 ms 0.2234 ms 136.7188 39.0625 - 676 KB
New 2.338 ms (-3%) 0.2539 ms 140.6250 (+3%) 35.1563 (-10%) - 676 KB (0%)
Old ParseProgram jquery-1.9.1 14.161 ms 0.6347 ms 625.0000 296.8750 - 3,764 KB
New 13.805 ms (-3%) 1.0131 ms 609.3750 (-3%) 296.8750 (0%) - 3,764 KB (0%)
Old ParseProgram jquery.mobile-1.4.2 22.922 ms 0.3971 ms 1031.2500 500.0000 31.2500 5,798 KB
New 22.039 ms (-4%) 2.4387 ms 1031.2500 (0%) 500.0000 (0%) 31.2500 (0%) 5,798 KB (0%)
Old ParseProgram mootools-1.4.5 11.000 ms 0.1603 ms 500.0000 250.0000 - 3,091 KB
New 10.776 ms (-2%) 0.4187 ms 500.0000 (0%) 250.0000 (0%) - 3,091 KB (0%)
Old ParseProgram underscore-1.5.2 2.022 ms 0.0332 ms 117.1875 42.9688 - 571 KB
New 1.930 ms (-5%) 0.0355 ms 113.2813 (-3%) 42.9688 (0%) - 571 KB (0%)
Old ParseProgram yui-3.12.0 10.352 ms 1.6380 ms 468.7500 234.3750 - 2,856 KB
New 9.828 ms (-5%) 1.0396 ms 468.7500 (0%) 234.3750 (0%) - 2,856 KB (0%)

}

internal char NotEscapeSequenceHead => Type == TokenType.Template ? ((TemplateHolder) _value!).NotEscapeSequenceHead : char.MinValue;
public bool Head => Type == TokenType.Template && ((TemplateHolder) _value!).Head;
Expand Down