[prev in list] [next in list] [prev in thread] [next in thread] 

List:       gnuplot-info-beta
Subject:    Re: enhanced_recursion()  and do_event(), can they be redesigned?
From:       Jérôme_Lodewyck <jerome.lodewyck () normalesup ! org>
Date:       2014-02-24 15:48:19
Message-ID: 530B69C3.2080704 () normalesup ! org
[Download RAW message or body]

Hi,

	I got a bit confused about your motivations for introducing an event 
loop in the "inboard" qt terminal. In qt_term.cpp, we use, by design, a 
very limited set of the Qt library, and rather push as much 
functionality as possible the in the gnuplot_qt "outboard" program. And 
I think that this set of functions doesn't require an event loop to work 
properly (As a matter of fact, I can observe by commenting out the 
QCoreApplication declaration in qt_term.cpp that it doesn't even require 
an instantiation of QCoreApplication to work -- except from the Windows 
specific QCoreApplication::applicationDirPath() call). More 
specifically, we use
- Storage classes (QString, QImage, QColor...) that just gather 
information but use no event mechanism.
- QLocalSocket that, according to the Qt documentation, can work without 
an event loop:
"Although QLocalSocket is designed for use with an event loop, it's 
possible to use it without one. In that case, you must use 
waitForConnected(), waitForReadyRead(), waitForBytesWritten(), and 
waitForDisconnected() which blocks until the operation is complete or 
the timeout expires.". In fact, I can imagine that the gnuplot inboard 
driver is perhaps precisely the kind of programs that the Qt developers 
had in mind when they wrote this sentence: a program that cannot run a 
Qt event loop because it implements its own event loop, but that still 
wants to send messages to another auxiliary program that runs a Qt event 
loop to manage a GUI.

So could you point out a specific past or present problem in the Qt 
terminal that is due to the fact that no event loop is running in the 
inboard driver ?
To my knowledge, here are the issues that arose on non Linux platform:
- OSX: in the past, there was no gnuplot_qt independent process, and the 
GUI was rather managed in a thread. As you point out, this is not 
supported in Qt, and while it kind of happened to work with Linux, it 
failed on OSX.
- OSX: currently, the QtGnuplotWidget has a hard time to resize itself 
to the correct size, but it is specific to the gnuplot_qt outboard driver.
- Windows: Watching at the same time the standard input and a 
QLocalSocket file descriptor is not supported on Windows using select() 
or the Qt API (which might use select() under the hood). This issue is 
not related to the EventLoop or not EventLoop question and has been 
recently fixed by specific calls to the win32 API.
- All platforms: waitForConnected() immediatly returns if not socket is 
found (in particular, this happens when the gnuplot_qt program is still 
being initialized), even when a timeout is set. This was solved by 
introducing a custom timeout mechanism, but could more elegantly be 
solved by some communication mechanism between gnuplot ("inboard") and 
gnuplot_qt ("outboard"). I can see that part of your code actually 
implements this.

Concerning QApplication vs. QCoreApplication & font metrics:
Until recently, the inboard driver (qt_term.cpp) used to instantiate a 
QApplication to determine the font metrics (no event loop was required 
for this though, and more generally, no Qt event loop, nor Qt event 
based mechanism has ever been running in the main gnuplot thread). As 
Ethan said, this mechanism has been moved to the "outboard" gnuplot_qt 
program, and now the inboard driver only instantiate a QCoreApplication 
(which, as I said above, even turns out not to be necessary in practice, 
although the Qt documentation does not certify this point). The 
motivation for this was a significant performance increase: the 
initialization of a QApplication requires more than 0.5 s against a few 
10ms for a QCoreApplication. Moving back the font metric business in the 
main gnuplot program will require to instantiate again a QApplication 
and thus cancel this performance improvement.

Concerning the code you propose: apart from the synchronization problems 
between the Qt event loop and the main thread that you mention, I am 
worried about the performances. As far as I can see, the communication 
between the main thread and the thread running the Qt event loop relies 
on the signal/slot mechanism, which I think can be significantly slower 
that direct function calls. From rough testing, plotting a graph with a 
million points takes about 3x more time with the code you posted.

Jérôme



Le 24/02/2014 08:10, Daniel J Sebald a écrit :
> On 02/24/2014 12:16 AM, sfeam wrote:
>> On Sunday, 23 February 2014 11:22:42 PM Daniel J Sebald wrote:
>>> On 02/23/2014 10:35 PM, sfeam wrote:
>>>> On Sunday, 23 February 2014 10:04:42 PM Daniel J Sebald wrote:
>>>>>
>>>>> 	// Set plot size
>>>>> 	if (qt_setSize)
>>>>> 	{
>>>>> 		term->xmax = qt_oversampling*qt_setWidth;
>>>>> 		term->ymax = qt_oversampling*qt_setHeight;
>>>>> 		qt_setSize = false;
>>>>> 	}
>>>>>
>>>>> In a separate thread, the "term->xmax =" will be done asynchronously.
>>>>> Hence a mutex/wait is needed to make sure the code in the separate
>>>>> thread has updated term->xmax and term->max before the core thread can
>>>>> continue onward.
>>>>
>>>> Ah. Now I'm with you.
>>>> Yeah, this is the piece of code that has changed the most in qt
>>>> because nothing seems to work properly on both linux and OSX.
>>>> You are quite correct that term->foo should not be referenced in
>>>> this part of the terminal driver.  It is supposed to return the revised
>>>> font information via an event GP_fontprops.  And it _was_ doing that
>>>> at one point.  I've now lost track of all the work-arounds and what
>>>> exactly they fixed, but certainly it would be good if you can get
>>>> back to the original intent.  You can look at other terminal
>>>> drivers as a model if needed.
>>>
>>> OK, thanks.  I'll look that over.  I think that may be the one thing
>>> that isn't working yet for what I've done, i.e., some text isn't showing
>>> up probably owing to the core is told the font height is zero.
>>
>> Let me summarize a bit of the history.
>>
>> - The core code and the qt_term bits of Qt are in the same process.
>>     Call this "inboard"
>>     The screen display is being managed by a separate Qt process.
>>     Call this "outboard"
>>     The inboard and outboard processes operate asynchronously.
>>
>> - To reserve space for some text element on the next plot, the core
>>     code needs to know how big the current font is so it sends a query
>>     to the inboard terminal driver.  This request can come either via
>>     term->set_font()  or in enhanced text mode via term->put_text.
>>
>> - It would be simplest if the inboard terminal driver could just reply
>>     immediately with the requested font metrics. No communication back
>>     and forth with the outboard terminal driver is required.  This is what
>>     the Qt terminal used to do, and still does in version 4.6.
>>     The font size information is obtained by calling
>>     QFontMetrics metrics(QFont(qt_currentFontName, qt_currentFontSize));
>>     Since the inboard driver and the core code are in the same process,
>>     the inboard driver can just set term->h_char and term->v_char
>>     directly and that's the end of it.
>>
>> - Now here comes the problem.  Apparently calling QFontMetrics without
>>     there being a full QApplication and maybe [not sure] an event
>>     doesn't work properly.  In particular is was causing problems
>>     on OSX, and it was ugly even on linux.
>>     See the comments in version 4.6 qt_term.cpp
>>     // Create a QApplication without event loop for QObject's that need it,
>>     // namely font handling
>>     // A better strategy would be to transfer the font handling to the
>>     // QtGnuplotWidget, but it would require
>>     // some synchronization between the widget and the gnuplot process.
>>
>> - So in January Jérôme followed through on that comment and moved
>>     the QFontMetrics call over to the outboard driver for version 4.7.
>>     That simplified things in one way, but introduced a new complication.
>>     Now the inboard driver has to send a change font request to the
>>     outboard driver (a different process) and then wait for the resulting
>>     size information to be sent back via a QEvent.   That's where
>>     waitforinput() and do_event() suddenly become involved where
>>     they hadn't been previously.  It was also slower, so he later
>>     introduced a cache of font metrics on the inboard side but let's
>>     disregard that for now.
>>
>> - So now I gather you are working to have an event loop in the
>>     inboard driver again, although I've lost track of exactly why.
>
> Hopefully to make OSX and Windows behave the same as Unix.  I don't know
> for sure because Qt is a big creature, but I suspect that if there is no
> event loop that timers aren't guaranteed to work and who knows what
> else.  So by providing an event, I'm hoping Qt performs the same on all
> platforms.  It's just following the examples that Qt documentation gives.
>
>
>>     But in that case I think it makes sense to move the
>>     font metrics query back into the inboard driver as well.
>>     At which point we return to a setup in which waitforinput()
>>     and do_event() are not involved in font processing.
>>
>> So much for font handling and the involvement of enhanced text
>> processing.
>>
>> There is a separate issue, however, that has been at the heart of
>> the recent attempts to get OSX working.  Information about the
>> size of the display window necessarily comes from the outboard
>> driver, which is a separate process. Right now this window size
>> information is also passed using a GE_fontprops event, but there's no
>> good reason for that.  It really should have a separate event type
>> to reduce confusion.  But changing the name of the event wouldn't
>> change the information flow in any way.
>
> Oh, OK.  Thanks.  I've got the big picture now and I think we will
> probably converge on something here that is about right.
>
> I'll correct one thing in that list which is I believe the issue is not
> the presence of the QApplication in the main process, but that the
> QApplication cannot have an event loop.  That is, one can't issue
> application.exec(), because doing so will hang gnuplot while
> application.exec() blocks to handle all of its realtime traffic
> (signals, slots, system calls, etc).  Graphics in Qt must be done in a
> main thread; it can't be done in a separate thread.  That leaves the Qt
> graphics code to be run in a separate process.  (Note, I've fixed the
> process communications a bit so that there aren't these while loops that
> keep trying to establish a connection to some external service.)
>
> The QApplication issue is why I've set out to put the bulk of the
> terminal interface code in a separate thread because in the separate
> thread and event loop, i.e., thread.exec(), is perfectly fine.  The
> thread acts on its own.  My preliminary code indicates there is no
> problem sending signals back and forth across that thread.  So, with an
> event loop running in the second thread, all the timers and whatever
> else should be fine--the only restriction being that graphics cannot be
> done there.  If it turns out OSX doesn't behave the same in the thread
> that has an event loop, then something isn't right with Qt.  (We'll know
> soon.)
>
> So here is what I'm aiming for right now, sort of combining everything:
>
>      MAIN PROGRAM         |       QTHREAD        ||  GNUPLOT_QT
>                           |                      ||  (outboard)
>                           |                      ||
>    gnuplot core           |  QtTerminalInterface ||  Currently has a
>    QtTerminalEmitter      |  (active event loop) ||  few things we hope
>    (inactive event loop)  |                      ||  to move back into
>                           |                      ||  QtTerminalInterface
>
> where the single line means separate thread and the double line means a
> separate process.
>
> OK, for the most part I'll be offline for the week.
>
> Dan
>
> ------------------------------------------------------------------------------
> Flow-based real-time traffic analytics software. Cisco certified tool.
> Monitor traffic, SLAs, QoS, Medianet, WAAS etc. with NetFlow Analyzer
> Customize your own dashboards, set traffic alerts and generate reports.
> Network behavioral analysis & security monitoring. All-in-one tool.
> http://pubads.g.doubleclick.net/gampad/clk?id=126839071&iu=/4140/ostg.clktrk
> _______________________________________________
> gnuplot-beta mailing list
> gnuplot-beta@lists.sourceforge.net
> Membership management via: https://lists.sourceforge.net/lists/listinfo/gnuplot-beta
>


------------------------------------------------------------------------------
Flow-based real-time traffic analytics software. Cisco certified tool.
Monitor traffic, SLAs, QoS, Medianet, WAAS etc. with NetFlow Analyzer
Customize your own dashboards, set traffic alerts and generate reports.
Network behavioral analysis & security monitoring. All-in-one tool.
http://pubads.g.doubleclick.net/gampad/clk?id=126839071&iu=/4140/ostg.clktrk
_______________________________________________
gnuplot-beta mailing list
gnuplot-beta@lists.sourceforge.net
Membership management via: https://lists.sourceforge.net/lists/listinfo/gnuplot-beta
[prev in list] [next in list] [prev in thread] [next in thread]