Re: [Plplot-devel] C++ class constructor question
Hi Alan The short answer is yes use a transmit and receive function to send the data, but no don't create a new object every time you want to send something. The current private object is definitely the way to go. I should have used a transmit and receive function in the current code. If I wasn't going to trample your uncommitted changes I would actually do that right now. The reason is partitioning of functionality. The job of the Memory map, as it stands is to provide a space in memory where the wxDevice can write data. You are absolutely correct that it would be much better to have a send and receive method - in reality the wxDevice does not care how the data gets sent, either via shared memory, over a network port, by writing to a file on the disk drive or even by messenger pigeon - it just wants to send it and sometimes get some back. By adding those functions we totally separate out the transmission from the other stuff that wxPLDevice does. This would mean actually we could create different classes that do the transmission in different ways and we could use different objects as slot in replacements if we liked. However, if you pull out the semaphores and have wxPLDevice look after those then we have messed up our nice separation of functionality again. Our wxPLDevice now knows that we are communicating with semaphores and memory maps. So what I should have done (and this probably came from the way things evolved and got more complicated than I expected) is created a PLCommunicator class. Then I could have inherited from this to make a PLMemoryMapCommunicator and all the code from wxDevice::Transmit, should go in there. In fact if you can give me some time to do that without stepping on your uncommitted edits then I will as I think it will make your life easier. Now in terms of having a default constructor that initialises everything invalid, well there is nothing wrong with that. I don't see how constructing a new object for every use is simpler, in fact the opposite. I'm also not sure where exactly you will be creating your shared memory. In PLMemoryMap? If you keep creating and destroying your PLMemoryMap objects then you will keep scrapping and recreating your shared memory won't you? It also seems that although you would remove a small amount of code from PlMemory map, you instead make the class more difficult to use, so it doesn't seem like the right choice for me in many ways. I hope that makes sense. Phil On 11 February 2017 at 21:46, Alan W. Irwin wrote: > Hi Phil: > > I have noticed the style of communications you are using between various > classes typically involves private variables such as > > PLMemoryMap m_outputMemoryMap; > > for wxPLDevice, a default constructors for > PLMemoryMap that essentially sets everything relevant to invalid > values, and no explicit use of the constructor that does valid > initialization for PLMemoryMap from within wxPLDevice. (Instead an > explicit create method is called to do the required valid > initialization.) > > Would there be anything wrong with dropping that private variable, > dropping the default invalid constructor, dropping the create method, > creating an instance of PLMemoryMap (with correct explicit valid > constructor with a mustExist argument to decide if you are going to > reuse external memory locations from a previously initialized > instance), and using the methods of that instance when you need them? > > That latter simplified approach is the one I have tentatively adopted > for the new PLTwoSemaphores class I have just implemented (see commit > cd13d65). The (only) constructor checks that the argument values > corresponding to the addresses of the two semaphores are non-NULL, and > then the internal private variables of the instance that correspond to > those two addresses are updated. If mustExist is true, nothing more > is done, but if mustExist is false, then sem_init is called for each > of those pointers. The (only) destructor for the PLTwoSemaphores class > calls sem_destroy for each of the (internal) pointers and sets those > internal pointers to NULL. And all other methods of PLTwoSemaphores > that use those internal pointers check that they are non-NULL. > > So far the two-semaphores implementation is incomplete so I have only > tested the new PLTwoSemaphores class with a build (with the g++ > compiler finding no issue with it). Do you anticipate I will be > running into trouble later on with the above simplified constructor > approach once I start to use PLTwoSemaphores class instances (e.g., in > the arguments for the currently empty PLMemoryMap::sendBytes and > PLMemoryMap::receiveBytes methods? > > If it turns out you do believe the current simplified PLTwoSemaphores > constructor approach will work OK, should we also switch to that > simplified approach everywhere else (e.g., to handle wxPLDevice's use > of PLMemoryMap)? > > Alan > __ > Alan W. Irwin > > Astronomical research affi
[Plplot-devel] The status of the wxwidgets IPC development
On 2017-02-12 23:08- Phil Rosenberg wrote: > Hi Alan > > The short answer is yes use a transmit and receive function to send > the data, but no don't create a new object every time you want to > send something. The current private object is definitely the way to > go. OK. I plan to stick with my local instance approach for now (automatically constructing a PLTwoSemaphore instance and destroying it again for each call to my transmitBytes and receiveBytes methods), but move to the private object approach for efficiency later on once I understand (hopefully with some further help from you) exactly what it does at run time compared with the local instance approach. > I should have used a transmit and receive function in the current > code. If I wasn't going to trample your uncommitted changes I would > actually do that right now. The reason is partitioning of > functionality. The job of the Memory map, as it stands is to provide a > space in memory where the wxDevice can write data. You are absolutely > correct that it would be much better to have a send and receive method Yes, that was my exact motivation. [...] > However, if you pull out the semaphores and have wxPLDevice look after > those then we have messed up our nice separation of functionality > again. Our wxPLDevice now knows that we are communicating with > semaphores and memory maps. > So what I should have done (and this probably came from the way things > evolved and got more complicated than I expected) is created a > PLCommunicator class. Then I could have inherited from this to make a > PLMemoryMapCommunicator and all the code from wxDevice::Transmit, > should go in there. In fact if you can give me some time to do that > without stepping on your uncommitted edits then I will as I think it > will make your life easier. Actually, I am right in the middle of modifying wxPLDevice::TransmitBuffer and wxPlFrame::ReadTransmission to use the transmitBytes and receiveBytes methods of PLMemoryMap (which are now completely implemented and they build, but I haven't pushed that commit because I have no way of testing those methods until the modifications to wxPLDevice::TransmitBuffer and wxPlFrame::ReadTransmission are done). So I have invested considerable time in understanding those routines as they are now organized. Also, my ongoing changes for the -DPL_WXWIDGETS_IPC2=ON case clearly separate assembling the data in wxPLDevice::TransmitBuffer and acting on it in wxPlFrame::ReadTransmission from the task of transmitting the assembled array of bytes from wxPLDevice::TransmitBuffer and receiving that same array of bytes in wxPlFrame::ReadTransmission. So my separation work (already mostly completed now) should give you a good start on that desired reorganization. So please hold off until later to reorganize the code. To summarize, here are the next steps I plan to do. (1) Get the two-semaphores approach fundamentally working one way from -dev wxwidgets to wxPLViewer by ignoring interactive examples and temporarily dropping text size feedback from wxPLViewer back to -dev wxwidgets and ultimately the PLplot core. (2) Refine that approach to make it two-way so that the -DPL_WXWIDGETS_IPC2=ON result passes all tests (including text size feedback as well as including example 1 with the -locate option) that are currently passed by the -DPL_WXWIDGETS_IPC2=OFF case. (3) Measure the efficiency of that local instance method and then try the private object method (once I understand exactly what it does at run-time) to see how much that efficiency improves. (4) Decide (with you) based on the simplicity and speed of the two-semaphores approach compared with the one-semaphore approach which approach we will take after that. (5) If the decision is for two semaphores, then the further steps I have in mind would be to (i) (Alan) implement a _named_ two-semaphores variation on the unnamed two-semaphores approach, (ii) (Phil) implement a named two-semaphores approach for the Windows case, and (iii) (Alan or Phil) completely remove all one-semaphore code. But if our decision goes against the two-semaphores approach, then (i') (Alan or Phil) strip all the two-semaphores code out instead but (ii') (Phil) keep the idea of separating the transmitBytes and receiveBytes methods (which have no logic other than to transmit and receive arrays of bytes) from everything else. It is not at all clear how long these steps are going to take, but I am hoping for a time scale of something like one day each for (1) through (3). And I hope you don't mind holding off any of the changes you are thinking about (especially the reorganizing ones) since such changes would slow me down, and we, in any case, really have no idea where we are going with the wxwidgets IPC until (4) is done. More later as I continue to make progress on (1) through (3). Alan __ Alan W. Irwin Astronomical research affiliation with Department of Physics and Astronomy, Univer
