0

i know this is a repost of a previous question i asked...

c# wanting multiple ui threads but getting cross-reference errors instead

...but my followup question wasn't answered so i'm posting again for help on the new problem. i'll repeat the intro here. thanks for you indulgence.

i'm still very new at c#, threads and forms. i've written a small data acquistion program. it has two threads: a sensor polling/logging thread and a main UI thread which can chart the sensor data. when the user clicks the "start-logging" button, it continuously polls the sensors (over a virtual COM port), writes the response to a file, updates the main form with some basic polling stats (how many pollings per second). if the user has clicked a "monitor" button, it opens a charting form and the polling thread begininvokes a method that adds the sensors values to the chart. the program works well but i found that if i have multiple charts open (so that i can view multiple sensors in realtime), the chart updates become sporadic or stop and only the window with the focus updates smoothly. (the comm port is only 56kbaud so it's not like the polling is being swamped with data.)

so i got the "bright" idea to make charting threads, thinking this would provide multiple UI loops (so i could interact with each chart) and would produce nice smooth charting on multiple chart forms. below is simplified code; e.g. here, the charting thread is started with the polling thread instead of when the user clicks the "monitor" button.

the problem is that the delegate is never performed. the stats on the main form is being updated nicely, the charting form is displayed, but is unresponsive and i get the "wait" cursor when i mouse it. advice greatly appreciated. thanks.

namespace WindowsFormsApplication2
{
    public partial class Main_Form : Form
    {
        delegate void UpdateUIStatsDelegate(string update);
        UpdateUIStatsDelegate update_stats_delegate;

        static BackgroundWorker polling_thread = new BackgroundWorker();
        static BackgroundWorker charting_thread = new BackgroundWorker();

        public static Chart_Form chart_form;

        public Main_Form()
        {
            Thread.CurrentThread.Name = "main";

            update_stats_delegate = new UpdateUIStatsDelegate(update_stats);

            polling_thread.DoWork += polling_thread_DoWork;
            charting_thread.DoWork += charting_thread_start;
        }

        private void start_polling_Click(object sender, EventArgs e)
        {
            // start polling thread
            polling_thread.RunWorkerAsync();

            // start charting plotting thread
            charting_thread.RunWorkerAsync();
        }

        private void polling_thread_DoWork(object sender, DoWorkEventArgs e)
        {
            string sensor_values;
            Thread.CurrentThread.Name = "polling";

           while (true)
            {
                sensor_values = poll_the_sensors_and_collect_the_responses();
                chart_form.BeginInvoke(chart_form.update_chart_delegate, new object[] { sensor_values });

                pps = compute_polling_performance();
                BeginInvoke(update_stats_delegate, new object[] { pps.ToString("00") });
            }
        }

        private string poll_the_sensors_and_collect_the_responses()
        {
            send_command_to_sensor(sensor_id, command_to_return_current_readings);
            return read_sensor_response(sensor_id);
        }

        private void update_stats(string stat)
        {
            pollings_per_second.Text = stat;
        }

        private void charting_thread_start(object sender, DoWorkEventArgs e)
        {
            Thread.CurrentThread.Name = "charting";
            chart_form = new Chart_Form();
            chart_form.Show();
            while (charting_is_active) { }
        }
    }

    public partial class Chart_Form : Form
    {
        public delegate void UpdateChartDelegate(string sensor_values);
        public UpdateChartDelegate update_chart_delegate;

        public Chart_Form()
        {
            update_chart_delegate = new UpdateChartDelegate(update_chart);
            this.Text = "a realtime plot of sensor values";
        }

        private void update_chart(string sensor_values)
        {
            int x = extract_x_value(sensor_values);
            int y = extract_y_value(sensor_values);

            chart1.Series[X_AXIS].Points.AddY(x);
            chart1.Series[Y_AXIS].Points.AddY(y);
        }
    }
}
Community
  • 1
  • 1
4mla1fn
  • 169
  • 1
  • 15
  • 3
    There is so much wrong with this that it cannot be answered except with a book or a class. If your charts are not updating smoothly, the last thing in the world this means is that they need an independent UI thread. Have you done any performance profiling? Shooting blindly in the dark is not the solution. – J... Jun 20 '13 at 02:28
  • Suggested reading : http://www.albahari.com/threading/ – J... Jun 20 '13 at 02:39
  • Also suggest that you post your previous code as a new question - show how you are updating your charts, where the data is coming from, and ask how you can improve the performance. This approach is really not the solution you want. – J... Jun 20 '13 at 02:42
  • I would be interested to see the contents of `poll_the_sensors_and_collect_the_responses`. Also note that you have re-used a variable name (public static `chart_form` and a local var `chart_form`) - if other sections of your code expect that these are the same chart this can cause obvious problems - like `chart_form.update_chart_delegate` -> this will refer to the static class variable and not the one created by the thread! – J... Jun 20 '13 at 10:48
  • @J: i took your advise on profiling. i got a trial version of dotTrace Performance profiler. i ran it on the version of this program that launches charts from from the main UI thread; not the multi-threaded attempt in the above post. dotTrace shows that when i have one chart form displayed, the `OnPaint` method consumes 90.45% of the time; 138 calls taking 7,600ms; 55ms per call. (http://www.eff1fan.com/profile-one-chart.png) – 4mla1fn Jun 20 '13 at 23:01
  • @J: yup, you're right about the chart_form. this is an error i made in writing the simplified code. this isn't the case in the actual program. – 4mla1fn Jun 20 '13 at 23:04
  • @J: i added `poll_the_sensors_and_collect_the_responses` above. it really is in essence that simple. (the actual method is slightly more complex since makes a request to each sensor on the network and packages up all the responses into one string.) since the sensor is slow to respond, the `read_sensor_response` is a while loop that keeps reading the com port until x bytes are received. – 4mla1fn Jun 21 '13 at 00:03
  • @J: also the charting function `update_chart` is, again, in essence that simple. not shown is that i remove the first point of each axis if i've collected X points (to give a scrolling stripchart effect), and i do some checking in case the sensor returned faulty data. other than that, what is shown is accurate. – 4mla1fn Jun 21 '13 at 00:08

2 Answers2

1

The problem is in your second UI thread. You can not put a infinite loop in a UI thread an expect it to work:

    while (charting_is_active) { }

The UI thread needs to run the windows message queue. My advice is that you create both forms only in the initial UI thread. But if you still want to go with the two threads approach, I think you should do something like:

private void charting_thread_start(object sender, DoWorkEventArgs e)
{
    Thread.CurrentThread.Name = "charting";
    Chart_Form chart_form = new Chart_Form();
    Application.Run(chart_form); 
}
fcuesta
  • 4,429
  • 1
  • 18
  • 13
  • 2
    While you can actually do this and it will "work", it cannot be stressed enough how fundamentally bad an idea this is. I think anyone who sits down and feels compelled to solve whatever programming problem with *this* as the solution needs realize that they are almost certainly doing something else very, very wrong. This is like `DoEvents` being turned loose into the jungle for fifteen years and coming back to town as a feral monster. – J... Jun 20 '13 at 02:33
  • thanks for the response. i was originally creating the chart forms from the initial UI thread (whenever the user clicked the "monitor" button) but this was where i experienced the sporadic updating behavior i'd mentioned. – 4mla1fn Jun 20 '13 at 02:40
  • 2
    @4mla1fn - Ok, but what information led you to believe that it was the charts that were the bottleneck? The funny thing about the UI thread is that no matter what it is busy with, everything will suffer. I can guarantee you that drawing the charts was not what was slowing it down (charts are meant to be drawn - they're UI components, they're meant to run in the UI thread). What you need to do is figure out what actually is loading your UI thread and either move *that* work off to another thread or refactor it to correct the problem. – J... Jun 20 '13 at 02:54
  • @fcuesta - indeed, and it is good advice. I just wanted to be clear on just how good of advice it is. ;) – J... Jun 20 '13 at 02:57
  • @J: the charting is just adding points to the chart and removing the first point when the total points exceeds some value (to give a scrolling graph). i agree, that shouldn't be expensive. so, i'll revert to an earlier version and look closer at what i'm doing and will learn about performance profiling. (i'd be happy to avoid the multiple UI/thread stuff.) thanks. – 4mla1fn Jun 20 '13 at 03:04
  • @4mla1fn I'd have a look at the polling thread - you've put some dummy functions in your sample code, but if that loop is fast you may be hammering the UI thread with millions of `BeginInvoke`s and swamping it. You might try throwing a `Sleep(30)` in that while loop just to see what happens. – J... Jun 20 '13 at 03:11
  • @J: thanks for the suggestion. as background, the sensor i'm polling is quite slow. i'm polling the sensor (i send a command then wait for the response to come back) as fast as i can and the best i can get is ~60 samples per second. this means the BeginInvoke is being hit 60 times a second. this seems slow to me but dunno. (the stats are updated at the same rate, but these are just labels that are updated and are surely less expensive to update than a chart/s.) – 4mla1fn Jun 20 '13 at 03:24
  • @J: sorry, i forgot to add that adding `Thread.Sleep(30)` didn't have a beneficial or diagnostic affect. – 4mla1fn Jun 20 '13 at 04:04
0

To follow up on your dotTrace data : take a close look at those numbers. 138 calls to OnPaint over ~8 seconds (58ms to draw the chart). Also note that you've called BeginInvoke 2630 times! update_logging_stats was handled over 2000 times - your polling thread seems to be running way too fast. It's feeding work to the UI thread faster than your eyes can see or the display can even render.

Since you call update_logging_stats once for every time you've updated the chart, this means that your Windows message queue has accumulated an enormous backlog of paint messages and cannot keep up with them all (this is causing your UI thread to choke). You're simply giving it too much work to do (way more than is necessary). While it is busy drawing the chart, twenty more messages have come in to paint it again. Eventually it ends up trying to service the queue and locks up.

What you may try is something like adding a stopwatch and metering your demands on the chart - only send it an update every 200ms or so :

private void polling_thread_DoWork(object sender, DoWorkEventArgs e)
{
    string sensor_values;
    Thread.CurrentThread.Name = "polling";

    Stopwatch spw = new Stopwatch();
    spw.Restart();

    while (true)
    {
        sensor_values = poll_the_sensors_and_collect_the_responses();
        if (spw.ElapsedMilliseconds > 200)
        {
            chart_form.BeginInvoke(chart_form.update_chart_delegate,
                                                 new object[] { sensor_values });
            spw.Restart();
        }

        pps = compute_polling_performance();
        BeginInvoke(update_stats_delegate, new object[] {pps.ToString("00")});
    }
}

You can still keep all of the data, of course, if you really need it with such resolution - do something else with sensor_values when you are not adding them to the chart (save them to an array, file, etc). You might even consider collecting a number of data points over a span of 200ms or so and then sending a cluster of points to plot at once (rather than trying to replot the whole set a hundred times per second) - again if you are really accumulating data at that speed.

J...
  • 30,968
  • 6
  • 66
  • 143
  • Also, if you find you really do want ridiculous update speeds, it's probably best to ditch the DataVisualization charts from MS and find something more appropriate (the MS charts are not high performance tools!) : http://www.genlogic.com/rt_chart_dotnet.html (for example). Otherwise, if you're spinning off a thread per chart and each one is running flat out then you're going to kill even a fast multicore system pretty quickly. – J... Jun 20 '13 at 23:53
  • @J: thanks. the 2096 calls to `update_logging_stats` is because with each sample, i update four different stats on the gui. (i know i only showed one update in the sample code above. it's hard to know upfront what will be important to show accurately and what to gloss over. sorry.) so the 524 calls to `update_monitor_chart` (which roughly matches the 526 `read_magtrace_response`) times 4 stats updates = 2096. regardless, you make a very good point. i'll disable the stats updating and profile again... – 4mla1fn Jun 21 '13 at 00:25
  • @J: new profile screen capture is here: http://www.eff1fan.com/profile-one-chart-no-update-stats.png i expanded several methods to show their profile. this shows that all the drawing is in service of the chart. i'm "only" wanting to update at ~66hz but that's evidently way too much for the chart control; it's only updating at about 18hz. i'll try your batch update and metering ideas. – 4mla1fn Jun 21 '13 at 00:51
  • 1
    @J: i implemented 25-sample batch updates. wow, what a difference! `OnPaint` time has plummeted to 12% and increases linearly with each additional chart. the graph is a bit jerky, but all the data is displayed. i'll impose a limit of 3 simultaneous charts and reduce the buffer size to smooth out the updates. i'd say this problem is solved! thanks very much J for yours suggestions, links and sticking with me on this! i've learned a ton; profiling is my new weapon! i appreciate it very much. – 4mla1fn Jun 21 '13 at 01:48
  • @4mla1fn Glad it helped. Again, it can't be stressed enough how bloaty the MS charts are - they're really not made for real-time updating. There are a lot of alternatives out there - free and otherwise, if you really want a good looking, smooth, and CPU friendly application. – J... Jun 21 '13 at 10:05
  • @4mla1fn you may also have a look, alternatively, through the Chart properties and make sure you have configured them for fast rendering (ie: turn off soft shadows, reduce or remove antialiasing quality for text and graphics, etc.) – J... Jun 21 '13 at 10:56
  • @J: i wasn't using any decorations. to ensure this, i switched to `SeriesChartType.FastLine` which dropped the `OnPaint` use to 6.5% for a 25-sample batch for a single chart. for grins, i'm made a version using ZedGraph mentioned in the link you provided. ZedGraph's `OnPaint` use is 2.2%. for a single form and 25-sample batch. now we're talking! thanks again. – 4mla1fn Jun 21 '13 at 14:06
  • @4mla1fn - glad to hear it's all coming together. Welcome to stackoverflow, by the way : http://meta.stackexchange.com/a/5235/222049 (not that you should at all accept this answer - in fact it doesn't really answer your question (and fcuesta's technically does), but in any case... );) – J... Jun 21 '13 at 14:23
  • @4mla1fn also, since you're now down to a reasonable CPU load with new charts, etc, you can obviously start trimming your update batch size and increasing the replot rate back to something closer to the real-time feel you're going for. – J... Jun 21 '13 at 14:56