List:Commits« Previous MessageNext Message »
From:Vladislav Vaintroub Date:October 4 2010 11:43am
Subject:RE: bzr commit into mysql-5.5-bugteam branch (vvaintroub:3220) Bug#56405 Bug#56585
View as plain text  

> -----Original Message-----
> From: Dmitry Lenev [mailto:Dmitry.Lenev@stripped]
> Sent: Monday, October 04, 2010 11:00 AM
> To: Vladislav Vaintroub
> Cc: commits@stripped
> Subject: Re: bzr commit into mysql-5.5-bugteam branch (vvaintroub:3220) Bug#56405
> Bug#56585

Hi Dmitry

 
> Hello Vladislav!
> 
> Here is my comments about new version of your patch:> 

[...]
> OK. If there is a significant interest in bringing rwlock improvement
> to Vista we can implement one of approaches that you have outlined
> above in a separate patch.

Davi pointed me to  yet another interesting option. Hacky, yet very simple .
http://locklessinc.com/articles/pthreads_on_windows/
contains a one-liner pthread_mutex_trywrlock on top of SRWLock, with explanation.

> I was a bit surpised to see that you have decided that you need copy
> the whole my_rw_lock_t structure for Windows instead of having something
> like:
> 
> typedef union
> {
> #ifdef _WIN32
>   /* Native rwlock (is_srwlock == TRUE) */
>   struct
>   {
>     SRWLOCK srwlock;             /* native reader writer lock */
>     BOOL have_exclusive_srwlock; /* used for unlock */
>   };
> #endif
> 
>  /*
>    Portable implementation (is_srwlock == FALSE)
>    Fields are identical with Unix my_rw_lock_t fields.
>  */
>  struct
>  {
>    pthread_mutex_t lock;       /* lock for structure		*/
>    ...
>  }
> } my_rw_lock_t;
> 
> Especially since both Windows-specific definition and generic portable
> implementation are still handled by the same calls.
> 
> Could you please explain your reasons for doing this?

I'm not sure anonymous structs are supported by every compiler. Or even I'm sure that they
are not. While it would be not a deal to
name the structs , and then modify the portable implementation to follow your suggestion,
this would still have a certain WTF effect
for non -Windows ( having  a union of consisting of a single struct). BTW, are you aware
of any OS except Windows, where generic
portable implementation is used. If it is Windows-only,  we could save a great deal of
ifdefs, including the one you cite.

> >
> > +#ifdef _WIN32
> > +
> > +static BOOL have_srwlock = TRUE;
> 
> Ouch... IMO the above looks wrong as it means that native rwlocks
> are going to be used even on pre-Vista systems.
Good catch, and yeah ouch :) Looks like last-moment edit.

> I wonder if you have tried to run server built with your patch on
> any pre-Vista system. If you have not, please do!

Fixed, just run the main suite on XP. It works then..

Pushed.

Thanks again

> 
> --
> Dmitry Lenev, Software Developer
> Oracle Development SPB/MySQL, www.mysql.com
> 
> Are you MySQL certified?  http://www.mysql.com/certification
> 
> --
> MySQL Code Commits Mailing List
> For list archives: http://lists.mysql.com/commits
> To unsubscribe:    http://lists.mysql.com/commits?unsub=1


Thread
bzr commit into mysql-5.5-bugteam branch (vvaintroub:3220) Bug#56405Bug#56585Vladislav Vaintroub4 Oct
Re: bzr commit into mysql-5.5-bugteam branch (vvaintroub:3220)Bug#56405 Bug#56585Dmitry Lenev4 Oct
  • RE: bzr commit into mysql-5.5-bugteam branch (vvaintroub:3220) Bug#56405 Bug#56585Vladislav Vaintroub4 Oct