Wireshark-dev: Re: [Wireshark-dev] [Wireshark-commits] master 43b77ae: Qt: Add an event process
From: Jeff Morriss <jeff.morriss.ws@xxxxxxxxx>
Date: Thu, 17 Sep 2015 20:21:19 -0400
On 09/17/2015 07:35 PM, Gerald Combs wrote:
On 9/17/15 2:12 PM, Jeff Morriss wrote:
On 09/17/15 16:25, Wireshark code review wrote:
URL:
https://code.wireshark.org/review/gitweb?p=wireshark.git;a=commit;h=43b77aeebf4e23d629bc45be0b10b65c8aa17ac9

Submitter: Gerald Combs (gerald@xxxxxxxxxxxxx)
Changed: branch: master
Repository: wireshark

Commits:

43b77ae by Gerald Combs (gerald@xxxxxxxxxxxxx):

      Qt: Add an event processing timer.

      When updating the progress dialog (which happens each time we read a
      packet) the GTK+ UI processes application events every 100ms. Do the
      same in the Qt UI.

Interesting...  I had been looking briefly at the Qt progress dialog
because I noticed that, when loading a very large capture file, the dialog
doesn't update very smoothly or regularly (this is Qt5 on Linux/X11).

With this 100msec timer it seems to update much more regularly but it only
updates every 2 *seconds* (about 10 updates during a 22-second file load).
I'm guessing it doesn't behave like that on other systems?

The GTK+ and Qt UIs update at the same intervals here for the test captures
I'm using. Out of curiosity can you try changing

     WiresharkApplication::processEvents();
     dlg->elapsed_timer->restart();

to

     dlg->elapsed_timer->restart();
     WiresharkApplication::processEvents();

No, the timer's not the problem. In my case the timer is letting in every single update (all 100 of them) it's just that the screen's only redrawing 10 times. IOW the timer didn't really change anything for me, I just thought it did.

Looking at the processEvents() documentation I (somewhat blindly) tried adding both flush() and sendPostedEvents() calls just to see if those would help but they didn't. processEvents() says it only affects the calling thread--I guess that's not the problem, right?

I did notice that the progress bar only updates every 500ms for one
capture, apparently because the file takes ~50s to load and...

(Also, side note: file.c doesn't call update_progress_dlg() for every
packet; it aims to call that function 100 times while reading a file.)

... 50 / 100 = 0.5. That is, for both GTK+ and Qt our update interval is
the greater of "100ms" or "the time it takes to process 1% of our packets."
Would it make sense to call update_progress_dlg() in file.c based on a
timeout instead of a percentage?

At least at a high level it does seem a bit silly to have both the caller (file.c in this case but there are many other callers) and the called function suppressing updates. You'd think one place or the other would be sufficient. Given that we have a timer in (both versions of) update_progress_dlg() shouldn't that be sufficient? (It would save a lot of cut-n-paste caller-suppression code too...)

Maybe I'll work on cleaning that up this evening...