1
\$\begingroup\$

I'm writing a program in Unity and am using a TextMeshProUGUI object (named history) to hold a list of messages to act as a terminal/console.

The problem I am having is that I am printing up to hundreds of lines a second, which can be detrimental to performance when I'm doing many string splits and then updating the text variable of the TextMeshProUGUI object, (which in turn gets rendered) every single frame.

My initial implementation looked like this:

private const int MAX_HISTORY = 64;  // the maximum number of lines to store
public TextMeshProUGUI history;      // value is set in Unity editor
private int inputs = 0;              // the size of the history object

public void Print(params string[] msgs)
{
    // add all messages to the history
    foreach (string msg in msgs)
    {
        if (msg == null)
            continue;
        inputs++;
        history.text += msg + '\n';
    }
    // and now reduce the size
    if (inputs > MAX_HISTORY)
    {
        string[] lines = history.text.Split('\n');
        history.text = "";
        for (int i = inputs - MAX_HISTORY; i < lines.Length; i++)
        {
            history.text += lines[i] + '\n';
        }
        inputs = MAX_HISTORY;
    }
}

and I have managed to reduce it to this:

public void Print(params string[] msgs)
{
    foreach (string msg in msgs)
    {
        if (msg == null)
            continue;
        inputs++;
        history.text += msg + '\n';
    }
    if (inputs > MAX_HISTORY)
    {
        history.text =
            history.text.Split(new char[] { '\n' }, inputs - MAX_HISTORY)[inputs - MAX_HISTORY - 1];
        inputs = MAX_HISTORY;
    }
}

But I am still concerned with the line in the if-statement with the large split function. I would appreciate it if somebody who has experience with C# and string manipulation could review this and tell me if my optimisation is (a) correct and (b) good.

Edit:

Changing the value of TextMeshProUGUI.text causes a layout and vertex recalculation which has poor performance so I've also added a StringBuilder to handle the concatenation and splits before setting the text value.

public void Print(params string[] msgs)
{
    StringBuilder text = new StringBuilder(history.text);
    foreach (string msg in msgs)
    {
        if (msg == null)
            continue;
        inputs++;
        text.AppendLine(msg);
    }
    while (inputs > MAX_HISTORY)
    {
        text.Remove(0, Convert.ToString(text).Split('\n').FirstOrDefault().Length + 1);
        inputs = MAX_HISTORY;
    }
    history.text = text.ToString();
}
\$\endgroup\$
3
  • \$\begingroup\$ @BCdotWEB Then I don't believe you have read the question. The title is exactly what the question states as its main task. It is the optimisation of many string concatenations and then many splits on the same string. If you have a suggestion please let me know because I do not have any other way to word it in simple terms. \$\endgroup\$ Commented Apr 22, 2020 at 11:15
  • \$\begingroup\$ Is it common for msgs count to be greater than MAX_HISTORY? If so maybe you can optimize by not adding msgs to text and then removing them. \$\endgroup\$ Commented Apr 22, 2020 at 18:13
  • \$\begingroup\$ @shanif Yes. It mimics a terminal and after around five commands (because of how much output there is) you hit that limit very fast. \$\endgroup\$ Commented Apr 22, 2020 at 18:46

1 Answer 1

Reset to default
3
\$\begingroup\$

I would use generic list to store messages history and Linq extension methods. If I understood correctly I would append new messages and crop to predefined maximum size. Could you test this solution if it has sufficient performance?

private List<string> _history = new List<string>();

public void Print(params string[] msgs)
{
    _history.AddRange(msgs.Where(m => !string.IsNullOrWhiteSpace(m)));
    _history = _history.Skip(Math.Max(0, _history.Count - MAX_HISTORY)).ToList();
    history.text = string.Join(Environment.NewLine, _history);
}

If you need messages ordered from newest to latest, you can add Reverse() method:

_history = _history.Skip(Math.Max(0, _history.Count - MAX_HISTORY)).Reverse().ToList();
\$\endgroup\$
2
  • \$\begingroup\$ Wow. I was getting an average nanosecond speed of ~400,000 and now it is down to ~110,000-137,000 so thank you very much. Quick question, is there any reason why I would want to use _history = _history.Skip... instead of _history.RemoveRange ? \$\endgroup\$ Commented Apr 22, 2020 at 1:31
  • 1
    \$\begingroup\$ I've just tested with RemoveRange and its 27 times faster when we ignore rendering so I might go with that. Thank you for more than doubling the speed though. The improvement is (visually) very obvious already. I cannot upvote your answer yet but I will when I get the reputation. \$\endgroup\$ Commented Apr 22, 2020 at 1:45

You must log in to answer this question.