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

Use a Dictionary to optimize performance of CompiledTemplate.TryGetFormalArgument #122

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 17 additions & 3 deletions Antlr4.StringTemplate/Compiler/CompiledTemplate.cs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@ public class CompiledTemplate

private List<FormalArgument> _formalArguments;

private Dictionary<string, FormalArgument> _formalArguments2;

private bool _hasFormalArgs;

/** A list of all regions and subtemplates */
Expand Down Expand Up @@ -218,6 +220,7 @@ public CommonTree Ast
}
}

// TODO: Problem - List is externally exposed and writable without an event hook to know when it's edited to make the dictionary. Making it non-exposed or non-writable would be breaking changes
public List<FormalArgument> FormalArguments
{
get
Expand All @@ -229,6 +232,7 @@ public List<FormalArgument> FormalArguments
{
_formalArguments = value;
_numberOfArgsWithDefaultValues = (_formalArguments != null) ? _formalArguments.Count(i => i.DefaultValueToken != null) : 0;
_formalArguments2 = value?.ToDictionary(i => i.Name);
}
}

Expand Down Expand Up @@ -354,10 +358,15 @@ public virtual FormalArgument TryGetFormalArgument(string name)
{
if (name == null)
throw new ArgumentNullException("name");
if (FormalArguments == null)
if (_formalArguments2 == null)
return null;

return FormalArguments.FirstOrDefault(i => i.Name == name);
if (_formalArguments2.TryGetValue(name, out FormalArgument value))
{
return value;
}

return default(FormalArgument);
}

/// <summary>
Expand All @@ -367,13 +376,15 @@ public virtual FormalArgument TryGetFormalArgument(string name)
/// </summary>
/// <returns>
/// A copy of the current <see cref="CompiledTemplate"/> instance. The copy is a shallow copy, with the
/// exception of the <see cref="_formalArguments"/> field which is also cloned.
/// exception of the <see cref="_formalArguments"/> and <see cref="_formalArguments2"/> fields which are also cloned.
/// </returns>
public CompiledTemplate Clone()
{
CompiledTemplate clone = (CompiledTemplate)MemberwiseClone();
if (_formalArguments != null)
_formalArguments = new List<FormalArgument>(_formalArguments);
if (_formalArguments2 != null)
_formalArguments2 = new Dictionary<string, FormalArgument>(_formalArguments2);

return clone;
}
Expand Down Expand Up @@ -452,6 +463,9 @@ public virtual void AddArgument(FormalArgument a)

a.Index = FormalArguments.Count;
FormalArguments.Add(a);
this._formalArguments2[a.Name] = a; // TODO: Is it supposed to be that duplicates are first-writer-wins or last? The test ST4.TestGroupSyntaxErrors.TestArg2 tries to add a duplicate, but passes with either first or last implementation.
// Last writer wins:
////if (!_formalArguments2.ContainsKey(a.Name)) _formalArguments2.Add(a.Name, a);
if (a.DefaultValueToken != null)
_numberOfArgsWithDefaultValues++;
}
Expand Down