Refactor Command parts to avoid duplicate characters. Reduces memory footprint

build-common/NHibernate.props

@@ -3,9 +3,9 @@
 
   <PropertyGroup>
     <NhVersion     Condition="'$(NhVersion)' == ''"    >5.5</NhVersion>
-    <VersionPatch  Condition="'$(VersionPatch)'  == ''">3</VersionPatch>
+    <VersionPatch  Condition="'$(VersionPatch)'  == ''">4</VersionPatch>
     <!-- Clear VersionSuffix for making release and set it to dev for making development builds -->
-    <VersionSuffix Condition="'$(VersionSuffix)' == ''"></VersionSuffix>
+    <VersionSuffix Condition="'$(VersionSuffix)' == ''">dev</VersionSuffix>
     <LangVersion Condition="'$(MSBuildProjectExtension)' != '.vbproj'">9.0</LangVersion>
 
     <VersionPrefix Condition="'$(VersionPrefix)' == ''">$(NhVersion).$(VersionPatch)</VersionPrefix>

src/NHibernate.Test/Async/NHSpecificTest/GH3643/FixtureByCode.cs

@@ -0,0 +1,124 @@
+//------------------------------------------------------------------------------
+// <auto-generated>
+//     This code was generated by AsyncGenerator.
+//
+//     Changes to this file may cause incorrect behavior and will be lost if
+//     the code is regenerated.
+// </auto-generated>
+//------------------------------------------------------------------------------
+
+
+using System.Linq;
+using NHibernate.Cfg;
+using NHibernate.Cfg.MappingSchema;
+using NHibernate.Linq;
+using NHibernate.Mapping.ByCode;
+using NUnit.Framework;
+
+namespace NHibernate.Test.NHSpecificTest.GH3643
+{
+	using System.Threading.Tasks;
+	using System.Threading;
+	[TestFixture]
+	public class FixtureByCodeAsync : TestCaseMappingByCode
+	{
+		protected override void Configure(Configuration configuration)
+		{
+			configuration.SetProperty(Environment.UseQueryCache, "true");
+			configuration.SetProperty(Environment.GenerateStatistics, "true");
+		}
+
+		protected override HbmMapping GetMappings()
+		{
+			var mapper = new ModelMapper();
+
+			mapper.Class<Entity>(
+				rc =>
+				{
+					rc.Id(x => x.Id);
+					rc.Bag(
+						x => x.Children,
+						m =>
+						{
+							m.Access(Accessor.Field);
+							m.Key(k => k.Column("EntityId"));
+							m.Cascade(Mapping.ByCode.Cascade.All);
+						},
+						r => r.OneToMany());
+
+					rc.Cache(
+						cm =>
+						{
+							cm.Include(CacheInclude.All);
+							cm.Usage(CacheUsage.ReadWrite);
+						});
+				});
+
+			mapper.Class<ChildEntity>(
+				rc =>
+				{
+					rc.Id(x => x.Id);
+					rc.Cache(
+						cm =>
+						{
+							cm.Include(CacheInclude.All);
+							cm.Usage(CacheUsage.ReadWrite);
+						});
+				});
+
+			return mapper.CompileMappingForAllExplicitlyAddedEntities();
+		}
+
+		protected override void OnSetUp()
+		{
+			using var session = OpenSession();
+			using var transaction = session.BeginTransaction();
+
+			var entity = new Entity { Id = EntityId.Id1 };
+			entity.Children.Add(new ChildEntity { Id = 0 });
+			entity.Children.Add(new ChildEntity { Id = 1 });
+			session.Save(entity);
+
+			transaction.Commit();
+		}
+
+		protected override void OnTearDown()
+		{
+			using var session = OpenSession();
+			using var transaction = session.BeginTransaction();
+
+			session.CreateQuery("delete from ChildEntity").ExecuteUpdate();
+			session.CreateQuery("delete from System.Object").ExecuteUpdate();
+
+			transaction.Commit();
+		}
+
+		[Test]
+		public async Task LoadsEntityWithEnumIdAndChildrenUsingQueryCacheAsync()
+		{
+			await (LoadEntityWithQueryCacheAsync()); // warm up cache
+
+			var entity = await (LoadEntityWithQueryCacheAsync());
+
+			Assert.That(entity.Children.Count(), Is.EqualTo(2));
+
+			Assert.That(Sfi.Statistics.QueryExecutionCount, Is.EqualTo(1), "Unexpected execution count");
+			Assert.That(Sfi.Statistics.QueryCachePutCount, Is.EqualTo(1), "Unexpected cache put count");
+			Assert.That(Sfi.Statistics.QueryCacheHitCount, Is.EqualTo(1), "Unexpected cache hit count");
+		}
+
+		private async Task<Entity> LoadEntityWithQueryCacheAsync(CancellationToken cancellationToken = default(CancellationToken))
+		{
+			using var session = OpenSession();
+			using var transaction = session.BeginTransaction();
+			var entity = (await (session
+				.Query<Entity>()
+				.FetchMany(x => x.Children)
+				.WithOptions(opt => opt.SetCacheable(true))
+				.ToListAsync(cancellationToken)))[0];
+
+			await (transaction.CommitAsync(cancellationToken));
+			return entity;
+		}
+	}
+}

src/NHibernate.Test/NHSpecificTest/GH3643/Entity.cs

@@ -0,0 +1,25 @@
+using System.Collections.Generic;
+
+// ReSharper disable CollectionNeverUpdated.Local
+// ReSharper disable UnassignedGetOnlyAutoProperty
+
+namespace NHibernate.Test.NHSpecificTest.GH3643
+{
+	class Entity
+	{
+		private readonly ICollection<ChildEntity> _children = new List<ChildEntity>();
+		public virtual EntityId Id { get; set; }
+		public virtual ICollection<ChildEntity> Children => _children;
+	}
+
+	class ChildEntity
+	{
+		public virtual int Id { get; set; }
+	}
+
+	enum EntityId
+	{
+		Id1,
+		Id2
+	}
+}

src/NHibernate.Test/NHSpecificTest/GH3643/FixtureByCode.cs

@@ -0,0 +1,112 @@
+using System.Linq;
+using NHibernate.Cfg;
+using NHibernate.Cfg.MappingSchema;
+using NHibernate.Linq;
+using NHibernate.Mapping.ByCode;
+using NUnit.Framework;
+
+namespace NHibernate.Test.NHSpecificTest.GH3643
+{
+	[TestFixture]
+	public class FixtureByCode : TestCaseMappingByCode
+	{
+		protected override void Configure(Configuration configuration)
+		{
+			configuration.SetProperty(Environment.UseQueryCache, "true");
+			configuration.SetProperty(Environment.GenerateStatistics, "true");
+		}
+
+		protected override HbmMapping GetMappings()
+		{
+			var mapper = new ModelMapper();
+
+			mapper.Class<Entity>(
+				rc =>
+				{
+					rc.Id(x => x.Id);
+					rc.Bag(
+						x => x.Children,
+						m =>
+						{
+							m.Access(Accessor.Field);
+							m.Key(k => k.Column("EntityId"));
+							m.Cascade(Mapping.ByCode.Cascade.All);
+						},
+						r => r.OneToMany());
+
+					rc.Cache(
+						cm =>
+						{
+							cm.Include(CacheInclude.All);
+							cm.Usage(CacheUsage.ReadWrite);
+						});
+				});
+
+			mapper.Class<ChildEntity>(
+				rc =>
+				{
+					rc.Id(x => x.Id);
+					rc.Cache(
+						cm =>
+						{
+							cm.Include(CacheInclude.All);
+							cm.Usage(CacheUsage.ReadWrite);
+						});
+				});
+
+			return mapper.CompileMappingForAllExplicitlyAddedEntities();
+		}
+
+		protected override void OnSetUp()
+		{
+			using var session = OpenSession();
+			using var transaction = session.BeginTransaction();
+
+			var entity = new Entity { Id = EntityId.Id1 };
+			entity.Children.Add(new ChildEntity { Id = 0 });
+			entity.Children.Add(new ChildEntity { Id = 1 });
+			session.Save(entity);
+
+			transaction.Commit();
+		}
+
+		protected override void OnTearDown()
+		{
+			using var session = OpenSession();
+			using var transaction = session.BeginTransaction();
+
+			session.CreateQuery("delete from ChildEntity").ExecuteUpdate();
+			session.CreateQuery("delete from System.Object").ExecuteUpdate();
+
+			transaction.Commit();
+		}
+
+		[Test]
+		public void LoadsEntityWithEnumIdAndChildrenUsingQueryCache()
+		{
+			LoadEntityWithQueryCache(); // warm up cache
+
+			var entity = LoadEntityWithQueryCache();
+
+			Assert.That(entity.Children.Count(), Is.EqualTo(2));
+
+			Assert.That(Sfi.Statistics.QueryExecutionCount, Is.EqualTo(1), "Unexpected execution count");
+			Assert.That(Sfi.Statistics.QueryCachePutCount, Is.EqualTo(1), "Unexpected cache put count");
+			Assert.That(Sfi.Statistics.QueryCacheHitCount, Is.EqualTo(1), "Unexpected cache hit count");
+		}
+
+		private Entity LoadEntityWithQueryCache()
+		{
+			using var session = OpenSession();
+			using var transaction = session.BeginTransaction();
+			var entity = session
+				.Query<Entity>()
+				.FetchMany(x => x.Children)
+				.WithOptions(opt => opt.SetCacheable(true))
+				.ToList()[0];
+
+			transaction.Commit();
+			return entity;
+		}
+	}
+}

src/NHibernate.Test/SqlCommandTest/SqlStringFixture.cs

@@ -3,6 +3,7 @@
 using System.Diagnostics;
 using System.Linq;
 using NHibernate.SqlCommand;
+using NHibernate.Util;
 using NUnit.Framework;
 
 namespace NHibernate.Test.SqlCommandTest
@@ -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()
 		{

src/NHibernate/Async/Type/TypeHelper.cs

@@ -125,6 +125,7 @@ internal static async Task InitializeCollectionsAsync(
 					continue;
 				}
 
+				value = await (pair.Value.KeyType.AssembleAsync(value, session, null, cancellationToken)).ConfigureAwait(false);
 				var collection = session.PersistenceContext.GetCollection(new CollectionKey(pair.Value, value));
 				await (collection.ForceInitializationAsync(cancellationToken)).ConfigureAwait(false);
 				assembleRow[pair.Key] = collection;

src/NHibernate/SqlCommand/SqlString.cs

@@ -5,6 +5,7 @@
 using System.Collections;
 using NHibernate.SqlCommand.Parser;
 using System.Text.RegularExpressions;
+using NHibernate.Util;
 
 namespace NHibernate.SqlCommand
 {
@@ -1064,7 +1065,14 @@ private struct Part : IEquatable<Part>
 			public Part(int sqlIndex, string content)
 			{
 				SqlIndex = sqlIndex;
-				Content = content;
+				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
+				};
 				IsParameter = false;
 			}
 

src/NHibernate/Type/TypeHelper.cs

@@ -1,4 +1,4 @@
-using System;
+using System;
 using System.Collections;
 using System.Collections.Generic;
 using NHibernate.Collection;
@@ -133,6 +133,7 @@ internal static void InitializeCollections(
 					continue;
 				}
 
+				value = pair.Value.KeyType.Assemble(value, session, null);
 				var collection = session.PersistenceContext.GetCollection(new CollectionKey(pair.Value, value));
 				collection.ForceInitialization();
 				assembleRow[pair.Key] = collection;