MonetDB: default - Reduce critical section in mal_resource

Sjoerd Mullender sjoerd at monetdb.org
Tue Mar 19 22:16:48 CET 2013


Why are you messing with this code?  What was wrong?

The ATOMIC_CAS was the initialization of the variable running.  It needs
to happen.  Why did you move it?
The ATOMIC_GET that you first commented out and now removed actually
already was just reading the variable without locks on most systems.
Why did you replace it?  It doesn't make *any* difference on x86_64 systems.
Obviously you didn't measure the impact since there cannot have been any.

On 2013-03-19 21:27, Martin Kersten wrote:
> Changeset: a43f806ebd08 for MonetDB
> URL: http://dev.monetdb.org/hg/MonetDB?cmd=changeset;node=a43f806ebd08
> Modified Files:
> 	monetdb5/mal/mal_resource.c
> Branch: default
> Log Message:
> 
> Reduce critical section in mal_resource
> 
> 
> diffs (31 lines):
> 
> diff --git a/monetdb5/mal/mal_resource.c b/monetdb5/mal/mal_resource.c
> --- a/monetdb5/mal/mal_resource.c
> +++ b/monetdb5/mal/mal_resource.c
> @@ -198,7 +198,6 @@ MALresourceFairness(lng usec)
>  	if ( usec > 0 && ( (usec = GDKusec()-usec)) <= TIMESLICE )
>  		return;
>  	threads = GDKnr_threads > 0 ? GDKnr_threads : 1;
> -	ATOMIC_CAS_int(running, 0, threads, runningLock, "MALresourceFairness");
>  
>  	/* use GDKmem_cursize as MT_getrss(); is to expensive */
>  	rss = GDKmem_cursize();
> @@ -210,16 +209,14 @@ MALresourceFairness(lng usec)
>  	clk =  usec / 1000;
>  
>  	if ( clk > DELAYUNIT ) {
> +		ATOMIC_CAS_int(running, 0, threads, runningLock, "MALresourceFairness");
>  		PARDEBUG mnstr_printf(GDKstdout, "#delay initial "LLFMT"n", clk);
>  		ATOMIC_DEC_int(running, runningLock, "MALresourceFairness");
> -		while (clk > 0) {
> +		/* always keep one running to avoid all waiting  */
> +		while (clk > 0 && running >= 2) {
>  			/* speed up wake up when we have memory */
>  			if (rss < MEMORY_THRESHOLD * monet_memory)
>  				break;
> -			/* always keep one running to avoid all waiting  */
> -			if ( running < 2) /* dirty read of shared variable is safe  here */
> -			//if ( (r =ATOMIC_GET_int(running, runningLock, "MALresourceFairness")) < 2)
> -				break;
>  			delay = (unsigned int) ( ((double)DELAYUNIT * running) / threads);
>  			if (delay) {
>  				if ( delayed++ == 0){
> _______________________________________________
> checkin-list mailing list
> checkin-list at monetdb.org
> http://mail.monetdb.org/mailman/listinfo/checkin-list
> 


-- 
Sjoerd Mullender

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 373 bytes
Desc: OpenPGP digital signature
URL: <http://www.monetdb.org/pipermail/developers-list/attachments/20130319/04bd32ea/attachment.sig>


More information about the developers-list mailing list