Skip to content

Commit b3aca78

Browse files
committed
Enable explicit connection pooling on Save()
While ADO.NET's built-in connection pooling _should_ be sufficient, with the new handling of unresolved references, it's easy to maintain an open connection for the duration of the recursive save. For most cases, where the save isn't recursive, this won't impact performance one way or the other. For recursive saves of large topic graphs, however, it eliminates some overhead of opening and closing a connection for every `Topic` in the topic graph.
1 parent 0c860ae commit b3aca78

1 file changed

Lines changed: 17 additions & 7 deletions

File tree

OnTopic.Data.Sql/SqlTopicRepository.cs

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -269,22 +269,27 @@ public override int Save([NotNull]Topic topic, bool isRecursive = false, bool is
269269
| Establish dependencies
270270
\-----------------------------------------------------------------------------------------------------------------------*/
271271
var unresolvedTopics = new List<Topic>();
272+
var connection = new SqlConnection(_connectionString);
273+
274+
connection.Open();
272275

273276
/*------------------------------------------------------------------------------------------------------------------------
274277
| Handle first pass
275278
\-----------------------------------------------------------------------------------------------------------------------*/
276-
var topicId = Save(topic, isRecursive, isDraft, unresolvedTopics);
279+
var topicId = Save(topic, isRecursive, isDraft, connection, unresolvedTopics);
277280

278281
/*------------------------------------------------------------------------------------------------------------------------
279282
| Attempt to resolve outstanding relationships
280283
\-----------------------------------------------------------------------------------------------------------------------*/
281284
foreach (var unresolvedTopic in unresolvedTopics) {
282-
Save(unresolvedTopic, false, isDraft, new List<Topic>());
285+
Save(unresolvedTopic, false, isDraft, connection, new List<Topic>());
283286
}
284287

285288
/*------------------------------------------------------------------------------------------------------------------------
286289
| Return value
287290
\-----------------------------------------------------------------------------------------------------------------------*/
291+
connection.Close();
292+
connection.Dispose();
288293
return topicId;
289294

290295
}
@@ -303,15 +308,22 @@ public override int Save([NotNull]Topic topic, bool isRecursive = false, bool is
303308
/// that include such references. This adds some overhead due to the duplicate <see cref="Save"/>, but helps avoid
304309
/// potential data loss when working with complex topic graphs.
305310
/// </para>
311+
/// <para>
312+
/// The connection sharing probably doesn't provide that much of a gain in that .NET does a good job of connection
313+
/// pooling. Nevertheless, there is some overhead to opening a new connection, so sharing an open connection when we
314+
/// doing a recursive save could potentially provide some performance benefit.
315+
/// </para>
306316
/// </remarks>
307317
/// <param name="topic">The source <see cref="Topic"/> to save.</param>
308318
/// <param name="isRecursive">Determines whether or not to recursively save <see cref="Topic.Children"/>.</param>
309319
/// <param name="isDraft">Determines if the <see cref="Topic"/> should be saved as a draft version.</param>
320+
/// <param name="connection">The open <see cref="SqlConnection"/> to use for executing <see cref="SqlCommand"/>s.</param>
310321
/// <param name="unresolvedRelationships">A list of <see cref="Topic"/>s with unresolved topic references.</param>
311322
private int Save(
312323
[NotNull]Topic topic,
313324
bool isRecursive,
314325
bool isDraft,
326+
SqlConnection connection,
315327
List<Topic> unresolvedRelationships
316328
) {
317329

@@ -406,7 +418,6 @@ void addUnmatchedAttribute(string key) {
406418
| Establish database connection
407419
\-----------------------------------------------------------------------------------------------------------------------*/
408420
var isNew = topic.Id == -1;
409-
var connection = new SqlConnection(_connectionString);
410421
var procedureName = isNew? "CreateTopic" : "UpdateTopic";
411422
var command = new SqlCommand(procedureName, connection) {
412423
CommandType = CommandType.StoredProcedure
@@ -446,7 +457,6 @@ void addUnmatchedAttribute(string key) {
446457
\-----------------------------------------------------------------------------------------------------------------------*/
447458
try {
448459

449-
connection.Open();
450460
command.ExecuteNonQuery();
451461

452462
topic.Id = command.GetReturnCode();
@@ -466,6 +476,7 @@ void addUnmatchedAttribute(string key) {
466476
| Catch exception
467477
\-----------------------------------------------------------------------------------------------------------------------*/
468478
catch (SqlException exception) {
479+
connection?.Dispose();
469480
throw new TopicRepositoryException(
470481
$"Failed to save Topic '{topic.Key}' ({topic.Id}) via '{_connectionString}': '{exception.Message}'",
471482
exception
@@ -477,7 +488,6 @@ void addUnmatchedAttribute(string key) {
477488
\-----------------------------------------------------------------------------------------------------------------------*/
478489
finally {
479490
command?.Dispose();
480-
connection?.Dispose();
481491
attributes.Dispose();
482492
}
483493

@@ -487,7 +497,7 @@ void addUnmatchedAttribute(string key) {
487497
if (isRecursive) {
488498
foreach (var childTopic in topic.Children) {
489499
childTopic.Attributes.SetValue("ParentID", topic.Id.ToString(CultureInfo.InvariantCulture));
490-
Save(childTopic, isRecursive, isDraft, unresolvedRelationships);
500+
Save(childTopic, isRecursive, isDraft, connection, unresolvedRelationships);
491501
}
492502
}
493503

@@ -697,7 +707,7 @@ private static string PersistRelations(Topic topic, SqlConnection connection, bo
697707
finally {
698708
command?.Dispose();
699709
targetIds.Dispose();
700-
//Since the SQL connection is being passed in, do not close connection; this allows command pooling.
710+
//Since the SQL connection is being passed in, do not close connection; this allows connection pooling.
701711
}
702712

703713
/*------------------------------------------------------------------------------------------------------------------------

0 commit comments

Comments
 (0)