0

I have a background thread that is reading messages from a device and formatting them and passing them to the 'Consumer' node of a Producer/Consumer collection that then prints all of the messages to the string. The problem I am running into is that the logging lags a little bit behind the end of the process, so I am trying to find a faster way to print to the screen. Here is my consumer method:

private void DisplayMessages(BlockingCollection<string[]> messages)
{
    try
    {
        foreach (var item in messages.GetConsumingEnumerable(_cancellationTokenSource.Token))
        {
            this.Invoke((MethodInvoker)delegate
            {
                outputTextBox.AppendText(String.Join(Environment.NewLine, item) + Environment.NewLine);
            });
        }
    }
    catch (OperationCanceledException)
    {
        //TODO:
    }
}

I have done some benchmark tests on my producer methods, and even logging to the console, and it does appear that writing to this TextBox is what is a little slower. During each process, I am logging ~61,000 lines that are about 60 characters long.

I have researched that it is better to use .AppendText() than to say textBox.Text += newText as this resets the entire TextBox's text. I am looking for a solution that may include a faster way to print to a TB (or a UI element better suited for quick logging?) or if using String.Join(Environment.NewLine, item) is inefficient and could be sped up in any way.

AdamMc331
  • 15,332
  • 9
  • 63
  • 121
  • 2
    use a stringbuilder perhaps instead of joining strings? – user1666620 Sep 14 '15 at 19:55
  • 3
    So you're calling this invoke method 61000 times? You're flooding the message pump with way to many requests. You need to update the UI on a timed based fashion. – Sievajet Sep 14 '15 at 19:56
  • 1
    try doing this with out invoking first. – Daniel A. White Sep 14 '15 at 19:56
  • Just some tip aside: "WordWrap=false" can help if not used already (from: http://stackoverflow.com/a/11276778/265165) – thmshd Sep 14 '15 at 19:59
  • @Sievajet I am looking into that as well (as the UI gets bogged down during the faster parts of the process) but I'm logging up to 10 strings in each array, so it helps to take some chunks out. – AdamMc331 Sep 14 '15 at 20:07
  • @user1666620 In this scenario, I've read that String.Join is faster: http://stackoverflow.com/questions/585860/string-join-vs-stringbuilder-which-is-faster – AdamMc331 Sep 14 '15 at 20:08
  • @thomasjaworski.com I will try this, since it looks like it may help a little bit. – AdamMc331 Sep 14 '15 at 20:09
  • Consider creating your own control that overrides TextBox and only fill in the TextBox with data that is actually showing in the viewable area. When the user scrolls, then you would readjust the data. – Icemanind Sep 14 '15 at 20:18
  • @Icemanind would that cause issues if the user tried to scroll while data was still being logged? – AdamMc331 Sep 14 '15 at 20:21
  • Do you need to be able to look back at all of the items in the log? If so, you need a virtual control. If not, you can keep a list of the most recent ten or twenty lines and just reset the Text in the TextBox when you get a new item. – Lorek Sep 14 '15 at 20:21
  • @Lorek yes, I would like to have all items visible while logging so we can scroll up and review messages without stopping the program though. – AdamMc331 Sep 14 '15 at 20:22
  • @McAdam331 - Since the visible area of the TextBox will only be like 20 lines maybe, plus 10 over or under, it should populate your TextBox instantly. You would have to do some multithreading magic to get it to work and you might want to put a Queue in between receiving the data and outputting it to the TextBox. That is just a precaution to make sure no data is lost. – Icemanind Sep 14 '15 at 20:25
  • @Icemanind it sounds like that would work, but I know that won't be any easy task so it may take a while to integrate but I think it would be really great for this application if I can accomplish it. Thanks for the idea! – AdamMc331 Sep 14 '15 at 20:28
  • Usability aside, you can flatten the collection with SelectMany and assign the resulting string array directly to TextBox.Lines – Panagiotis Kanavos Sep 15 '15 at 16:07
  • I'd rethink UX before going too far down this rabbit hole. I have a hard time imagining a textbox with raw text being pumped into it faster than it can be displayed as being useful to any real end-user. Any time you spend figuring out how to fix the performance issue, is probably better spent figuring out a better way to present the info and circumvent the issue entirely. – lobsterism Sep 15 '15 at 16:30
  • @lobsterism that's a good point, but I am writing a CAN bus logger recording communication between our device and a vehicle, which is happening *very, very, very* fast. Unfortunately, we haven't figured out the best way to display this in a readable way, perhaps I can record all items in a list and display a set amount of items every second so it is readable, and then dump the rest when the user chooses to? I will throw this idea around the other devs. – AdamMc331 Sep 15 '15 at 18:04
  • @McAdam331 I do HW too. Ideas: 1. just log to file and view it in notepad++. 2. Show the last 50 (say) live in a textbox. 3. allow that live view to be paused. 4. have a textbox where you can enter a search string, and only show matching lines. 5. whatever else useful you can think of. All this can be done easily with a List, a lock, and a winforms timer, with no BlockingCollection or Invoke complexity. A plain textbox has rarely been useful to me without some additional functionality. – lobsterism Sep 15 '15 at 18:33
  • 1
    @McAdam331 for 1, add a button to your app to launch the notepad++ process for that file. For the rest, you may not even need the lock or list; it *may* be simpler and easier on your RAM to just read the log file directly with each GUI update http://stackoverflow.com/questions/3817477/simultaneous-read-write-a-file-in-c-sharp. Also as another UX idea, a hex/ascii/"parsed" radio button is nice to have. – lobsterism Sep 15 '15 at 18:55
  • @lobsterism I did something similar to your second comment without N++. I logged everything to a file, and added a timer to update the UI every so often (still determining what timer is best for us). This, in combination with a pause button to view data easily without interrupting logging appears to work great so far. Thanks so much for the advice. – AdamMc331 Sep 16 '15 at 15:57

2 Answers2

3

Code with O(n^2) performance is expected to be slow.

If you can't use String.Join to construct whole output once then consider using list-oriented controls or even grids. If you have very large number of rows most list controls and grids have "virtual" mode where text is actually requested on demand.

Alexei Levenkov
  • 94,391
  • 12
  • 114
  • 159
  • I will look into virtual mode. I don't know the source on this, but a coworker told me they experienced issues with ListBox around 65K items (and I am getting close to it here, many of our flash processes will have well over 65k items) so I did not try that. I will consider a DataGridView. I'm not sure what would make that faster or slower than a TextBox, but I will experiment with it. – AdamMc331 Sep 14 '15 at 20:03
  • If 65K+ lines is your normal case grids would likely be better choice - try and measure (don't limit your search with just DataGridView so). – Alexei Levenkov Sep 14 '15 at 20:10
1

String.Join isn't inefficient. The way it is called though causes it to be called N times (once for each message arrau in the collection) instead of just once. You could avoid this by flattening the collection with SelectMany. SelectMany takes an IEnumerable as input and returns each individual item in it:

var allLines=messages.SelectMany(msg=>msg);
var bigText=String.Join(allLines,Environment.NewLine);

Working with such a big text box though will be very difficult. You should consider using a virtualized grid or listbox, adding filters and search functionality to make it easier for users to find the messages they want.

Another option may be to simply assign the lines to the TextBox.Lines property without creating a string:

var allLines=messages.SelectMany(msg=>msg);
outputTextBox.Lines=allLines.ToArray();
Panagiotis Kanavos
  • 90,087
  • 9
  • 138
  • 171