4

I'm working on an application that does lots of encryption and decryption in-application and this is probably the number-one bottleneck, so I've been spending some time making performance tweaks to it. A lot of this has involved simply caching things in memory (I realize there is something of a security tradeoff in doing that), but I noticed during profiling that Dispose() was a fair amount of the time doing decryption (I believe for .NET cryptography stuff it zeroes over everything so this makes sense). So I came up with this idea:

Have a "dispose pool." Instead of using blocks, create objects, use them, return the result, and add them to the dispose pool in the finally block. Internally, the dispose pool uses a queue and a timer and every time the timer fires it dequeues the objects and disposes them.

I tried implementing this and it seems to work and improve performance, but then again, profiling it locally is not a really realistic use case. Is this sound? Am I likely to run into runaway performance issues I'm not currently thinking about?

I suppose I should add that this is an ASP.NET MVC application so everything revolves around requests.

  • 1
    What would be the benefit of a dispose pool over a regular object pool? Wouldn't it be more straightforward to just reuse objects rather than cleaning them up so frequently? – M. Dudley Jul 21 '14 at 17:01
  • @M.Dudley Yeah, but it's not really an option because each tenant has its own key and then each record has its own salt, so the decryptors are only useful for a single record. So you know, if you have 100 records, 100 instantiate-use-dispose cycles, which is what I'm trying to improve on. – Casey Jul 21 '14 at 17:03
  • 1
    Are you using the .NET4.5 ServerGC? It is a big improvement upon the previous variants of the GC. blogs.msdn.com/b/dotnet/archive/2012/07/20/… – Patrick Jul 21 '14 at 18:16
  • 1
    I think your "dispose pool" idea is clever, but before you get too wedded to it, do you have a way to load the system with a heavy request load? If you can profile simulating it running at a large scale, you would feel more at ease about the scalability of the concept. – Mike Supports Monica Jul 21 '14 at 19:26
  • 1
    Could there be specific security concerns in this case if you don't clear keep cryptography data and results around in order to recycle the memory? – dbc Jul 25 '14 at 11:17
2

You might take a look at Microsoft's ScheduledDisposable. I've never used it, but it looks as though it will queue your objects for disposal on a separate thread.

But if a pool is what you're looking for, I think this will work:

public interface IDisposableWrapper<TDisposable> : IDisposable where TDisposable : class, IDisposable
{
    TDisposable Reference { get; }
}

public interface IDisposableWrapperFactory<TDisposable> where TDisposable : class, IDisposable
{
    IDisposableWrapper<TDisposable> Create();
}

public sealed class ReusableDisposableFactory<TDisposable> : IDisposableWrapperFactory<TDisposable>, IDisposable
    where TDisposable : class, IDisposable
{
    readonly object padlock = new object();
    Func<TDisposable> getReference;
    Stack<TDisposable> stack;
    int capacity;

    public ReusableDisposableFactory(Func<TDisposable> getReference, int capacity)
    {
        if (getReference == null)
            throw new ArgumentNullException("getReference");
        this.stack = new Stack<TDisposable>(capacity);
        this.capacity = capacity;
        this.getReference = getReference;
    }

    bool IsDisposed { get { return stack == null; } }

    void ThrowOnDisposed()
    {
        if (IsDisposed)
            throw new ObjectDisposedException(GetType().Name);
    }

    sealed class ReusableDisposableWrapper : IDisposableWrapper<TDisposable>
    {
        ReusableDisposableFactory<TDisposable> factory;
        TDisposable reference;

        internal ReusableDisposableWrapper(ReusableDisposableFactory<TDisposable> factory, TDisposable reference)
        {
            if (factory == null)
                throw new ArgumentNullException("factory");
            this.factory = factory;
            this.reference = reference;
        }

        public bool IsDisposed { get { return reference == null; } }

        #region IDisposableWrapper<TDisposable> Members

        public TDisposable Reference
        {
            get { return reference; }
            private set { reference = value; }
        }

        #endregion

        #region IDisposable Members

        public void Dispose()
        {
            // Dispose of unmanaged resources.
            Dispose(true);
            // Suppress finalization.  Since this class actually has no finalizer, this does nothing.
            GC.SuppressFinalize(this);
        }

        void Dispose(bool disposing)
        {
            if (disposing)
            {
                // Free any other managed objects here.
                var reference = Interlocked.Exchange(ref this.reference, null);
                if (reference != null)
                    factory.DisposeReference(reference);
            }
            // Free any unmanaged objects here. 
        }

        #endregion

        public override string ToString()
        {
            var theReference = Reference;
            if (IsDisposed || theReference == null)
                return base.ToString() + ": Disposed";
            else
                return base.ToString() + ": " + theReference.ToString();
        }
    }

    #region IDisposableWrapperFactory<TDisposable> Members

    public IDisposableWrapper<TDisposable> Create()
    {
        lock (padlock)
        {
            ThrowOnDisposed();
            TDisposable reference;
            if (stack.Count > 0)
            {
                reference = stack.Pop();
            }
            else
            {
                reference = getReference();
            }
            return new ReusableDisposableWrapper(this, reference);
        }
    }

    void DisposeReference(TDisposable reference)
    {
        lock (padlock)
        {
            if (reference == null)
                return;
            ThrowOnDisposed();
            if (stack.Count < capacity)
            {
                stack.Push(reference);
            }
            else
            {
                reference.Dispose();
            }
        }
    }

    #endregion

    #region IDisposable Members

    public void Dispose()
    {
        // Dispose of unmanaged resources.
        Dispose(true);
        // Suppress finalization.  Since this class actually has no finalizer, this does nothing.
        GC.SuppressFinalize(this);
    }

    void Dispose(bool disposing)
    {
        if (disposing)
        {
            lock (padlock)
            {
                if (!IsDisposed)
                {
                    while (stack.Count > 0)
                    {
                        var reference = stack.Pop();
                        reference.Dispose();
                    }
                    stack = null;
                    getReference = null;
                }
            }
        }
        // Free any unmanaged objects here. 
    }

    #endregion

    public override string ToString()
    {
        string str = base.ToString();
        if (!Monitor.TryEnter(padlock))
        {
            // Don't block for ToString()
            str = str + ", locked.";
        }
        else
        {
            try
            {
                if (IsDisposed)
                    str = str + ", Disposed";
                else
                    str = string.Format("{0}: {1} {2} cached", str, stack.Count, typeof(TDisposable).Name);
            }
            finally
            {
                Monitor.Exit(padlock);
            }
        }
        return str;
    }
}

Note the factory itself is disposable. I certainly would be reluctant to use this for finalizable objects however.

  • I'll have to look at ScheduledDisposable more; I didn't know that existed. Anyway, the way you've chosen to implement this is pretty different than mine, but it's an interesting perspective. At its core my implementation is using a Queue<IDisposable> and clients just add to the "pool" (maybe this is the wrong term) and the pool disposes on its own (with a timer -- when it fires it dequeues everything and disposes it). – Casey Jul 26 '14 at 1:09
1

maybe you want a collector that regularly runs and when it does, it cleans up these .. how can we call them.. I know - 'garbage' objects. We can call this thing a garbage collector.

Dispose was added to provide deterministic finalisation so you can have objects clean up immediately rather than waiting for later, making a non-deterministic Dispose is just redundant. There is no need to reinvent the wheel.

So, you want finalisation that occurs at a later time, then just use a finaliser. The GC will run that when load is lower. Chances are you don't want that as you do want your dead objects to be memory-wiped immediately, to prevent the security risk of something reading the contents later, probably important in a security application.

Performance will be the same overall - you are simply shifting the place where the cleanup occurs from 'right now' to 'a bit later'. Usually this means the user won't notice the time spent cleaning up but in a heavily used system there's no way to avoid it.

  • OK, and how do you propose I change the finalizers for classes that are part of the .NET framework exactly? – Casey Jul 26 '14 at 16:54
  • You don't, but you can wrap the class in your own object and call Dispose in the finaliser. After all, isn't that the pattern you use if your object doesn't get Disposed directly or in a using block. – gbjbaanb Jul 26 '14 at 20:44
  • 2
    Be sure to read everything Eric Lippert says about finalizers in 2015. – rwong May 22 '15 at 11:52
  • @rwong crikey. Finalizers are worse than I thought - and I thought they were pretty bad. – gbjbaanb May 22 '15 at 12:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.