From 44623f619e45e5d236e00952a75b091abe2e0d3f Mon Sep 17 00:00:00 2001 From: AlexUstinov Date: Fri, 10 Apr 2020 14:20:26 -0700 Subject: [PATCH 1/6] Use ref struct as disposable instance to avoid allocation. Also avoids closure allocation --- .../Collections/BindingCollection.cs | 105 ++++++++++-------- .../Orm/Linq/LinqBindingCollection.cs | 9 +- .../Orm/Linq/Translator.Queryable.cs | 4 +- 3 files changed, 69 insertions(+), 49 deletions(-) diff --git a/Orm/Xtensive.Orm/Collections/BindingCollection.cs b/Orm/Xtensive.Orm/Collections/BindingCollection.cs index 2cfd6da950..0ae40098ac 100644 --- a/Orm/Xtensive.Orm/Collections/BindingCollection.cs +++ b/Orm/Xtensive.Orm/Collections/BindingCollection.cs @@ -8,7 +8,6 @@ using System.Collections; using System.Collections.Generic; using System.Diagnostics; -using Xtensive.Core; namespace Xtensive.Collections @@ -21,8 +20,50 @@ namespace Xtensive.Collections /// [Serializable] [DebuggerDisplay("Count = {Count}")] - public class BindingCollection : IEnumerable> + public class BindingCollection : IReadOnlyCollection> { + public readonly ref struct BindingScope + { + public static BindingScope Empty => new BindingScope(); + + private readonly BindingCollection owner; + private readonly TKey key; + private readonly TValue prevValue; + private readonly bool prevValueExists; + + public void Dispose() + { + if (owner == null) { + return; + } + + if (prevValueExists) { + if (!owner.permanentBindings.Contains(key)) { + owner.bindings[key] = prevValue; + } + } + else { + if (!owner.permanentBindings.Contains(key)) { + owner.bindings.Remove(key); + } + } + } + + public BindingScope(BindingCollection owner, TKey key) : this() + { + this.owner = owner; + this.key = key; + } + + public BindingScope(BindingCollection owner, TKey key, TValue prevValue) + { + this.owner = owner; + this.key = key; + this.prevValue = prevValue; + prevValueExists = true; + } + } + [DebuggerBrowsable(DebuggerBrowsableState.RootHidden)] private readonly Dictionary bindings = new Dictionary(); private readonly HashSet permanentBindings = new HashSet(); @@ -32,7 +73,7 @@ public class BindingCollection : IEnumerable public virtual int Count { [DebuggerStepThrough] - get { return bindings.Count; } + get => bindings.Count; } /// @@ -41,7 +82,7 @@ public virtual int Count { /// public virtual TValue this[TKey key] { [DebuggerStepThrough] - get { return bindings[key]; } + get => bindings[key]; } /// @@ -51,24 +92,15 @@ public virtual TValue this[TKey key] { /// The value to bind. /// Disposable object that will /// destroy the binding on its disposal. - public virtual IDisposable Add(TKey key, TValue value) + public virtual BindingScope Add(TKey key, TValue value) { - TValue previous; - - if (bindings.TryGetValue(key, out previous)) { + if (bindings.TryGetValue(key, out var previous)) { bindings[key] = value; - return new Disposable(isDisposing => { - if (!permanentBindings.Contains(key)) - bindings[key] = previous; - }); - } - else { - bindings.Add(key, value); - return new Disposable(isDisposing => { - if (!permanentBindings.Contains(key)) - bindings.Remove(key); - }); + return new BindingScope(this, key, previous); } + + bindings.Add(key, value); + return new BindingScope(this, key); } /// @@ -80,8 +112,9 @@ public virtual IDisposable Add(TKey key, TValue value) public virtual void PermanentAdd(TKey key, TValue value) { bindings[key] = value; - if (!permanentBindings.Contains(key)) + if (!permanentBindings.Contains(key)) { permanentBindings.Add(key); + } } /// @@ -92,8 +125,10 @@ public virtual void PermanentAdd(TKey key, TValue value) /// Key isn't found. public virtual void ReplaceBound(TKey key, TValue value) { - if (!bindings.ContainsKey(key)) + if (!bindings.ContainsKey(key)) { throw new KeyNotFoundException(); + } + bindings[key] = value; } @@ -108,10 +143,7 @@ public virtual void ReplaceBound(TKey key, TValue value) /// contains an element with the specified key; /// otherwise, . [DebuggerStepThrough] - public virtual bool TryGetValue(TKey key, out TValue value) - { - return bindings.TryGetValue(key, out value); - } + public virtual bool TryGetValue(TKey key, out TValue value) => bindings.TryGetValue(key, out value); /// /// Gets the sequence of bound keys. @@ -119,34 +151,19 @@ public virtual bool TryGetValue(TKey key, out TValue value) /// The sequence of bound keys. public virtual IEnumerable GetKeys() { - foreach (var key in bindings.Keys) + foreach (var key in bindings.Keys) { yield return key; + } } #region IEnumerable<...> methods /// - public virtual IEnumerator> GetEnumerator() - { - return bindings.GetEnumerator(); - } + public virtual IEnumerator> GetEnumerator() => bindings.GetEnumerator(); /// - IEnumerator IEnumerable.GetEnumerator() - { - return GetEnumerator(); - } + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); #endregion - - - // Constructors - - /// - /// Initializes new instance of this type. - /// - public BindingCollection() - { - } } } \ No newline at end of file diff --git a/Orm/Xtensive.Orm/Orm/Linq/LinqBindingCollection.cs b/Orm/Xtensive.Orm/Orm/Linq/LinqBindingCollection.cs index f76c1063a6..1d5533edf6 100644 --- a/Orm/Xtensive.Orm/Orm/Linq/LinqBindingCollection.cs +++ b/Orm/Xtensive.Orm/Orm/Linq/LinqBindingCollection.cs @@ -19,10 +19,13 @@ internal class LinqBindingCollection : BindingCollection> linkedParameters = new Dictionary>(); - public override IDisposable Add(ParameterExpression key, ProjectionExpression value) + public override BindingScope Add(ParameterExpression key, ProjectionExpression value) { - if (!key.Type.IsAssignableFrom(value.ItemProjector.Type)) - throw new ArgumentException(Strings.ExParameterExpressionMustHaveSameTypeAsProjectionExpressionItemProjector, "key"); + if (!key.Type.IsAssignableFrom(value.ItemProjector.Type)) { + throw new ArgumentException( + Strings.ExParameterExpressionMustHaveSameTypeAsProjectionExpressionItemProjector, nameof(key)); + } + return base.Add(key, value); } diff --git a/Orm/Xtensive.Orm/Orm/Linq/Translator.Queryable.cs b/Orm/Xtensive.Orm/Orm/Linq/Translator.Queryable.cs index 224b254a00..d41803aa3b 100644 --- a/Orm/Xtensive.Orm/Orm/Linq/Translator.Queryable.cs +++ b/Orm/Xtensive.Orm/Orm/Linq/Translator.Queryable.cs @@ -1072,7 +1072,7 @@ private ProjectionExpression VisitSelectMany(Expression source, LambdaExpression var visitedSource = Visit(source); var sequence = VisitSequence(visitedSource); - IDisposable indexBinding = null; + var indexBinding = BindingCollection.BindingScope.Empty; if (collectionSelector.Parameters.Count==2) { var indexProjection = GetIndexBinding(collectionSelector, ref sequence); indexBinding = context.Bindings.Add(collectionSelector.Parameters[1], indexProjection); @@ -1191,7 +1191,7 @@ private ProjectionExpression VisitWhere(Expression expression, LambdaExpression { var parameter = le.Parameters[0]; ProjectionExpression visitedSource = VisitSequence(expression); - IDisposable indexBinding = null; + var indexBinding = BindingCollection.BindingScope.Empty; if (le.Parameters.Count==2) { var indexProjection = GetIndexBinding(le, ref visitedSource); indexBinding = context.Bindings.Add(le.Parameters[1], indexProjection); From 71b7039965da061076e09b325e2a371f50b174ec Mon Sep 17 00:00:00 2001 From: AlexUstinov Date: Fri, 10 Apr 2020 14:40:26 -0700 Subject: [PATCH 2/6] Use ref struct as disposable instance to avoid heap allocation in LinqBindingCollection.LinkParameters method --- .../Orm/Linq/LinqBindingCollection.cs | 41 ++++++++++++++----- 1 file changed, 30 insertions(+), 11 deletions(-) diff --git a/Orm/Xtensive.Orm/Orm/Linq/LinqBindingCollection.cs b/Orm/Xtensive.Orm/Orm/Linq/LinqBindingCollection.cs index 1d5533edf6..758bdcd62c 100644 --- a/Orm/Xtensive.Orm/Orm/Linq/LinqBindingCollection.cs +++ b/Orm/Xtensive.Orm/Orm/Linq/LinqBindingCollection.cs @@ -8,7 +8,6 @@ using System.Collections.Generic; using System.Linq.Expressions; using Xtensive.Collections; -using Xtensive.Core; using Xtensive.Orm.Linq.Expressions; namespace Xtensive.Orm.Linq @@ -31,16 +30,18 @@ public override BindingScope Add(ParameterExpression key, ProjectionExpression v public override void PermanentAdd(ParameterExpression key, ProjectionExpression value) { - if (!key.Type.IsAssignableFrom(value.ItemProjector.Type)) - throw new ArgumentException(Strings.ExParameterExpressionMustHaveSameTypeAsProjectionExpressionItemProjector, "key"); + if (!key.Type.IsAssignableFrom(value.ItemProjector.Type)) { + throw new ArgumentException( + Strings.ExParameterExpressionMustHaveSameTypeAsProjectionExpressionItemProjector, nameof(key)); + } + base.PermanentAdd(key, value); } public override void ReplaceBound(ParameterExpression key, ProjectionExpression value) { base.ReplaceBound(key, value); - IEnumerable parameters; - if (linkedParameters.TryGetValue(key, out parameters)) { + if (linkedParameters.TryGetValue(key, out var parameters)) { foreach (var parameter in parameters) { if (parameter!=key) { var projection = this[parameter]; @@ -55,16 +56,34 @@ public override void ReplaceBound(ParameterExpression key, ProjectionExpression } } } - - public Disposable LinkParameters(IEnumerable parameters) + + internal readonly ref struct ParameterScope { - foreach (var parameter in parameters) - linkedParameters.Add(parameter, parameters); - return new Disposable(isDisposing => { + private readonly LinqBindingCollection owner; + private readonly IReadOnlyCollection parameters; + + public void Dispose() + { + var linkedParameters = owner.linkedParameters; foreach (var parameter in parameters) { linkedParameters.Remove(parameter); } - }); + } + + public ParameterScope(LinqBindingCollection owner, IReadOnlyCollection parameters) + { + this.owner = owner; + this.parameters = parameters; + } + } + + public ParameterScope LinkParameters(IReadOnlyCollection parameters) + { + foreach (var parameter in parameters) { + linkedParameters.Add(parameter, parameters); + } + + return new ParameterScope(this, parameters); } } } \ No newline at end of file From 88e29fceecc94c76c3df8bbca972504cb765b323 Mon Sep 17 00:00:00 2001 From: AlexUstinov Date: Fri, 10 Apr 2020 14:40:56 -0700 Subject: [PATCH 3/6] Use ref struct as disposable instance to avoid heap allocation in SkipTakeRewriterState.CreateScope method --- .../Internals/SkipTakeRewriterState.cs | 23 ++++++++++++++----- 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/Orm/Xtensive.Orm/Orm/Rse/Transformation/Internals/SkipTakeRewriterState.cs b/Orm/Xtensive.Orm/Orm/Rse/Transformation/Internals/SkipTakeRewriterState.cs index cfde3142d3..6d89be04ec 100644 --- a/Orm/Xtensive.Orm/Orm/Rse/Transformation/Internals/SkipTakeRewriterState.cs +++ b/Orm/Xtensive.Orm/Orm/Rse/Transformation/Internals/SkipTakeRewriterState.cs @@ -5,8 +5,6 @@ // Created: 2010.01.21 using System; -using Xtensive.Core; - namespace Xtensive.Orm.Rse.Transformation { @@ -56,12 +54,25 @@ private static Func PositiveSelector(Func valueSelector) }; } - public IDisposable CreateScope() + internal readonly ref struct SkipTakeRewriterScope + { + private readonly SkipTakeRewriter rewriter; + private readonly SkipTakeRewriterState prevState; + + public void Dispose() => rewriter.State = prevState; + + public SkipTakeRewriterScope(SkipTakeRewriter rewriter, SkipTakeRewriterState prevState) + { + this.rewriter = rewriter; + this.prevState = prevState; + } + } + + public SkipTakeRewriterScope CreateScope() { var currentState = rewriter.State; - var newState = new SkipTakeRewriterState(currentState); - rewriter.State = newState; - return new Disposable(_ => rewriter.State = currentState); + rewriter.State = new SkipTakeRewriterState(currentState); + return new SkipTakeRewriterScope(rewriter, currentState); } From 39efe701b550a5823809f75add125343357dbd79 Mon Sep 17 00:00:00 2001 From: AlexUstinov Date: Wed, 5 Aug 2020 18:40:12 -0700 Subject: [PATCH 4/6] Move ParameterScope structure to the top of the LinqBindingCollection class declaration --- .../Orm/Linq/LinqBindingCollection.cs | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/Orm/Xtensive.Orm/Orm/Linq/LinqBindingCollection.cs b/Orm/Xtensive.Orm/Orm/Linq/LinqBindingCollection.cs index 758bdcd62c..ef421afadd 100644 --- a/Orm/Xtensive.Orm/Orm/Linq/LinqBindingCollection.cs +++ b/Orm/Xtensive.Orm/Orm/Linq/LinqBindingCollection.cs @@ -15,6 +15,26 @@ namespace Xtensive.Orm.Linq [Serializable] internal class LinqBindingCollection : BindingCollection { + internal readonly ref struct ParameterScope + { + private readonly LinqBindingCollection owner; + private readonly IReadOnlyCollection parameters; + + public void Dispose() + { + var linkedParameters = owner.linkedParameters; + foreach (var parameter in parameters) { + linkedParameters.Remove(parameter); + } + } + + public ParameterScope(LinqBindingCollection owner, IReadOnlyCollection parameters) + { + this.owner = owner; + this.parameters = parameters; + } + } + private readonly Dictionary> linkedParameters = new Dictionary>(); @@ -57,26 +77,6 @@ public override void ReplaceBound(ParameterExpression key, ProjectionExpression } } - internal readonly ref struct ParameterScope - { - private readonly LinqBindingCollection owner; - private readonly IReadOnlyCollection parameters; - - public void Dispose() - { - var linkedParameters = owner.linkedParameters; - foreach (var parameter in parameters) { - linkedParameters.Remove(parameter); - } - } - - public ParameterScope(LinqBindingCollection owner, IReadOnlyCollection parameters) - { - this.owner = owner; - this.parameters = parameters; - } - } - public ParameterScope LinkParameters(IReadOnlyCollection parameters) { foreach (var parameter in parameters) { From a77cf70ec376e7f25a2329261582b76b6a3243a2 Mon Sep 17 00:00:00 2001 From: AlexUstinov Date: Wed, 5 Aug 2020 18:41:31 -0700 Subject: [PATCH 5/6] Move SkipTakeRewriterScope structure to the top of the SkipTakeRewriterState class declaration --- .../Internals/SkipTakeRewriterState.cs | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/Orm/Xtensive.Orm/Orm/Rse/Transformation/Internals/SkipTakeRewriterState.cs b/Orm/Xtensive.Orm/Orm/Rse/Transformation/Internals/SkipTakeRewriterState.cs index 6d89be04ec..50c04ee0d4 100644 --- a/Orm/Xtensive.Orm/Orm/Rse/Transformation/Internals/SkipTakeRewriterState.cs +++ b/Orm/Xtensive.Orm/Orm/Rse/Transformation/Internals/SkipTakeRewriterState.cs @@ -10,6 +10,20 @@ namespace Xtensive.Orm.Rse.Transformation { internal sealed class SkipTakeRewriterState { + internal readonly ref struct SkipTakeRewriterScope + { + private readonly SkipTakeRewriter rewriter; + private readonly SkipTakeRewriterState prevState; + + public void Dispose() => rewriter.State = prevState; + + public SkipTakeRewriterScope(SkipTakeRewriter rewriter, SkipTakeRewriterState prevState) + { + this.rewriter = rewriter; + this.prevState = prevState; + } + } + private readonly SkipTakeRewriter rewriter; public Func Skip { get; private set; } @@ -54,20 +68,6 @@ private static Func PositiveSelector(Func valueSelector) }; } - internal readonly ref struct SkipTakeRewriterScope - { - private readonly SkipTakeRewriter rewriter; - private readonly SkipTakeRewriterState prevState; - - public void Dispose() => rewriter.State = prevState; - - public SkipTakeRewriterScope(SkipTakeRewriter rewriter, SkipTakeRewriterState prevState) - { - this.rewriter = rewriter; - this.prevState = prevState; - } - } - public SkipTakeRewriterScope CreateScope() { var currentState = rewriter.State; From a07cd6090887b03e9dc5775aa8a6f7b046f65004 Mon Sep 17 00:00:00 2001 From: AlexUstinov Date: Wed, 5 Aug 2020 18:45:35 -0700 Subject: [PATCH 6/6] Change copyright comment of the affected files --- Orm/Xtensive.Orm/Collections/BindingCollection.cs | 6 +++--- Orm/Xtensive.Orm/Orm/Linq/LinqBindingCollection.cs | 6 +++--- Orm/Xtensive.Orm/Orm/Linq/Translator.Queryable.cs | 6 +++--- .../Rse/Transformation/Internals/SkipTakeRewriterState.cs | 6 +++--- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/Orm/Xtensive.Orm/Collections/BindingCollection.cs b/Orm/Xtensive.Orm/Collections/BindingCollection.cs index 0ae40098ac..75ac6952fb 100644 --- a/Orm/Xtensive.Orm/Collections/BindingCollection.cs +++ b/Orm/Xtensive.Orm/Collections/BindingCollection.cs @@ -1,6 +1,6 @@ -// Copyright (C) 2003-2010 Xtensive LLC. -// All rights reserved. -// For conditions of distribution and use, see license. +// Copyright (C) 2009-2020 Xtensive LLC. +// This code is distributed under MIT license terms. +// See the License.txt file in the project root for more information. // Created by: Alexis Kochetov // Created: 2009.03.12 diff --git a/Orm/Xtensive.Orm/Orm/Linq/LinqBindingCollection.cs b/Orm/Xtensive.Orm/Orm/Linq/LinqBindingCollection.cs index ef421afadd..dea1ce0101 100644 --- a/Orm/Xtensive.Orm/Orm/Linq/LinqBindingCollection.cs +++ b/Orm/Xtensive.Orm/Orm/Linq/LinqBindingCollection.cs @@ -1,6 +1,6 @@ -// Copyright (C) 2003-2010 Xtensive LLC. -// All rights reserved. -// For conditions of distribution and use, see license. +// Copyright (C) 2009-2020 Xtensive LLC. +// This code is distributed under MIT license terms. +// See the License.txt file in the project root for more information. // Created by: Alexey Gamzov // Created: 2009.06.30 diff --git a/Orm/Xtensive.Orm/Orm/Linq/Translator.Queryable.cs b/Orm/Xtensive.Orm/Orm/Linq/Translator.Queryable.cs index d41803aa3b..f48379db56 100644 --- a/Orm/Xtensive.Orm/Orm/Linq/Translator.Queryable.cs +++ b/Orm/Xtensive.Orm/Orm/Linq/Translator.Queryable.cs @@ -1,6 +1,6 @@ -// Copyright (C) 2003-2010 Xtensive LLC. -// All rights reserved. -// For conditions of distribution and use, see license. +// Copyright (C) 2009-2020 Xtensive LLC. +// This code is distributed under MIT license terms. +// See the License.txt file in the project root for more information. // Created by: Alexis Kochetov // Created: 2009.02.27 diff --git a/Orm/Xtensive.Orm/Orm/Rse/Transformation/Internals/SkipTakeRewriterState.cs b/Orm/Xtensive.Orm/Orm/Rse/Transformation/Internals/SkipTakeRewriterState.cs index 50c04ee0d4..48e928c652 100644 --- a/Orm/Xtensive.Orm/Orm/Rse/Transformation/Internals/SkipTakeRewriterState.cs +++ b/Orm/Xtensive.Orm/Orm/Rse/Transformation/Internals/SkipTakeRewriterState.cs @@ -1,6 +1,6 @@ -// Copyright (C) 2010 Xtensive LLC. -// All rights reserved. -// For conditions of distribution and use, see license. +// Copyright (C) 2010-2020 Xtensive LLC. +// This code is distributed under MIT license terms. +// See the License.txt file in the project root for more information. // Created by: Alexey Gamzov // Created: 2010.01.21