Re: Reworked qt terminal is much faster – and fails to work on Mac

[Mojca M. <moj...@gm...>]

On Mon, Feb 10, 2014 at 1:57 AM, sfeam wrote:
> On Sunday, 09 February 2014 12:29:34 PM Mojca Miklavec wrote:
>> On Sun, Feb 9, 2014 at 12:11 PM, Mojca Miklavec wrote:
>>
>> According to a gdb a lot of that "infinite cycling" (when not properly
>> connected) happens with
>>
>> while (!receivedFontPropos)
>>     qt->socket.waitForReadyRead(1000);
>>     while (qt->socket.bytesAvailable() >= (int)sizeof(gp_event_t))
>> }
>
> It should not be possible for that while {} construction to gobble CPU.
> It only wakes up and checks the socket once per second.
> Unless (which I'm beginning to suspect) all the 1000 msec timeouts
> are acting as NOOPs on OSX.  Could it really be that Qt on OSX
> doesn't implement event-driven waits?

Sorry, I don't know the answer to that, but I can do some
quick-and-dirty test if you can suggest me what to test.

But in my opinion the problem is that qt isn't properly initialized by
the time it executes the "problematic" part of the code.

>> And indeed if I change the timeout:
>>
>> --- a/src/qtterminal/qt_term.cpp
>> +++ b/src/qtterminal/qt_term.cpp
>> @@ -251,7 +252,7
>>
>>         // The QLocalSocket::waitForConnected does not respect the
>> time out argument when the
>>         // gnuplot_qt application is not yet started. To wait for it,
>> we need to implement the timeout ourselves
>> -       QDateTime timeout = QDateTime::currentDateTime().addMSecs(1000);
>> +       QDateTime timeout = QDateTime::currentDateTime().addMSecs(10000);
>>         do
>>         {
>>                 qt->socket.connectToServer(server);
>>
>> it suddenly almost starts working. I'm saying almost because the first
>> plot doesn't work, but the second one does.
>
> Hence my suspicion that timeouts < 1000msec are not working.
> I suggest you do a global search and replace for timeouts and make
> sure they are all >1000 msec.

I'm not sure how to search for all timeouts. I need some help with that.

I see the following (but I'm not sure how to change it):

if (options == TERM_ONLY_CHECK_MOUSING) {
    timeout = &one_msec;
    one_msec.tv_sec = 0;
    one_msec.tv_usec = TERM_EVENT_POLL_TIMEOUT;
}

and I'm not sure how to reliable find all the others.

> Finally a trace that makes sense in that it involves a recent change.
> This bit belongs to the patch:
>
> 2014-01-26  Jérôme Lodewyck  <lod...@us...>
>
>         * src/qtterminal/qt_term.cpp: Implement font metric caching. This solves
>         a flickering issue when a large number of font changes are called (for
>         example when rotating the world plot in world2.dem).
>
>         * src/qtterminal/QtGnuplotInstance.*: New public function that sends a
>         command to gnuplot and blocks until it receives the answer.
>
> Can you back out just that one set of changes from current CVS and see
> if that makes your qt terminal work?

If I go back to the following version ...

Date:   Sat Jan 18 21:30:19 2014 +0000
    interactive color character art terminal using libcaca

(I didn't try to revert just a single commit, I simply went back to
the version before the one you claimed to be problematic.)

... then gnuplot doesn't get stuck at 99% CPU any longer. It returns
back to accepting input and the second plot works fine. (But please
note that this isn't really a problem as long as timeouts are
sufficiently large. I didn't check, but I suspect that the infinite
loop happens just because Qt isn't properly initialized at the time
when the code is trying to do something.)

Just to make it clear. The following happens:

Terminal type set to 'qt'
gnuplot> plot sin(x) # too short timeout, so another gnuplot_qt is started
Could not connect gnuplot_qt "" . Starting a new one
    # starts a new gnuplot_qt, but timeouts are still too short, so
nothing can be seen
gnuplot> plot cos(x) # the first plot that works ok
gnuplot> quit # only closes the second gnuplot_qt, one gnuplot_qt
needs to be closed manually

Independent on the font patch (which may not necessarily be bad, I
didn't check) the following are the main issues from my point of view:

1.) The following part of the code:

    while((qt->socket.state() != QLocalSocket::ConnectedState) &&
(QDateTime::currentDateTime() < timeout));
    // Still not connected...
    if ((qt->socket.state() != QLocalSocket::ConnectedState) && retry)

gives up too soon (1000 ms seems too short) and simply opens a new
"gnuplot_qt". This is a problem also because the old gnuplot_qt never
gets closed as a consequence. But this can be easily fixed by
increasing the timeout, at least the majority of problems would go
away.

2.) The second time when the same code gets executed (with
retry=false), it gives up too soon again. But now the problem is that
gnuplot blindly assumes that the code succeeded and connection is
alive. In my opinion there should be another check at the end of
qt_connectToServer (or somewhere else):

    if (qt->socket.state() != QLocalSocket::ConnectedState))

that would throw an error and prevent Qt from proceeding in case that
connection didn't succeed. Gnuplot shouldn't blindly assume that
connection succeeded the second time it tried.


Curiously, gnuplot_qt has about 40 font files open when I run just
"plot sin(x)". I don't understand why. For example:

/Library/Fonts/STIXIntSmReg.otf
/Library/Fonts/STIXSizThreeSymBol.otf
/Library/Fonts/STIXSizFiveSymReg.otf
/Library/Fonts/Microsoft/Marlett.ttf
/Library/Fonts/STIXSizOneSymBol.otf
/Library/Fonts/STIXSizFourSymReg.otf
/Library/Fonts/STIXSizFourSymBol.otf
/Library/Fonts/STIXIntUpSmReg.otf
/Library/Fonts/STIXSizThreeSymReg.otf
/Library/Fonts/STIXSizOneSymReg.otf
/Library/Fonts/STIXVar.otf
/Library/Fonts/STIXIntUpBol.otf
/Library/Fonts/STIXSizTwoSymReg.otf
/System/Library/Fonts/Symbol.ttf
/Library/Fonts/Microsoft/MS Reference Specialty.ttf
/Library/Fonts/Microsoft/Bookshelf Symbol 7.ttf
/Library/Fonts/STIXIntDReg.otf
/Library/Fonts/STIXNonUniBolIta.otf
/System/Library/Fonts/Apple Braille Pinpoint 8 Dot.ttf
/System/Library/Fonts/Apple Braille Outline 8 Dot.ttf
/Library/Fonts/STIXIntUpReg.otf
/System/Library/Fonts/ZapfDingbats.ttf
/Library/Fonts/STIXNonUniIta.otf
/Library/Fonts/STIXIntUpDBol.otf
/Library/Fonts/Hoefler Text Ornaments.ttf
/Library/Fonts/STIXNonUni.otf
/Library/Fonts/Bodoni Ornaments ITC TT/..namedfork/rsrc
/Library/Fonts/STIXNonUniBol.otf
/System/Library/Fonts/Apple Braille Outline 6 Dot.ttf
/Library/Fonts/STIXIntUpSmBol.otf
/System/Library/Fonts/Apple Braille Pinpoint 6 Dot.ttf
/System/Library/Fonts/Apple Color Emoji.ttf
/Library/Fonts/STIXIntSmBol.otf
/Library/Fonts/STIXSizTwoSymBol.otf
/Library/Fonts/STIXIntDBol.otf
/Library/Fonts/Type Embellishmnt One LET/..namedfork/rsrc
/System/Library/Fonts/Apple Braille.ttf
/System/Library/Fonts/LucidaGrande.ttc

----------------------

In summary. If I do both:
(a) revert to the version from Sat Jan 18 21:30:19 2014
(b) increase QDateTime timeout = QDateTime::currentDateTime().addMSecs(10000);
then gnuplot works fine.

If I do just (b) on the latest version, then the first plot fails.
If I do just (a), the first plot fails, a second "gnuplot_qt" is
started (which is bad) and the second plot succeeds.

Mojca