[Octave-patch-tracker] [patch #8016] Prototype octave_value_list transfer

[Dan Sebald]

Follow-up Comment #2, patch #8016 (project octave):

Attached is a changeset that replaces the callback functions with a background
command queue.  (Actually, this is very similar in behavior, using the
do_process_events() poll loop as a starting point.)  The feature it adds is
passing command results back to the GUI in the background.  It uses QMutex and
QWaitCondition to make sure the Octave worker thread is in a wait state before
handling data on the GUI side of the thread divide.  octave_value_list does no
copy/delete in the signal/slot queuing mechanism across threads.

For this version of the changeset I've left in a number of fprintf()s to give
a person an idea of what is happening, and to coincide with the results to
follow.  I've also used USE_CALLBACKS to ifdef out code rather than removing
everything.  That will make the changeset diff file a little smaller and
easier to follow.  I will create a new changeset with callbacks removed in a
hour or two.

Here are comments from working with this:

1) Consider renaming libinterp/interp-core/oct-obj.h to
libinterp/interp-core/ov-list.h or something similar.  It took a long time for
me to find where octave_value_list is declared.

2) There is already something like the Octave background command.  It is
QTerminal::relay_command (command) which sends a command to the input stream
of Octave core and is used in a couple places for changing a directory and
whatnot.  However, that does not allow returning data and always puts some
text on the terminal output and command history list.  The background command
queue is kind of nice in the sense it is low priority GUI events that happen
between Octave worker thread's bigger tasks when idle at the command line.

3) Right now, there is a bit of mixing and matching with the octave_link.  In
some cases there are calls to static functions.  In some cases a pointer to an
instance of the object is used.  Perhaps just one method is preferred down the
road.

4) I put some fprintf commands at key locations (not include in this
patch...to verbose and slow):
* octave_value copy constructor
* wherever there is a octave_value refcount decrement
* right before emitting signal across threads
* right after emitting signal across threads
First, there are an absolute ton of copies and destructs of octave_values for
any sort of activity, even trivial things like "return" at the command line. 
If atomic references means more sophisticated copy, slowness will be an
issue.

Second, here is a sample in which a breakpoint is set so that Octave core does
a "dbstop" via eval_string() (I typed AAAA so that I new where the response
began for the command):


octave:1> AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAYour wish is my
command:  dbstop ("testfun", 4);
Made an octave_value copy.
Destructed an octave_value.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Made an octave_value copy.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Destructed an octave_value.
Made an octave_value copy.
Destructed an octave_value.
Made an octave_value copy.
Destructed an octave_value.
Made an octave_value copy.
Destructed an octave_value.
Made an octave_value copy.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Made an octave_value copy.
Destructed an octave_value.
Made an octave_value copy.
Destructed an octave_value.
Made an octave_value copy.
Destructed an octave_value.
Made an octave_value copy.
Removed an octave_value via operator.
Destructed an octave_value.
Made an octave_value copy.
Removed an octave_value via operator.
Destructed an octave_value.
Made an octave_value copy.
Destructed an octave_value.
Made an octave_value copy.
Removed an octave_value via operator.
Destructed an octave_value.
Made an octave_value copy.
Removed an octave_value via operator.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Made an octave_value copy.
Destructed an octave_value.
Destructed an octave_value.
Made an octave_value copy.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Made an octave_value copy.
Destructed an octave_value.
Made an octave_value copy.
Destructed an octave_value.
Made an octave_value copy.
Destructed an octave_value.
Removed an octave_value via operator.
Destructed an octave_value.
Removed an octave_value via operator.
Destructed an octave_value.
Made an octave_value copy.
Destructed an octave_value.
Made an octave_value copy.
Destructed an octave_value.
Made an octave_value copy.
Removed an octave_value via operator.
Destructed an octave_value.
Made an octave_value copy.
Destructed an octave_value.
Destructed an octave_value.
Removed an octave_value via operator.
Destructed an octave_value.
Destructed an octave_value.
Removed an octave_value via operator.
Destructed an octave_value.
Made an octave_value copy.
Destructed an octave_value.
Made an octave_value copy.
Destructed an octave_value.
Removed an octave_value via operator.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Removed an octave_value via operator.
Destructed an octave_value.
Destructed an octave_value.
Here's right before transmitting signal.
Here's right after transmitting signal.
GUI?  Here's the result for:  dbstop ("testfun", 4);
This isn't for me.
OK, All done.
Destructed an octave_value.


Note all the activity going on in the core.  And note that there appears to be
no octave_value copy happening between before and after emitting (i.e.,
queueing) the signal transferring data across the thread.  (That's the 5th and
6th last line above.)  I take it that means that using octave_value_list is
better than using octave_value.

To verify this, I changed the emitted signal so that it has an unused
octave_value as one of its arguments like this:


octave_value junk;
fprintf (stderr, "Here's right before transmitting signal.n");
      emit transmit_octave_command_result (ci.senderid, ci.command,
                                           command_status, retval, junk);
fprintf (stderr, "Here's right after transmitting signal.n");


and for a similar "dbstop" command get the following output:


octave:1> BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBYour wish is my command: 
dbstop ("testfun", 6);
Made an octave_value copy.
Destructed an octave_value.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Made an octave_value copy.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Removed an octave_value via operator.
Destructed an octave_value.
Made an octave_value copy.
Destructed an octave_value.
Made an octave_value copy.
Destructed an octave_value.
Made an octave_value copy.
Destructed an octave_value.
Made an octave_value copy.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Made an octave_value copy.
Destructed an octave_value.
Made an octave_value copy.
Destructed an octave_value.
Made an octave_value copy.
Destructed an octave_value.
Made an octave_value copy.
Removed an octave_value via operator.
Destructed an octave_value.
Made an octave_value copy.
Removed an octave_value via operator.
Destructed an octave_value.
Made an octave_value copy.
Destructed an octave_value.
Made an octave_value copy.
Removed an octave_value via operator.
Destructed an octave_value.
Made an octave_value copy.
Removed an octave_value via operator.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Made an octave_value copy.
Destructed an octave_value.
Destructed an octave_value.
Made an octave_value copy.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Made an octave_value copy.
Destructed an octave_value.
Made an octave_value copy.
Destructed an octave_value.
Made an octave_value copy.
Destructed an octave_value.
Removed an octave_value via operator.
Destructed an octave_value.
Removed an octave_value via operator.
Destructed an octave_value.
Made an octave_value copy.
Destructed an octave_value.
Made an octave_value copy.
Destructed an octave_value.
Made an octave_value copy.
Removed an octave_value via operator.
Destructed an octave_value.
Made an octave_value copy.
Destructed an octave_value.
Destructed an octave_value.
Removed an octave_value via operator.
Destructed an octave_value.
Destructed an octave_value.
Removed an octave_value via operator.
Destructed an octave_value.
Made an octave_value copy.
Destructed an octave_value.
Made an octave_value copy.
Destructed an octave_value.
Removed an octave_value via operator.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Destructed an octave_value.
Removed an octave_value via operator.
Destructed an octave_value.
Destructed an octave_value.
Here's right before transmitting signal.
Made an octave_value copy.               <<<<<<<<<<<<  NOTICE (NOTE ADDED
MANUALLY)
Here's right after transmitting signal.
Destructed an octave_value.
GUI?  Here's the result for:  dbstop ("testfun", 6);
This isn't for me.
OK, All done.
Destructed an octave_value.              <<<<<<<<<<<<  NOTICE (NOTE ADDED
MANUALLY)
Destructed an octave_value.


Notice that there is an octave-value-copy associated with the emitted (queued)
signal and an extra destructor call associated with everything finished when
including an octave_value in the list of arguments.  The above is the
dangerous scenario because rep->count can be modified in an intermixed way
between threads.  Hence, octave_value_list is the preferred data format.

Lastly, here's the situation when a routine in the GUI thread actually uses
the data from the octave_value_list signaled from the Octave worker thread. 
(I had to add more specific fprintf count++ and --count to tally things.) 
This is the dbstatus command when attempting to restore breakpoints:


octave:1> CCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCYour wish is my
command:  dbstatus ();
Created octave_value:  count++
[SNIP]
Removed an octave_value via operator.  --count
Created an octave_value via operator:  count++
Destructed an octave_value:  --count
Destructed an octave_value:  --count
Destructed an octave_value:  --count
Removed an octave_value via operator.  --count
Created an octave_value via operator:  count++
Here's right before transmitting signal.
Here's right after transmitting signal.
GUI?  Here's the result for:  dbstatus ();
Created octave_value:  count++           <<<<<<<<< NOTICE START
Removed an octave_value via operator.  --count
Created an octave_value via operator:  count++
Destructed an octave_value:  --count
Removed an octave_value via operator.  --count
Created an octave_value via operator:  count++
Created octave_value:  count++
Removed an octave_value via operator.  --count
Created an octave_value via operator:  count++
Destructed an octave_value:  --count
Removed an octave_value via operator.  --count
Created an octave_value via operator:  count++
Created octave_value:  count++
Removed an octave_value via operator.  --count
Created an octave_value via operator:  count++
Destructed an octave_value:  --count
Removed an octave_value via operator.  --count
Created an octave_value via operator:  count++
Destructed an octave_value:  --count
Destructed an octave_value:  --count
Created octave_value:  count++
Removed an octave_value via operator.  --count
Created an octave_value via operator:  count++
Destructed an octave_value:  --count
Removed an octave_value via operator.  --count
Created an octave_value via operator:  count++
Created octave_value:  count++
Removed an octave_value via operator.  --count
Created an octave_value via operator:  count++
Destructed an octave_value:  --count
Removed an octave_value via operator.  --count
Created an octave_value via operator:  count++
Created octave_value:  count++
Removed an octave_value via operator.  --count
Created an octave_value via operator:  count++
Destructed an octave_value:  --count
Removed an octave_value via operator.  --count
Created an octave_value via operator:  count++
Destructed an octave_value:  --count
Destructed an octave_value:  --count
Created octave_value:  count++
Removed an octave_value via operator.  --count
Created an octave_value via operator:  count++
Destructed an octave_value:  --count
Destructed an octave_value:  --count
Destructed an octave_value:  --count
Destructed an octave_value:  --count
Destructed an octave_value:  --count     <<<<<<<<<<< NOTICE END
OK, All done.
Now waking worker thread.
Destructed an octave_value:  --count


Notice all that new activity due to the manipulations of octave_value
instances in the GUI thread with the command results.  I count 28 --count but
only 20 count++.  Maybe I missed some, but the point is that the GUI thread
can work on all that data because the Octave worker thread is asleep.  If
things are programmed properly, the rep-count changes should be a zero sum and
things properly cleaned up by time the GUI is done with computations.

6) With this setup it doesn't seem like atomic reference counting is
necessary.  The 


// FIXME -- we should not be including config.h in header files.
#ifdef HAVE_CONFIG_H
#include <config.h>
#endif


is getting a little cumbersome though.  Perhaps there is a better way to
configure reference counting than having to include <config.h> in derivative
header files.

6) I'm not completely happy with the setup for file_editor and file_editor_tab
pointers and container classes.  The signal-to-signal connections lose the
original sender of the signal.  (Could easily add that to list, but prefer to
hold off if it isn't necessary.)  Things are OK now, maybe global _octave_link
is better, but not sure.  The critical issue is that right now a
file_editor_tab is created, then a ->load_file() is attempted and if that
passes then connections are made.  However, it is the load process where an
inquiry for dbstatus() should be done, but that requires a connection which
isn't yet set.  I just didn't want to mess with that right now and cause
problems that are sort of tangent to the code here.  The octave_qt_link is a
proper and good mechanism, it's just that this needs cleanup for better
sequence of construction--consequence of not being modular and object oriented
enough at beginning...which is OK.

7) dbstatus() is the only routine right now using the data coming from the
background queue.  The ones that don't need data really don't need the sleep
mechanism, but I think it is better to always use that, just for consistency.

8) I think with the setup of having two core slots on both sides of the thread
for the outer level GUI objects to attached to is pretty safe.  If folks
aren't comfortable with the computations going on in the GUI thread, it
wouldn't be too difficult to interpret the commands in the background command
queue (e.g., "dbstatus") and then emit signals accordingly.  However, I sort
of like things the way they are.  It requires some knowledge on part of the
programmer, but it stops the growing list of callbacks that aren't as
rigorously defined as the core data objects are.  It also makes for adding new
features and behavior rather easy by just compiling GUI code and not having to
compile core code.  That is, this is for that class of GUI-initiated,
non-workspace modifying events.  (Command line generated is a different issue
for a different time.)


(file #27902)
    _______________________________________________________

Additional Item Attachment:

File name: octave-background_command-2013apr18_v1.patch Size:33 KB


    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/patch/?8016>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/