Skip to content
Open
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
40 changes: 40 additions & 0 deletions src/NHibernate.Test/SqlCommandTest/SqlStringFixture.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using System.Diagnostics;
using System.Linq;
using NHibernate.SqlCommand;
using NHibernate.Util;
using NUnit.Framework;

namespace NHibernate.Test.SqlCommandTest
Expand Down Expand Up @@ -418,6 +419,45 @@ public void GetSubselectStringWithOrderByInSubselect()
Assert.AreEqual(" from table where (col = test) and id in (select id from foo order by bar)", sql.GetSubselectString().ToString());
}

[Test]
public void PartDeduplicatesCommonSeparatorStrings()
{
// new string(char[]) produces a runtime instance that is NOT interned and NOT
// reference-equal to the StringHelper constants — exactly what happens when
// NHibernate builds separator strings dynamically via StringBuilder.ToString().
var commaSpace = new string(new[] { ',', ' ' });
var closedParen = new string(new[] { ')' });
var openParen = new string(new[] { '(' });
var comma = new string(new[] { ',' });

// Precondition: these must be distinct instances from the constants
Assert.That(ReferenceEquals(commaSpace, StringHelper.CommaSpace), Is.False, "Precondition");
Assert.That(ReferenceEquals(closedParen, StringHelper.ClosedParen), Is.False, "Precondition");
Assert.That(ReferenceEquals(openParen, StringHelper.OpenParen), Is.False, "Precondition");
Assert.That(ReferenceEquals(comma, StringHelper.Comma), Is.False, "Precondition");

// Sandwiching each separator between two parameters ensures it lands in a
// "middle" Part whose Content is returned directly by the enumerator
// (no Substring call), enabling reference-equality verification.
var sqlCommaSpace = new SqlString(new object[] { Parameter.Placeholder, commaSpace, Parameter.Placeholder });
var sqlClosedParen = new SqlString(new object[] { Parameter.Placeholder, closedParen, Parameter.Placeholder });
var sqlOpenParen = new SqlString(new object[] { Parameter.Placeholder, openParen, Parameter.Placeholder });
var sqlComma = new SqlString(new object[] { Parameter.Placeholder, comma, Parameter.Placeholder });

Assert.That(ReferenceEquals(sqlCommaSpace.OfType<string>().Single(), StringHelper.CommaSpace), Is.True, "', ' should be deduplicated");
Assert.That(ReferenceEquals(sqlClosedParen.OfType<string>().Single(), StringHelper.ClosedParen), Is.True, "')' should be deduplicated");
Assert.That(ReferenceEquals(sqlOpenParen.OfType<string>().Single(), StringHelper.OpenParen), Is.True, "'(' should be deduplicated");
Assert.That(ReferenceEquals(sqlComma.OfType<string>().Single(), StringHelper.Comma), Is.True, "',' should be deduplicated");
}

[Test]
public void PartPreservesNonSeparatorContent()
{
const string fragment = "select x from y where z = ";
var sql = new SqlString(new object[] { Parameter.Placeholder, fragment, Parameter.Placeholder });
Assert.That(sql.OfType<string>().Single(), Is.EqualTo(fragment));
}

[Test]
public void ParameterPropertyShouldReturnNewInstances()
{
Expand Down
18 changes: 17 additions & 1 deletion src/NHibernate/SqlCommand/SqlString.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Collections;
using NHibernate.SqlCommand.Parser;
using System.Text.RegularExpressions;
using NHibernate.Util;

namespace NHibernate.SqlCommand
{
Expand Down Expand Up @@ -1067,7 +1068,22 @@ private struct Part : IEquatable<Part>
public Part(int sqlIndex, string content)
{
SqlIndex = sqlIndex;
Content = content;
// Deduplicate the most common single- and two-character tokens by returning references
// to the compile-time-interned StringHelper constants instead of holding onto the caller's
// freshly allocated string. Parens and commas dominate SQL fragment memory after long runs
// (see #3608), and this removes their duplication without allocating. The explicit switch
// is intentional: benchmarks showed it to be ~5–50× faster than string.IsInterned in this
// size range, since the length-gated comparisons against the constants typically fold down
// to reference equality, while IsInterned pays for hashing and intern-table synchronisation
// on every call.
Content = content.Length switch
{
1 when content == StringHelper.ClosedParen => StringHelper.ClosedParen,
1 when content == StringHelper.OpenParen => StringHelper.OpenParen,
1 when content == StringHelper.Comma => StringHelper.Comma,
2 when content == StringHelper.CommaSpace => StringHelper.CommaSpace,
_ => content
};
Comment thread
fredericDelaporte marked this conversation as resolved.
Comment thread
fredericDelaporte marked this conversation as resolved.
IsParameter = false;
}

Expand Down
Loading