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.

Sign up for GitHub

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

feat: Ensure Format instances get returned to object pool #402

Merged
merged 2 commits into from
May 25, 2024
+68 −34
Merged

feat: Ensure Format instances get returned to object pool #402

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6c676a3
feat: Ensure Format instances get returned to object pool
axunonb May 25, 2024
3fedada
Remove redundant SmartFormatter.Format(FormatDetails, Format, object?)
axunonb May 25, 2024
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
12 changes: 11 additions & 1 deletion src/SmartFormat.Tests/Core/FormatCacheTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,16 @@ public void Format_WithCache()
Assert.That(formatter.Format(format, data), Is.EqualTo($"{data.Name}, {data.City}"));
}

[Test]
public void Format_WithCache_List_Args()
{
var data = new System.Collections.Generic.List<object?> { "Joe", "Melbourne" };
var formatter = GetSimpleFormatter();
var formatString = "{0}, {1}";
var format = formatter.Parser.ParseFormat(formatString);
Assert.That(formatter.Format(format, data), Is.EqualTo($"{data[0]}, {data[1]}"));
}

[Test]
public void Format_WithCache_Into_StringOutput()
{
Expand All @@ -48,4 +58,4 @@ public void Format_WithCache_Into_ZStringOutput()
formatter.FormatInto(output, null, format, data);
Assert.That(output.ToString(), Is.EqualTo($"{data.Name}, {data.City}"));
}
}
}
13 changes: 5 additions & 8 deletions src/SmartFormat/Core/Parsing/Parser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ private struct IndexContainer
public int Selector;

/// <summary>
/// Adds a number to number to the index and returns the sum, but not more than <see cref="ObjectLength"/>.
/// Adds a number to the index and returns the sum, but not more than <see cref="ObjectLength"/>.
/// </summary>
/// <param name="index"></param>
/// <param name="add"></param>
Expand Down Expand Up @@ -234,8 +234,8 @@ public Format ParseFormat(string inputFormat)
NamedFormatterOptionsStart = PositionUndefined, NamedFormatterOptionsEnd = PositionUndefined,
Operator = PositionUndefined, Selector = PositionUndefined
};
// Initialize - will be re-assigned with new placeholders

// Initialize - can be re-assigned with new placeholders, while _resultFormat will become the parent
_resultFormat = FormatPool.Instance.Get().Initialize(Settings, _inputFormat);

// Store parsing errors until parsing is finished:
Expand Down Expand Up @@ -275,7 +275,7 @@ public Format ParseFormat(string inputFormat)
// Make sure that this is a nested placeholder before we un-nest it:
if (HasProcessedTooMayClosingBraces(parsingErrors)) continue;

// End of the placeholder's Format, _resultFormat will change to parent.parent
// End of the placeholder's Format, _resultFormat will change to ParentPlaceholder.Parent
FinishPlaceholderFormat(ref nestedDepth);
}
else if (inputChar == _parserSettings.CharLiteralEscapeChar && _parserSettings.ConvertCharacterStringLiterals ||
Expand Down Expand Up @@ -427,8 +427,6 @@ private void FinishPlaceholderFormat(ref int nestedDepth)
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private void ParseAlternativeEscaping()
{
// 2021-05-03/axuno: Removed "index.NamedFormatterStart = PositionUndefined"

// See what is the next character
var indexNextChar = _index.SafeAdd(_index.Current, 1);
if (indexNextChar >= _inputFormat.Length)
Expand Down Expand Up @@ -589,7 +587,7 @@ private void ParseSelector(ref Placeholder? currentPlaceholder, ParsingErrors pa
// Start the format:
var newFormat = FormatPool.Instance.Get().Initialize(Settings, currentPlaceholder, _index.Current + 1);
currentPlaceholder.Format = newFormat;
//FormatPool.Instance.Return(_resultFormat); // return to pool before reassigning
// _parentFormat still lives in the current placeholder!
_resultFormat = newFormat;
currentPlaceholder = null;
// named formatters will not be parsed with string.Format compatibility switched ON.
Expand All @@ -605,7 +603,6 @@ private void ParseSelector(ref Placeholder? currentPlaceholder, ParsingErrors pa
// End the placeholder with no format:
nestedDepth--;
currentPlaceholder.EndIndex = _index.SafeAdd(_index.Current, 1);
//_resultFormat = currentPlaceholder.Parent; // removed 2021-08-08: The parent always is the _resultFormat
currentPlaceholder = null;
}
else
Expand Down
77 changes: 52 additions & 25 deletions src/SmartFormat/SmartFormatter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ public SmartFormatter InsertExtension(int position, IFormatter formatterExtensio
if (_formatterExtensions.Exists(sx => sx.GetType() == formatterExtension.GetType())) return this;

// Extension name is in use by a different type
if(_formatterExtensions.Exists(fx => fx.Name.Equals(formatterExtension.Name)))
if (_formatterExtensions.Exists(fx => fx.Name.Equals(formatterExtension.Name)))
throw new ArgumentException($"Formatter '{formatterExtension.GetType().Name}' uses existing name.", nameof(formatterExtension));

if (formatterExtension is IInitializer formatterToInitialize)
Expand Down Expand Up @@ -288,18 +288,24 @@ public string Format(IFormatProvider? provider, string format, params object?[]
/// <returns>Returns the formatted input with items replaced with their string representation.</returns>
public string Format(IFormatProvider? provider, string format, IList<object?> args)
{
var formatParsed = Parser.ParseFormat(format); // The parser gets the Format from the pool
var result = Format(provider, formatParsed, args);
FormatPool.Instance.Return(formatParsed);
return result;
var formatParsed = Parser.ParseFormat(format);
try
{
var result = Format(provider, formatParsed, args);
return result;
}
finally
{
FormatPool.Instance.Return(formatParsed); // The parser gets the Format from the pool
}
}

#region ** Format overloads with cached Format **

/// <summary>
/// Replaces one or more format items in as specified string with the string representation of a specific object.
/// </summary>
/// <param name="formatParsed">An instance of <see cref="Core.Parsing.Format"/> that was returned by <see cref="SmartFormat.Core.Parsing.Parser.ParseFormat"/>.</param>
/// <param name="formatParsed">An instance of <see cref="Core.Parsing.Format"/> that was returned by <see cref="Parser.ParseFormat"/>.</param>
/// <param name="args">The object to format.</param>
/// <returns>Returns the formatted input with items replaced with their string representation.</returns>
public string Format(Format formatParsed, params object?[] args)
Expand All @@ -310,7 +316,7 @@ public string Format(Format formatParsed, params object?[] args)
/// <summary>
/// Replaces one or more format items in as specified string with the string representation of a specific object.
/// </summary>
/// <param name="formatParsed">An instance of <see cref="Core.Parsing.Format"/> that was returned by <see cref="SmartFormat.Core.Parsing.Parser.ParseFormat"/>.</param>
/// <param name="formatParsed">An instance of <see cref="Core.Parsing.Format"/> that was returned by <see cref="Parser.ParseFormat"/>.</param>
/// <param name="args">The object to format.</param>
/// <returns>Returns the formatted input with items replaced with their string representation.</returns>
public string Format(Format formatParsed, IList<object?> args)
Expand All @@ -322,7 +328,7 @@ public string Format(Format formatParsed, IList<object?> args)
/// Replaces one or more format items in a specified string with the string representation of a specific object.
/// </summary>
/// <param name="provider">The <see cref="IFormatProvider" /> to use.</param>
/// <param name="formatParsed">An instance of <see cref="Core.Parsing.Format"/> that was returned by <see cref="SmartFormat.Core.Parsing.Parser.ParseFormat"/>.</param>
/// <param name="formatParsed">An instance of <see cref="Core.Parsing.Format"/> that was returned by <see cref="Parser.ParseFormat"/>.</param>
/// <param name="args">The object to format.</param>
/// <returns>Returns the formatted input with items replaced with their string representation.</returns>
public string Format(IFormatProvider? provider, Format formatParsed, params object?[] args)
Expand All @@ -334,7 +340,7 @@ public string Format(IFormatProvider? provider, Format formatParsed, params obje
/// Replaces one or more format items in a specified string with the string representation of a specific object.
/// </summary>
/// <param name="provider">The <see cref="IFormatProvider" /> to use.</param>
/// <param name="formatParsed">An instance of <see cref="Core.Parsing.Format"/> that was returned by <see cref="SmartFormat.Core.Parsing.Parser.ParseFormat"/>.</param>
/// <param name="formatParsed">An instance of <see cref="Core.Parsing.Format"/> that was returned by <see cref="Parser.ParseFormat"/>.</param>
/// <param name="args">The object to format.</param>
/// <returns>Returns the formatted input with items replaced with their string representation.</returns>
public string Format(IFormatProvider? provider, Format formatParsed, IList<object?> args)
Expand Down Expand Up @@ -394,13 +400,6 @@ public void Format(FormattingInfo formattingInfo)
}
}

private void Format(FormatDetails formatDetails, Format format, object? current)
{
var formattingInfo = FormattingInfoPool.Instance.Get().Initialize(formatDetails, format, current);
Format(formattingInfo);
FormattingInfoPool.Instance.Return(formattingInfo);
}

#endregion

#region: FormatInto Overloads :
Expand Down Expand Up @@ -437,8 +436,14 @@ public void FormatInto(IOutput output, string format, IList<object?> args)
public void FormatInto(IOutput output, IFormatProvider? provider, string format, IList<object?> args)
{
var formatParsed = Parser.ParseFormat(format);
FormatInto(output, provider, formatParsed, args);
FormatPool.Instance.Return(formatParsed); // The parser gets the Format from the pool
try
{
FormatInto(output, provider, formatParsed, args);
}
finally
{
FormatPool.Instance.Return(formatParsed); // The parser gets the Format from the pool
}
}

#region: FormatInto Overloads with cached Format :
Expand All @@ -448,7 +453,7 @@ public void FormatInto(IOutput output, IFormatProvider? provider, string format,
/// </summary>
/// <param name="output">The <see cref="IOutput"/> where the result is written to.</param>
/// <param name="provider"></param>
/// <param name="format">An instance of <see cref="Core.Parsing.Format"/> that was returned by <see cref="SmartFormat.Core.Parsing.Parser.ParseFormat"/>.</param>
/// <param name="format">An instance of <see cref="Core.Parsing.Format"/> that was returned by <see cref="Parser.ParseFormat"/>.</param>
/// <param name="args">The objects to format.</param>
public void FormatInto(IOutput output, IFormatProvider? provider, Format format, params object?[] args)
{
Expand All @@ -460,15 +465,11 @@ public void FormatInto(IOutput output, IFormatProvider? provider, Format format,
/// </summary>
/// <param name="output">The <see cref="IOutput"/> where the result is written to.</param>
/// <param name="provider"></param>
/// <param name="formatParsed">An instance of <see cref="Core.Parsing.Format"/> that was returned by <see cref="SmartFormat.Core.Parsing.Parser.ParseFormat"/>.</param>
/// <param name="formatParsed">An instance of <see cref="Core.Parsing.Format"/> that was returned by <see cref="Parser.ParseFormat"/>.</param>
/// <param name="args">The objects to format.</param>
public void FormatInto(IOutput output, IFormatProvider? provider, Format formatParsed, IList<object?> args)
{
var current = args.Count > 0 ? args[0] : args; // The first item is the default.

var formatDetails = FormatDetailsPool.Instance.Get().Initialize(this, formatParsed, args, provider, output);
Format(formatDetails, formatParsed, current);
FormatDetailsPool.Instance.Return(formatDetails);
PerformActionWithFormattingInfo(this, provider, formatParsed, args, output, Format);
}

#endregion
Expand Down Expand Up @@ -648,5 +649,31 @@ private bool InvokeFormatterExtensions(FormattingInfo formattingInfo)
return false;
}

/// <summary>
/// Creates a new instance of <see cref="FormattingInfo"/>
/// and performs the <see paramref="work"/> action
/// using the <see cref="FormattingInfo"/>.
/// </summary>
/// <param name="formatter"></param>
/// <param name="provider"></param>
/// <param name="formatParsed"></param>
/// <param name="args">The data argument.</param>
/// <param name="output"></param>
/// <param name="doWork">The <see cref="Action{T}"/>to invoke.</param>
/// <remarks>
/// The method uses object pooling to reduce GC pressure,
/// and assures that objects are returned to the pool after
/// <see paramref="work"/> is done (or an exception is thrown).
/// </remarks>
private static void PerformActionWithFormattingInfo(SmartFormatter formatter, IFormatProvider? provider, Format formatParsed, IList<object?> args, IOutput? output, Action<FormattingInfo> doWork)
{
var current = args.Count > 0 ? args[0] : args; // The first item is the default.
using var fdo = FormatDetailsPool.Instance.Pool.Get(out var formatDetails);
formatDetails.Initialize(formatter, formatParsed, args, provider, output ?? new ZStringOutput(ZStringBuilderUtilities.CalcCapacity(formatParsed)));
using var fio = FormattingInfoPool.Instance.Pool.Get(out var formattingInfo);
formattingInfo.Initialize(formatDetails, formatParsed, current);
doWork(formattingInfo);
}

#endregion
}