[Bf-committers] Wondering about the review of the command port patch...

Dietrich Bollmann diresu at web.de
Mon Sep 22 07:15:11 CEST 2008


Hi Stéphane,

Inside of the command port patch I use condition variables.  The "busy
waiting" is part of blenders event system and therefor I had to use it
also in order to not rewrite blenders eventloop from the beginning...

This really is ugly and a waste of processor time - so here the code
which should be changed:

================================================================================
source/blender/src/editscreen.c
--------------------------------------------------------------------------------

/* ************ SCREEN MANAGEMENT ************** */

static int statechanged= 0;
void BIF_wait_for_statechange(void)
{
	if (!statechanged) {
			/* Safety, don't wait more than 0.1 seconds */
		double stime= PIL_check_seconds_timer();
		while (!statechanged) {
			winlay_process_events(1);
			if ((PIL_check_seconds_timer()-stime)>0.1) break;
		}
		statechanged= 0;
	}
	else PIL_sleep_ms(3);	/* statechanged can be set '1' while holding
mousebutton, causing locks */

}
================================================================================

The function BIF_wait_for_statechange() is called all over the place:

g -r BIF_wait_for_statechange

blender/include/BIF_mywindow.h:void BIF_wait_for_statechange(void);
blender/src/editsima.c:         BIF_wait_for_statechange();
blender/src/editnode.c:         else BIF_wait_for_statechange();
blender/src/editnode.c:         else BIF_wait_for_statechange();
blender/src/editnode.c:         else BIF_wait_for_statechange();
blender/src/editscreen.c:
BIF_wait_for_statechange();
blender/src/editscreen.c:void BIF_wait_for_statechange(void)
blender/src/editscreen.c:               else BIF_wait_for_statechange();
blender/src/editscreen.c:
BIF_wait_for_statechange();
blender/src/editscreen.c:
BIF_wait_for_statechange();
blender/src/editseq.c:                  BIF_wait_for_statechange();
blender/src/editseq.c:          else BIF_wait_for_statechange();
blender/src/drawipo.c:          else BIF_wait_for_statechange();
blender/src/drawipo.c:          else BIF_wait_for_statechange();
blender/src/drawipo.c:          else BIF_wait_for_statechange();
blender/src/editoops.c:         else BIF_wait_for_statechange();
blender/src/view.c:                     BIF_wait_for_statechange();
blender/src/editimasel.c:               else BIF_wait_for_statechange();
blender/src/editkey.c:          else BIF_wait_for_statechange();
blender/src/drawtext.c:                 BIF_wait_for_statechange();
blender/src/drawtext.c:                 BIF_wait_for_statechange();
blender/src/sculptmode.c:               else BIF_wait_for_statechange();
blender/src/drawimage.c:                else BIF_wait_for_statechange();
blender/src/vpaint.c:           else BIF_wait_for_statechange();
blender/src/vpaint.c:           else BIF_wait_for_statechange();
blender/src/editipo.c:          BIF_wait_for_statechange();
blender/src/editipo.c:  while(get_mbut()&L_MOUSE)
BIF_wait_for_statechange();
blender/src/interface.c:        while (get_mbut() & L_MOUSE)
BIF_wait_for_statechange();
blender/src/interface.c:
BIF_wait_for_statechange();
blender/src/interface.c:                else BIF_wait_for_statechange();
blender/src/interface.c:        while(get_mbut() & L_MOUSE)
BIF_wait_for_statechange();
blender/src/interface.c:                else BIF_wait_for_statechange();
blender/src/interface.c:        while (get_mbut() & L_MOUSE)
BIF_wait_for_statechange();
blender/src/interface.c:                else BIF_wait_for_statechange();
blender/src/interface.c:
BIF_wait_for_statechange();
blender/src/interface.c:                BIF_wait_for_statechange();
blender/src/interface.c:
BIF_wait_for_statechange();
blender/src/interface.c:                BIF_wait_for_statechange();
blender/src/drawseq.c:          else BIF_wait_for_statechange();
blender/src/editcurve.c:        else while(get_mbut()&R_MOUSE)
BIF_wait_for_statechange();
blender/src/editobject.c:                       while(get_mbut() &
mousebut) BIF_wait_for_statechange();
blender/src/imagepaint.c:
BIF_wait_for_statechange();
blender/src/filesel.c:          else BIF_wait_for_statechange();


...the variable "statechanged" is changed to 1 whenever an event is
added to the queue:

================================================================================
source/blender/src/editscreen.c
--------------------------------------------------------------------------------

void add_to_mainqueue(Window *win, void *user_data, short evt, short
val, char ascii)
{

	statechanged= 1;

	/*  accept the extended ascii set (ton) */
	if( !val || ascii<32 ) {
		ascii= '\0';
	}

	mainqenter_ext(evt, val, ascii);
}
================================================================================

By the way - another interesting waste of resources is the
implementation of the main queue - implemented via memmove()  ...   :)

================================================================================
source/blender/src/mainqueue.c
--------------------------------------------------------------------------------

void mainqenter_ext(unsigned short event, short val, char ascii)
{
	if (!event)
		return;

	if (nevents<MAXQUEUE) {
		memmove(mainqueue+1, mainqueue, sizeof(*mainqueue)*nevents);	
		mainqueue[0].event= event;
		mainqueue[0].val= val;
		mainqueue[0].ascii= ascii;
		
		nevents++;
	}
}

================================================================================

I originally wanted to write some patches --- but unfortunately got too
busy with my own stuff :(


Regards, 
Dietrich



On Fri, 2008-09-19 at 08:24 +0200, Stéphane SOPPERA wrote:
> > +/**
> > +   busy waiting for new events.
> > +
> > +   Note: The blocking GHOST event processing is
> > +   implemented in the same way using busy waiting (polling)
> > +   on the GHOST event queue.  Doing the same on the Blender 
> > +   event queue makes it possible to also react to events
> > +   inserted into the Blender main event queue by parallel
> > +   threads - for example the Blender command port thread.
> > +
> > +   Added to make the command port work :)
> > +   (dietrich)
> > +*/
> > +static void wait_for_event()
> > +{
> > +	while (!mainqtest()) {
> > +		PIL_sleep_ms(5);          /* sleep for 5 milliseconds */
> > +		winlay_process_events(0); /* if there are events in the GHOST queue,
> > +									 (non-blocking polling of GHOST events) 
> > +									 preprocess them and copy them over
> > +									 into the Blender main event queue.
> > +								  */
> > +	}
> > +}
> >   
> Hi,
> 
> Instead of this busy waiting with a 5ms sleep, wouldn't it be better to 
> use a pthread condition?
> http://www.humbug.org.au/talks/pthreads/pthread_cond.html
> 
> That would prevent this hugly busy waiting and enable any thread adding 
> an event to break the wait on the condition and make the event loop 
> handle the new message.
> As far as I know this is the usual way to implement a blocking queue 
> with pthread and it would make the process nicely sleep when no events 
> are available. Moreover when events come fast and are handled fast, 
> there would not be a sleep of 5ms between the handling of each event.
> 
> Finally, on windows, there is the PostMessage function that can be used 
> to post a message on the events queue of a thread, and using a WM_USER 
> message would be far easier than having to add a custom lock+condition. 
> I don't know XLib programming, maybe there is the equivalent functionality.
> 
> Regards,
> Stéphane
> 
> _______________________________________________
> Bf-committers mailing list
> Bf-committers at blender.org
> http://lists.blender.org/mailman/listinfo/bf-committers




More information about the Bf-committers mailing list