Does registering unregistering to a delegate create garbage?

Im doing this and im seeing very small amounts of garbage being generated.

i have 2 action delegates.

    private static Action<MgStringBuilder, Keys> OnCharacterKeysPressed;
    private static Action<Keys, Keys, Keys> OnControlKeysPressed;

    public static void AddMethodToRaiseOnTextInput(Action<MgStringBuilder, Keys> textInputMethodToCall)
        {
            OnCharacterKeysPressed += textInputMethodToCall;
        }
    public static void RemoveMethodToRaiseOnTextInput(Action<MgStringBuilder, Keys> textInputMethodToCall)
        {
            OnCharacterKeysPressed -= textInputMethodToCall;
        }


    //Registering or Unregistering depending on if the item in question has focus.

It looks like when i register or un-register a method as above the delegate is creating collectable garbage ? Now underneath this is called by textinput so question is.

Is that a side effect of just using delegates under c# ? Or is that somehow on my side ?

Yes it is a extremely small amount and i have to click a few times to even see it. However it is still sort of annoying enough that i might rethink this if that is the case.

I thought this was an interesting question and looked into it a bit.
No clear answers to report on my own yet, but I did come across this that seems to line up with what you’ve noticed:
https://jacksondunstan.com/articles/3264

Ill have to test this later myself when i get a chance.

From reading that it indicates it probably is the case. Though im not sure i like the way he is testing im more interested in memory de-allocations.

I can think of a way around it but it’s a ugly thought. I suppose i could keep a list of delegates and replace the concept of registering unregistering via registering a index or struct identifyer of some type to match a preregistered method in the list, but ya i don’t really like that idea at all.

That gets ugly when i want to have multiple things called back at once. It basically means pooling delegates and a firing list and only registering methods to them never un-registering methods except at controlled times when required and then letting the gc clean up.

So basically its write a pooling delegate class.

Ya im guessing the delegates are using a linked list of some sort in the true sense that you cant just clear obviously to pool with them.

The += operation for Events creates a new immutable list of delegates. The reason is that way it’s thread safe.
I suppose the += operation for delegates works similarly (after all, events are basically delegates).

And I don’t really know what is going on with Actions, it will probably create an object if you add an Action delegate that uses local parameters.

Using pure delegates doesn’t generate garbage. Like in:
myObject.CallbackMethod = this.myCallbackMethod;

I guess you will have to implement your own custom list of delegates by overriding the Add/Remove operations if that’s possible.

Delegate registration is syntactic sugar in C#. Pass your code through ILSpy and you’ll see it’s transformed to something like this:

Program.OnControlKeysPressed = (Action<Keys, Keys, Keys>)Delegate.Combine(Program.OnControlKeysPressed, new Action<Keys, Keys, Keys>(Program.textInputMethodToCall));

notice the “new Action ()” at the end of the line. That’s what generates garbage.

The trick is creating that new Action() beforehand and assigning into a variable, then passing that variable to the registration. That will avoid the garbage when registering multiple times. I think it’s called delegate caching.

1 Like

And I don’t really know what is going on with Actions, it will probably
create an object if you add an Action delegate that uses local
parameters.

Action and Func are just overloads around custom delegates. They just save the time of writing out the extra stuff. So if it applys to delegates it applys to these as well.

notice the “new Action ()” at the end of the line. That’s what generates garbage.

That will do it un-register and that memory will end up collected.

Ill need some ugly strategy to get around that.

It’s not that ugly, it’s just caching the delegate. It’s been being known and used since the XNA early days.

I found the document I learned it from: http://www.shawnhargreaves.com/blog/delegates-events-and-garbage.html . 12 years old but still actual :slight_smile:

Except for the key quote in that blog post which is the primary concern…

I am not aware of any way to subscribe to an event without causing
memory allocations.

Which is a even worse implication for unsubscribing.

what i mean when i say a ugly strategy.

you’ll be fine as long as you can hook everything up ahead of
time while loading your levels.

What is left un-said also includes,

unloading your levels
or unsubscribing components
or unsubscribing items

and reloading resubscribing them.

I see, however as I said that text is 12 years old, C# has improved a lot since, some of the information there may not be accurate anymore. I was linking it as an example on how to register/unregister without garbage,

I think nowadays you can also cache the new EventHandler (myEventHandlerFunc) as you can cache the delegates but TBH I’ve never done it. The event problems come more when calling the event, where you must create an xxEventArgs object which indeed generates garbage. Sometimes you can fix that with a xxEventArgs object pool though.

As the class you posted your question for is static I supposed that it was a long lived object, so in this particular example (AddMethodToRaiseOnTextInput) you should never have garbage until you leave the program. However for objects you’re mentioning (components, items) you may have a hard time trying to get rid of them even if pooling them, it all depens on how you use the subscription.

Personally I’m using the delegate caching a lot but only for realtime opeartions. It’s a must in BepuPhysics1 to avoid generating garbage, specially taxing in raycast operations (lots of per-frame operations). However I decided to forget about events/delegates garbage on operations like loading/unloading levels. For me the impact of some extra garbage when changing level is negligible when you maybe have to load 25 new models and textures, but this is my particular view and every project/programmer is different :slight_smile:

Im going to try to make a pooling pattern that just adds a layer of use onto registering so the fundamental idea is no longer register unregister but instead

just register once then
turnOn(this) turnOff(this)

ill obviously lose the generic ability of the thing but thats ok.

I was looking for something like this when researching. Thanks for the detail!

So here is what i came up with.

Edit improved tested no garbage.

Pattern changes here as follows…
Each thing registers once.
Everything registers to the single method.

Each thing can turn its notifications on or off thru the disable enable calls.

However 2 problems 1) as stated the delegates have to be wrapped so i have to make one of these classes for each delegate type (that sucks but fine) unless i wanted to try to make the whole thing generic which just nooo. It could be optimized better as well don’t care for the moment.

Edit 3
Changed this to make improvements.

// ....
// ....
//
//TestClass t;
//public Game1() : base()
//{
//    //,,,
//    t = new TestClass();
//}
//protected override void Initialize()
//{
//    ActionTextInputPoolingList.Initialize(Window);
//    base.Initialize();
//}
//protected override void Update(GameTime gameTime)
//{
//    if (GamePad.GetState(PlayerIndex.One).Buttons.Back == ButtonState.Pressed || Keyboard.GetState().IsKeyDown(Keys.Escape))Exit();
//    
//    t.Update(gameTime);
//    base.Update(gameTime);
//}
//protected override void Draw(GameTime gameTime)
//{
//    GraphicsDevice.Clear(Color.CornflowerBlue);
//    spriteBatch.Begin();
//    spriteBatch.DrawString(spriteFont, test.sbMsg, new Vector2(100, 100), Color.Wheat);
//    test.Draw(spriteBatch, spriteFont, test.sbMsg, new Vector2(200, 100), Color.Wheat, gameTime);
//    spriteBatch.End();
//    base.Draw(gameTime);
//}

public class TestClass
{
    public StringBuilder sbMsg = new StringBuilder(64);

    public TestClass()
    {
        ActionTextInputPoolingList.InitialRegistration(CallBackForTest, this);
        var dummyTest2 = new TestClassB();
    }

    public void CallBackForTest(char s, Keys k)
    {
        sbMsg.Clear();
        sbMsg.Append(" CallBackForTest ");
        sbMsg.Append(s);
    }

    public void Update(GameTime gameTime)
    {
        // over and over.
        if (Keyboard.GetState().IsKeyDown(Keys.T))
        {
            ActionTextInputPoolingList.Disable(this);
            ActionTextInputPoolingList.Enable(this);
        }

        if (Keyboard.GetState().IsKeyDown(Keys.D1))
            ActionTextInputPoolingList.Enable(this);
        if (Keyboard.GetState().IsKeyDown(Keys.D2))
            ActionTextInputPoolingList.Disable(this);

        if (Keyboard.GetState().IsKeyDown(Keys.R))
            ActionTextInputPoolingList.InitialRegistration(CallBackForTest, this);
        if (Keyboard.GetState().IsKeyDown(Keys.U))
            ActionTextInputPoolingList.UnRegister(CallBackForTest, this);
    }
    public void Draw(SpriteBatch spriteBatch, SpriteFont spriteFont, Vector2 pos, Color color, GameTime gameTime)
    {
        spriteBatch.DrawString(spriteFont, sbMsg, pos, color);
    }
}

// disconnected class test.
public class TestClassB
{
    public StringBuilder sbMsg = new StringBuilder(64);

    public TestClassB()
    {
        ActionTextInputPoolingList.InitialRegistration(CallBackForTestB, this);
    }
    public void CallBackForTestB(char s, Keys k)
    {
        sbMsg.Clear();
        sbMsg.Append(" CallBackForTestB ");
        sbMsg.Append(s);
    }
    public void Draw(SpriteBatch spriteBatch, SpriteFont spriteFont, Vector2 pos, Color color, GameTime gameTime)
    {
        spriteBatch.DrawString(spriteFont, sbMsg, pos, color);
    }
}

public static class ActionTextInputPoolingList
{
    static readonly object accessLock = new object();

    static List<Action<char, Keys>> actionList = new List<Action<char, Keys>>();
    static List<ActionListTrackingAndMapping> actionListMap = new List<ActionListTrackingAndMapping>();

    public static void Initialize(GameWindow window)
    {
        window.TextInput += TextInputCallBack;
    }

    private static void TextInputCallBack(object sender, TextInputEventArgs args)
    {
        for (int forindex = 0; forindex < actionListMap.Count; forindex++)
        {
            if (actionListMap[forindex].actionIsEnabled)
                actionList[forindex]?.Invoke(args.Character, args.Key);
        }
    }

    public static void InitialRegistration<T>(Action<char, Keys> methodToCall, T inst)
    {
        try
        {
            Monitor.Enter(accessLock);
            InitialRegistration(methodToCall, inst.GetHashCode());
        }
        catch (Exception e)
        { 
            Console.WriteLine(e.ToString());
        }
    }

    public static void UnRegister<T>(Action<char, Keys> methodToCall, T inst)
    {
        try
        {
            Monitor.Enter(accessLock);
            UnRegister(methodToCall, inst.GetHashCode());
        }
        catch (Exception e)
        {
            Console.WriteLine(e.ToString());
        }
    }
    public static void Enable<T>(T inst)
    {
        EnableCallback(inst.GetHashCode());
    }
    public static void Disable<T>(T inst)
    {
        DisableCallback(inst.GetHashCode());
    }

    private static void InitialRegistration(Action<char, Keys> methodToCall, int id)
    {
        bool match = false;
        int index = actionListMap.Count;
        int theid = id;
        bool isEnabled = false;
        for (int forindex = 0; forindex < actionListMap.Count; forindex++)
        {
            var m = actionListMap[forindex];
            if (m.actionID == id)
            {
                match = true;
                index = forindex;
                isEnabled = m.actionIsEnabled;
                forindex = actionListMap.Count; // break
            }
        }
        if (match == false)
        {
            actionList.Add(methodToCall);
            actionListMap.Add(new ActionListTrackingAndMapping(id, index, true));
        }
    }

    private static void UnRegister(Action<char, Keys> methodToCall, int id)
    {
        bool match = false;
        int index = actionListMap.Count;
        int theid = id;
        bool isEnabled = false;
        for (int forindex = 0; forindex < actionListMap.Count; forindex++)
        {
            var m = actionListMap[forindex];
            if (m.actionID == id)
            {
                match = true;
                index = forindex;
                isEnabled = m.actionIsEnabled;
                forindex = actionListMap.Count; // break
            }
        }
        if (match == true) // its not in the list already user error throw msg.
        {
            actionList.Remove(methodToCall);
            actionListMap.Remove(new ActionListTrackingAndMapping(id, index, true));
        }
    }

    private static void EnableCallback(int id)
    {
        for (int forindex = 0; forindex < actionListMap.Count; forindex++)
        {
            if (actionListMap[forindex].actionID == id)
            {
                actionListMap[forindex] = new ActionListTrackingAndMapping(id, forindex, true);
                forindex = actionListMap.Count; // break
            }
        }
    }

    private static void DisableCallback(int id)
    {
        for (int forindex = 0; forindex < actionListMap.Count; forindex++)
        {
            if (actionListMap[forindex].actionID == id)
            {
                actionListMap[forindex] = new ActionListTrackingAndMapping(id, forindex, false);
                forindex = actionListMap.Count; // break
            }
        }
    }

    public struct ActionListTrackingAndMapping
    {
        public int actionID { get; set; }
        public int actionIndexToListForId;
        public bool actionIsEnabled;

        public ActionListTrackingAndMapping(int id, int index, bool alive)
        {
            actionID = id;
            actionIndexToListForId = index;
            actionIsEnabled = alive;
        }
    }
}

}

ok there we go lot of work to make that work.

It’s possible to create your own event handler that is simply a list of a specific type of delegate that invokes all delegates in the list. You can set an initial capacity for the list and cache the delegates added to it so it doesn’t allocate more memory as long as you don’t overshoot the capacity of the list. Rough example:

public class DelegateInvoker
{
    public delegate void MyDelegate();
    public List<MyDelegate> Listeners = new List<MyDelegate>(10);

    public void AddListener(MyDelegate del)
    {
        Listeners.Add(del);
    }

    public void RemoveListener(MyDelegate del)
    {
        Listeners.Remove(del);
    }

    public void Invoke()
    {
        for (int i = 0; i < Listeners.Count; i++)
            Listeners[i].Invoke();
    }
}

You can refine this further, but this is just an outline. You gain more control over garbage at the cost of flexibility. It’s not as nice and doesn’t have as many safety nets built in as events (Ex. thread-safety), but this is one way to do this.

It’s ok if the list grows added garbage isn’t added pressure. It’s de-allocations that create garbage.

While allocations is what you look for to see find deallocations. It’s not actually the former which causes stutters and such.

The implementation of MulticastDelegate.Combine resizes the internal array and returns a new MulticastDelegate when adding or removing elements. This doesn’t happen with the list approach. As a result, you’d have fewer allocations and deallocations with the list - even fewer if the delegates added to the list are cached.

1 Like

Won’t that prevent objects from been collected?
Is it possible to use WeakRefs to store delegates?

I am pretty sure it’s the other way around.

Additionally you’d need to lock/sync the addition/removal of delegates from different threads since your list is no longer immutable like with MulticastDelegate.Combine().

Won’t that prevent objects from been collected?

Yes exactly till i want to release them on my terms. More specifically the objects will pool, I just threw it together. You need a un-register method that the class instance calls specifically that initially registered it. I just figured that was a given. In this case the intent is that the responsibility for that is on the destination object not the source and the source responsibiliy has changed to hold onto the reference to the destination since it can properly add and remove it without causing collections.

Is it possible to use WeakRefs to store delegates?

No clue but the problem here is the collection of objects namely unregistered object method references right. So then this is surely a strong reference now from the source to destination with the idea of registering once and un-registering once and in-between just changing the source calling order thru enable disable from the destination. So while action delegates are multi cast im purposely using them single cast so i can track them with that small struct that goes with it.

It's ok if the list grows added garbage isn't added pressure. It's de-allocations that create garbage.

I am pretty sure it’s the other way around.

Ya sorry i was tired, lol that really is a backwards statement thanks. All increased memory is garbage. This should be insignificant pressure. The rest is about right, de-allocations create collections.

Additionally you'd need to lock/sync the addition/removal of delegates from different threads since your list is no longer immutable like with MulticastDelegate.Combine().

Didn’t think about that, but i don’t think in this particular case it will matter though.

I edited the prior class and was testing it more today till i got it working no garbage, unless you register unregister but under this you don’t have to but one time each.

Then enable or disable instead of registering unregistering.

I edited my code post above. Is this the right way to lock this ?
I have very little experience with setting up my own thread safety.

.
.

Well i got it working it can do everything the multicast did and do it without garbage collections.
Will it build up more memory then otherwise sure but its a trivial amount.
Console.Writeline is generating collectable garbage though not the class so i drew to screen instead.