Quantcast

Re: core/GUI separation

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: core/GUI separation

Rik-4
On 05/09/2017 10:04 PM, [hidden email] wrote:
Subject:
Re: Eliminating Singleton Objects (Re: Coloring error and warning messages in the gui console)
From:
Daniel J Sebald [hidden email]
Date:
05/09/2017 01:13 PM
To:
[hidden email]
List-Post:
[hidden email]
Content-Transfer-Encoding:
7bit
Precedence:
list
MIME-Version:
1.0
References:
[hidden email] <MTAwMDAwOC5ub21hZA.1493423406@quikprotect> [hidden email] <MTAwMDAyNy5ub21hZA.1493609293@quikprotect>
In-Reply-To:
<MTAwMDAyNy5ub21hZA.1493609293@quikprotect>
Message-ID:
[hidden email]
Content-Type:
text/plain; charset=windows-1252; format=flowed
Message:
4

On 04/30/2017 10:28 PM, Rik wrote:
On 04/28/2017 06:17 PM, John W. Eaton wrote:
On 04/28/2017 07:49 PM, Rik wrote:

Is there a performance issue here?  If you start passing too many
arguments to a function, more than can conveniently fit into the number
of scratch registers that your CPU has, then you have to pass arguments
on the stack which will slow things down.  But I can't quite tell if you
are suggesting another parameter, or whether you want to include the
pointer to the interpreter as say the first entry in the
octave_value_list passed as inputs to the function.  Presumably the
octave_value_list is already passed as a pointer/reference since it
could be large.  In that case it wouldn't have an impact.

At first, I thought it would be easier to just add a pointer to the
interpreter object to the octave_value_list that is passed to functions.
 But it turned out to be difficult to chase down all the places where it
is needed.  There are many locations where it could be needed and I
couldn't figure out an easy way to get help from the compiler.

So I started working on passing the pointer to the interpreter as an
additional parameter to DEFUN functions.  It seems much cleaner this
way.  It also means that these functions will now accept three arguments
instead of two.  One is a pointer to the interpreter, one is a const
reference to the octave_value_list containing the arguments, and one is
an int for nargout.  I doubt that passing one additional pointer to these
functions will make a big difference overall, but we can certainly
measure it to be sure.

It's true.  I don't think register pressure is going to develop at 2 or 3
function parameters.  I thought we might already be at the 4-5 parameter
range in which case another could make a difference.

If the compiler is no longer doing initialization of the singleton, is this
the cause of certain data race bugs between the interpreter and the GUI
thread?  Bug #50880 and #50396 both seem to be caused by the GUI looking
for interpreter static variables that have not been initialized in time.

Also, if the pointer to the interpreter really is used all over the code
base, maybe it does make sense to have it be a global?  I know it is
generally frowned upon, but it might actually be simpler then re-working
the API to pass the pointer through to all functions.

(I wrote this a couple weeks back, but I think JWE had followed up on Rik's idea.  However, a similar issue came up with a post regarding GUI error colors, so I'm posting this more as a general reply.)

The global variable idea is frowned upon for good reason.  Global variables get abused for quick fixes which leads to strange bugs that are difficult to track.

Philosophically "pointer to interpreter all over code base" is something I've tried to deter because I think it is going the wrong direction. For example, intermixing of GUI and core/interpreter is unnecessary, as I feel there is a wealth of built-in commands that offer up all the information the GUI needs, if only there were a simple routine to return the result of an interpreter command (which can be done across a thread by passing a pointer to an octave_value_list rather than a reference to an octave_value_list).

Now that we require C++11 for the core, it wouldn't be a bad idea to use the mechanisms for parallel programming built into the language.  For example, exchanging information safely between concurrent threads could be done with std::future and std::promise.  Right now we definitely have issues associated with using multiple threads.  The one that comes to mind immediately is the fact that an exception thrown in the GUI thread will go to std::terminate because we have installed our own exception and signal handlers, but they are in the core thread.  C++11 does make it possible to request data from another thread and capture any exception so I do advocate looking at the facilities of the language more closely.

--Rik


Not only is keeping things separate good programming practice, it also achieves a couple things.  One, the concept of passing an interpreter command result back to the GUI is a narrow pathway such that any GUI, not just this particular GUI, can benefit if it just utilizes that pathway.  Two, if the interpreter and GUI become too much like one, it means expertise in the callbacks, memory sharing, etc. is required by the programmer.  In other words, people like JWE are required to make the whole thing work.  Keeping things separate means people who've worked on the core can just concentrate on that--and the interpreter seems pretty stable in recent years so that means freeing up some people's effort.

The only argument I could see for meshing the GUI and interpreter is to do very efficient realtime processing like streaming data from some exterior application into Octave, but such a thing is unlikely to happen any time soon, if ever.

Dan

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: core/GUI separation

Daniel Sebald
On 05/10/2017 12:03 PM, Rik wrote:

> On 05/09/2017 10:04 PM, [hidden email] wrote:
>> Subject:
>> Re: Eliminating Singleton Objects (Re: Coloring error and warning
>> messages in the gui console)
>> From:
>> Daniel J Sebald <[hidden email]>
>> Date:
>> 05/09/2017 01:13 PM
>>
>> To:
>> [hidden email]
>>
>> List-Post:
>> <mailto:[hidden email]>
>> Content-Transfer-Encoding:
>> 7bit
>> Precedence:
>> list
>> MIME-Version:
>> 1.0
>> References:
>> <[hidden email]>
>> <MTAwMDAwOC5ub21hZA.1493423406@quikprotect>
>> <[hidden email]>
>> <MTAwMDAyNy5ub21hZA.1493609293@quikprotect>
>> In-Reply-To:
>> <MTAwMDAyNy5ub21hZA.1493609293@quikprotect>
>> Message-ID:
>> <[hidden email]>
>> Content-Type:
>> text/plain; charset=windows-1252; format=flowed
>> Message:
>> 4
>>
>>
>> On 04/30/2017 10:28 PM, Rik wrote:
>>> On 04/28/2017 06:17 PM, John W. Eaton wrote:
>>>> On 04/28/2017 07:49 PM, Rik wrote:
>>>>
>>>>> Is there a performance issue here?  If you start passing too many
>>>>> arguments to a function, more than can conveniently fit into the
>>>>> number
>>>>> of scratch registers that your CPU has, then you have to pass
>>>>> arguments
>>>>> on the stack which will slow things down.  But I can't quite tell
>>>>> if you
>>>>> are suggesting another parameter, or whether you want to include the
>>>>> pointer to the interpreter as say the first entry in the
>>>>> octave_value_list passed as inputs to the function.  Presumably the
>>>>> octave_value_list is already passed as a pointer/reference since it
>>>>> could be large.  In that case it wouldn't have an impact.
>>>>
>>>> At first, I thought it would be easier to just add a pointer to the
>>>> interpreter object to the octave_value_list that is passed to
>>>> functions.
>>>>  But it turned out to be difficult to chase down all the places
>>>> where it
>>>> is needed.  There are many locations where it could be needed and I
>>>> couldn't figure out an easy way to get help from the compiler.
>>>>
>>>> So I started working on passing the pointer to the interpreter as an
>>>> additional parameter to DEFUN functions.  It seems much cleaner this
>>>> way.  It also means that these functions will now accept three
>>>> arguments
>>>> instead of two.  One is a pointer to the interpreter, one is a const
>>>> reference to the octave_value_list containing the arguments, and one is
>>>> an int for nargout.  I doubt that passing one additional pointer to
>>>> these
>>>> functions will make a big difference overall, but we can certainly
>>>> measure it to be sure.
>>>
>>> It's true.  I don't think register pressure is going to develop at 2
>>> or 3
>>> function parameters.  I thought we might already be at the 4-5 parameter
>>> range in which case another could make a difference.
>>>
>>> If the compiler is no longer doing initialization of the singleton,
>>> is this
>>> the cause of certain data race bugs between the interpreter and the GUI
>>> thread?  Bug #50880 and #50396 both seem to be caused by the GUI looking
>>> for interpreter static variables that have not been initialized in time.
>>>
>>> Also, if the pointer to the interpreter really is used all over the code
>>> base, maybe it does make sense to have it be a global?  I know it is
>>> generally frowned upon, but it might actually be simpler then re-working
>>> the API to pass the pointer through to all functions.
>>
>> (I wrote this a couple weeks back, but I think JWE had followed up on
>> Rik's idea.  However, a similar issue came up with a post regarding
>> GUI error colors, so I'm posting this more as a general reply.)
>>
>> The global variable idea is frowned upon for good reason.  Global
>> variables get abused for quick fixes which leads to strange bugs that
>> are difficult to track.
>>
>> Philosophically "pointer to interpreter all over code base" is
>> something I've tried to deter because I think it is going the wrong
>> direction. For example, intermixing of GUI and core/interpreter is
>> unnecessary, as I feel there is a wealth of built-in commands that
>> offer up all the information the GUI needs, if only there were a
>> simple routine to return the result of an interpreter command (which
>> can be done across a thread by passing a pointer to an
>> octave_value_list rather than a reference to an octave_value_list).
>
> Now that we require C++11 for the core, it wouldn't be a bad idea to use
> the mechanisms for parallel programming built into the language.  For
> example, exchanging information safely between concurrent threads could
> be done with std::future and std::promise.  Right now we definitely have
> issues associated with using multiple threads.  The one that comes to
> mind immediately is the fact that an exception thrown in the GUI thread
> will go to std::terminate because we have installed our own exception
> and signal handlers, but they are in the core thread.  C++11 does make
> it possible to request data from another thread and capture any
> exception so I do advocate looking at the facilities of the language
> more closely.
>
> --Rik

Yeah, that exception/signaling issue is a challenge.  It would be nice
to keep that sort of on the GUI side of things if possible, because
otherwise it would mean requiring C++ in the GUI.  There may be third
party GUI's out there with a C, Python, etc. framework, and I see no
reason to exclude them.

As a reminder, the issue with exchanging octave_value_list across
threads is the reference counting.  Doing so changes the reference count
on both sides of the thread which can lead to problems if the timing is
wrong.  That's why I investigate exchanging the pointer, which doesn't
affect the reference count.  Seemed to work.

But the exception/signaling could also help in the area of shutdown.
There is some patch on Savanah I wrote that was meant to gracefully
shudown if the core was busy doing something, or hanged, or whatever.
The GUI issues the "exit" in the queue; then sets its own timer for a
few seconds; when the timer expires the GUI attempts to kill the
process; etc.  It sort of worked, but there were all these tricky
situations of "What if the user presses the exit button multiple times
while the GUI is waiting?", "What if the Force Quit? dialog box is up
and then the core does finally respond?", and so on.  I couldn't seem to
cover all the cases just right and it started to feel like it was more
complex than it needed to be.

Dan

Loading...