Memory leak in Balder.Execution.Property<T,TP>

Jan 12, 2011 at 10:15 PM

I'm using Balder for an application that regularly adds and removes geometries from the scene.  I've been watching the memory usage and it steadily goes up as I use the app, even if I clear out the scene completely.

I decided to use SciTech's .NET Memory Profiler to track down what objects weren't being garbage collected.  I took memory snapshots before adding a geometry to the scene, after adding it, and after removing it and running garbage collection.  What it found was thousands of Balder.Execution.Propertys that weren't being garbage collected.  These were properties for coordinates, vectors, strings, etc.  All the basic types.

Here's my guess at why these aren't being garbage collected.  I'm definitely no expert at this, and I don't know the Property<> class well enough to rebuild it yet, but here's my guess.

In the Initialize method, it registers the dependency property.  The propertyChangedCallback is set to the PropertyChanged method.  Registering this dependency property actually hooks up an event handler.  Because of that, typically a static method is used for the callback, so that the event handler isn't holding a reference to an object instance.  In the Property<> class, PropertyChanged is a private instance method, so the dependency property system now has a reference to a particular instance of Property.  There's no code in the class to release that reference, so garbage collection never happens.

Initially I tried to change this to a static method and follow the trail of compilation errors to work it out, but quickly realized there was a LOT going on in this class and a lot of things depending on it, so I'm hoping you guys have more insight into this.

Thanks!

Jan 12, 2011 at 11:08 PM
Edited Jan 12, 2011 at 11:09 PM

Okay, I just ran through it again and I think I was wrong about Property<> being the issue.  What I found is that ObjectProperty<T> (for any T) is not being released.

Here's a screenshot of my realtime memory inspection:  http://i74.photobucket.com/albums/i263/jwetzel1492/Other/MemoryUsage.jpg

It's showing instances of five of the ObjectProperty types (ObjectProperty<bool>, ObjectProperty<Coordinate>, etc).  Each jump in the number of instances was when I added about 30 geometries to the scene.  What you don't see in the graph is that a few seconds after adding those geometries, I removed them from the scene.  A similar graph for the geometries showed that they were being garbage collected.  Same for the Property<> instances.  But the number of ObjectProperty<> instances never goes down.  They are only created.

Jan 13, 2011 at 6:16 PM

Okay, I've done a little more investigation and what is holding the references to the ObjectProperties is the _objectPropertyBag dictionary in Property<T,TP>.  It adds an entry to the dictionary every time a a new property is registered (for example, when you instantiate a cylinder).  There IS code to remove those dictionary entries in the RemoveObjectProperties method, but it only gets called if the value of the property changes.  There's no way to unregister a property when its owner gets garbage collected.  So even though my cylinders are being garbage collected, the ObjectProperty entries in the dictionary live on.

Jan 13, 2011 at 11:49 PM

Okay, I've got a fix, but it's maybe not optimal yet.

My first thought was, "okay, ObjectProperties need to be released from the dictionary when I remove nodes from the scene.  Let's make Node implement IDisposable, and then each child of Node can make sure that its properties get released when it disposes."  I did this for Node and its children.  Didn't take too long.  And when I profiled it, I did get some memory back when removing nodes.  A lot of their ObjectProperties got collected.

But not all.

So I did a search to see everywhere else that Property was used in Balder and there are a lot of places.  Materials, etc.  I didn't want to have to implement logic to decide when to dispose all of those, so I looked for another option.

I ended up replacing Dictionary with a WeakDictionary.  I made a dictionary that automatically cleans up the ObjectProperties.  It removes them if the object they applied to is no longer alive.

The downside is that cleaning up is an expensive operation, so you don't want to do it too often.  I ended up using a random decision and it performs a clean after adding a value 1/100,000th of the time.  That percentage turned out to be small enough not to noticeably impact my framerate (When I did it 1/1, I dropped from 60ftp to 18fps.) but still often enough that memory usage doesn't climb to 1GB just from adding and removing a single cylinder a few hundred times.

I still don't know that this is the best solution.  Seems to work for now.  If anyone is interested, I can post the code.

Thanks!

Coordinator
Jan 15, 2011 at 10:40 AM

Hi,

thanks for all the input on what the problem is.  I have been debugging it quite a few times to figure it out. With your input, it should be easier to pin-point and come up with a good solution for this. And, as you say, it shouldn't be doing a GC all the time, that will kill performance. 

I'll put it up for investigation and see what I'll find out.

 

Coordinator
Jan 15, 2011 at 10:40 AM
This discussion has been copied to a work item. Click here to go to the work item and continue the discussion.
Jan 16, 2011 at 1:19 AM
bob1492,

My use of Balder has me adding and removing many objects over the course of the applications run.  I would be interested in your solution.  I have noticed a memory problem but I've not been able to figure it out.  I'm going to check out that memory profiler too. Seems that could be useful in other applications I'm working on.  If you’re willing to share that code, I'd appreciate it!

 Thanks for the great work!

Jan 17, 2011 at 8:07 PM

Sure!  Again, this code seems to work for me.  Your mileage may vary.

First class you'll need is Balder.Execution.EquatableWeakReference:

using System;
using System.Net;
using System.Windows;
using System.Windows.Controls;
using System.Windows.Documents;
using System.Windows.Ink;
using System.Windows.Input;
using System.Windows.Media;
using System.Windows.Media.Animation;
using System.Windows.Shapes;

namespace Balder.Execution
{
    public class EquatableWeakReference
    {
        #region Private Members

        private int _targetHashCode;
        private WeakReference _weakReferenceToTarget;

        private void setTarget(object target)
        {
            _targetHashCode = target.GetHashCode();
            _weakReferenceToTarget = new WeakReference(target);
        }

        #endregion

        #region Public Interface

        public EquatableWeakReference(object target)
        {
            setTarget(target);
        }

        public object Target
        {
            get { return _weakReferenceToTarget.Target; }
            set
            {
                setTarget(value);
            }
        }

        public bool IsAlive
        {
            get
            {
                return _weakReferenceToTarget.IsAlive;
            }
        }

        public override int GetHashCode()
        {
            return _targetHashCode;
        }

        public override bool Equals(object obj)
        {
            return _targetHashCode == obj.GetHashCode();
        }

        #endregion
    }
}
Then you'll need Balder.Execution.WeakDictionary:
using System;
using System.Net;
using System.Windows;
using System.Windows.Controls;
using System.Windows.Documents;
using System.Windows.Ink;
using System.Windows.Input;
using System.Windows.Media;
using System.Windows.Media.Animation;
using System.Windows.Shapes;

using System.Collections.Generic;
using System.Windows.Threading;

using Balder.Extensions;

namespace Balder.Execution
{
    public class WeakDictionary<TP>
    {
        private Dictionary<EquatableWeakReferenceObjectProperty<TP>> _propertyBag = new Dictionary<EquatableWeakReferenceObjectProperty<TP>>();
        private List<EquatableWeakReference> _weakReferences = new List<EquatableWeakReference>();
        private int _cleanUpCounter = 0;

        public bool ContainsKey(object obj)
        {
            EquatableWeakReference newRef = new EquatableWeakReference(obj);

            if (!_propertyBag.ContainsKey(newRef))
            { return false; }

            return true;
        }

        public void Remove(object obj)
        {
            if (ContainsKey(obj))
            {
                EquatableWeakReference key = new EquatableWeakReference(obj);
                _propertyBag.Remove(key);
                _weakReferences.Remove(key);
            }
        }

        public ObjectProperty<TP> this[object obj]
        {
            get
            {
                return _propertyBag[new EquatableWeakReference(obj)];
            }
            set
            {
                EquatableWeakReference key = new EquatableWeakReference(obj);
                if (!ContainsKey(key))
                { _weakReferences.Add(key); }

                _propertyBag[key] = value;

                _cleanUpCounter++;
                if (_cleanUpCounter > 50000)
                {
                    _cleanUpCounter = 0;
                    cleanUp();
                }
            }
        }

        private void cleanUp()
        {
            GC.Collect();
            GC.WaitForPendingFinalizers();
            GC.Collect();

            List<EquatableWeakReference> toCleanUp = new List<EquatableWeakReference>();

            foreach (EquatableWeakReference weakRef in _weakReferences)
            {
                if (!weakRef.IsAlive)
                {
                    toCleanUp.Add(weakRef);
                }
            }

            foreach (EquatableWeakReference weakRef in toCleanUp)
            {
                _propertyBag.Remove(weakRef);
                _weakReferences.Remove(weakRef);
            }
        }
    }
}
Finally, here's my modified code for Balder.Execution.Property:
#region License
//
// Author: Einar Ingebrigtsen <einar@dolittle.com>
// Copyright (c) 2007-2010, DoLittle Studios
//
// Licensed under the Microsoft Permissive License (Ms-PL), Version 1.1 (the "License")
// you may not use this file except in compliance with the License.
// You may obtain a copy of the license at 
//
//   http://balder.codeplex.com/license
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//
#endregion

using System;
using System.Collections.Generic;
using System.Linq.Expressions;
using Balder.Rendering;
#if(SILVERLIGHT)
using System.ComponentModel;
using System.Windows;
#endif
using Balder.Extensions;

namespace Balder.Execution
{
public class Property<T,TP> : IProperty
#if(SILVERLIGHT)
where T:DependencyObject
#endif
{
private readonly PropertyDescriptor _propertyDescriptor;
private readonly WeakDictionary<TP> _objectPropertyBag;
private readonly TP _defaultValue;
private readonly PropertyValueChanged<TP> _propertyValueChanged;

private Property(Expression<Func<T, TP>> expression, TP defaultValue, PropertyValueChanged<TP> propertyValueChanged)
{
_propertyDescriptor = new PropertyDescriptor(typeof(T), typeof(TP), expression);
_objectPropertyBag = new WeakDictionary<TP>();
_defaultValue = defaultValue;
_propertyValueChanged = propertyValueChanged;
Initialize();
}

#region Register
public static Property<T, TP> Register(Expression<Func<T, TP>> expression)
{
var property = Register(expression, default(TP));
return property;
}

public static Property<T, TP> Register(Expression<Func<T, TP>> expression, PropertyValueChanged<TP> propertyValueChanged)
{
var property = Register(expression, propertyValueChanged, default(TP));
return property;
}

public static Property<T, TP> Register(Expression<Func<T, TP>> expression, TP defaultValue)
{
var property = Register(expression, null, defaultValue);
return property;
}

public static Property<T, TP> Register(Expression<Func<T, TP>> expression, PropertyValueChanged<TP> propertyValueChanged, TP defaultValue)
{
var property = new Property<T, TP>(expression, defaultValue, propertyValueChanged);
return property;
}
#endregion

public TP GetValue(T obj)
{
var objectProperty = GetObjectProperty(obj);
return objectProperty.Value;
}

public void SetValue(T obj, TP value)
{
var objectProperty = GetObjectProperty(obj);

HandleRuntimeContext(obj, objectProperty, value);
var causesChange = objectProperty.DoesValueCauseChange(value);
if (causesChange)
{
objectProperty.SignalRendering();
CleanupChildren(objectProperty.Value);
}
if (objectProperty.CallFromExternal)
{
return;
}

objectProperty.CallFromProperty = true;
if (causesChange)
{
HandleSetValue(obj, objectProperty, value);
}
objectProperty.CallFromProperty = false;
}

public void SetRuntimeContext(object obj, IRuntimeContext runtimeContext)
{
if (null == obj)
{
return;
}
var objectProperty = GetObjectProperty((T)obj);
if (null != objectProperty.RuntimeContext)
{
return;
}
objectProperty.RuntimeContext = runtimeContext;
}

public void RemoveObjectProperties(object obj)
{
            if (_objectPropertyBag.ContainsKey(obj))
            {
                var objectProperty = GetObjectProperty((T)obj);
                CleanupChildren(objectProperty.Value);
                _objectPropertyBag.Remove(obj);
            }
}


        public void CleanupChildren(object previousValue)
        {
            if (null == previousValue)
            {
                return;
            }
            foreach (var childProperty in _propertyDescriptor.ChildProperties)
            {
                childProperty.RemoveObjectProperties(previousValue);
            }
        }


private ObjectProperty<TP> GetObjectProperty(T obj)
{
ObjectProperty<TP> objectProperty;
if( _objectPropertyBag.ContainsKey(obj))
{
objectProperty = _objectPropertyBag[obj];

            else
{
objectProperty = new ObjectProperty<TP>(obj, _propertyDescriptor, _defaultValue, _propertyValueChanged);
_objectPropertyBag[obj] = objectProperty;
}
return objectProperty;
}



private void SetRuntimeContextOnChildren(TP obj, IRuntimeContext runtimeContext)
{
foreachvar childProperty in _propertyDescriptor.ChildProperties )
{
childProperty.SetRuntimeContext(obj, runtimeContext);
}
}


     private void HandleSetValue(T obj, ObjectProperty<TP> objectProperty, TP value)
     {
var oldValue = objectProperty.Value;
if (_propertyDescriptor.IsCopyable && objectProperty.Value != null)
{
((ICopyable)value).CopyTo(objectProperty.Value);
}
else
{
objectProperty.Value = value;
OnSet(obj, value, objectProperty);
}
     HandleNotification(obj, value, oldValue);
     }

     private void HandleRuntimeContext(T obj, ObjectProperty<TP> objectProperty, TP value)
     {
     if (null == objectProperty.RuntimeContext && obj is IHaveRuntimeContext)
     {
     var runtimeContext = ((IHaveRuntimeContext)obj).RuntimeContext;
#if(SILVERLIGHT)
     if (null == runtimeContext && obj is FrameworkElement)
     {
     var frameworkElement = obj as FrameworkElement;
     frameworkElement.Loaded += (s, e) =>
                                 {
                                 runtimeContext = ((IHaveRuntimeContext) obj).RuntimeContext;
                                 SetRuntimeContext(obj, runtimeContext);
                                 if (null != objectProperty.RuntimeContext && !objectProperty.ChildrenRuntimeContextSet)
                                 {
                                 SetRuntimeContextOnChildren(value, objectProperty.RuntimeContext);
                                 objectProperty.ChildrenRuntimeContextSet = true;
                                 }
                                 };
     }
#endif

     if (null != runtimeContext)
     {
     SetRuntimeContext(obj, runtimeContext);
     }
     }

if (null != objectProperty.RuntimeContext && !objectProperty.ChildrenRuntimeContextSet)
{
SetRuntimeContextOnChildren(value, objectProperty.RuntimeContext);
objectProperty.ChildrenRuntimeContextSet = true;
}
     }

     private void HandleNotification(T obj, TP newValue, TP oldValue)
{
if (_propertyDescriptor.CanNotify)
{
((ICanNotifyChanges)obj).Notify(_propertyDescriptor.PropertyInfo.Name, oldValue, newValue);
}
ifnull != _propertyValueChanged )
{
_propertyValueChanged(obj, oldValue, newValue);
}
}


#if(SILVERLIGHT)

public DependencyProperty ActualDependencyProperty { getprivate set; }

private void PropertyChanged(DependencyObject obj, DependencyPropertyChangedEventArgs e)
{
var objectProperty = GetObjectProperty((T)obj);
var oldValue = objectProperty.Value;
var newValue = (TP) e.NewValue;
var causesChange = objectProperty.DoesValueCauseChange(newValue); 
if( causesChange &&
!objectProperty.CallFromProperty)
{
HandleNotification((T)obj,newValue,oldValue);
objectProperty.SignalRendering();
}

objectProperty.Value = newValue;

if (!objectProperty.CallFromProperty)
{
objectProperty.CallFromExternal = true;
_propertyDescriptor.PropertyInfo.SetValue(obj, e.NewValue, null);
objectProperty.CallFromExternal = false;
}
}

private void Initialize()
{
ActualDependencyProperty =
DependencyProperty.Register(
_propertyDescriptor.PropertyInfo.Name,
_propertyDescriptor.PropertyInfo.PropertyType,
_propertyDescriptor.OwnerType,
new PropertyMetadata(_defaultValue, PropertyChanged)
);
}

private void OnSet(T obj, TP value, ObjectProperty<TP> objectProperty)
{
ifnull == Deployment.Current || Deployment.Current.Dispatcher.CheckAccess())
{
obj.SetValue(ActualDependencyProperty, value);
else
{
Deployment.Current.Dispatcher.BeginInvoke(
() => obj.SetValue(ActualDependencyProperty, value));
}
}
#else
private void Initialize()
{

}

private void OnSet(T obj, TP value, ObjectProperty<TP> objectProperty)
{

}

#endif


}
}