List:Commits« Previous MessageNext Message »
From:Joerg Bruehe Date:June 4 2008 10:59am
Subject:Re: commit into mysql-6.0-wtf:mysql-6.0-wtf branch (vvaintroub:2653)
Bug#24509, WL#3049
View as plain text  
Hi Davi !


Davi Arnaut wrote:
> Hi Vlad,
> 
> Vladislav Vaintroub wrote:
>> #At bzr+ssh://bk-internal.mysql.com/bzrroot/server/mysql-6.0-wtf/
>>
>>  [[...]]
> 
> Wouldn't this be a good opportunity to start to untangle the ifdefery
> mess in mysys? We could start splitting the implementations into
> different directories, for example:
> 
> mysys/win32/file_io.c
> mysys/unix/file_io.c
> mysys/netware/file_io.c
> 
> And each one would implement it's own specified version of my_foo.

I have a very strong feeling against such a split into separate files 
for platform-specific implementations.


Sure, it would get rid of several "#if"/"#ifdef"/... and so make reading 
a bit easier.


But it would also cause several source modules (files) which have to 
implement the same interface (for the caller), so any change to that 
interface would need to be done in several files in sync.

As long as these various implementations are in the same file, it is 
easy to search for the relevant identifier(s) using your editor, and 
find all occurrences.
Hopefully, it occurs only once, and any differentiation occurs in its 
body - this guarantees an interface change is done in the same way for 
all implementations.

The moment you have multiple modules (by implementation) for one 
interface, keeping them in sync gets much more complicated - even more 
so, because your local testing will typically be on one platform only.

If you really feel different implementations should be split into 
different files, at least keep one common entry (mapper) module for 
them. (The drawback of this is of course reduced chances for the 
optimizer, as now the call crosses file boundaries.)

IMHO, a mapper function for the interface, with its body being just some 
"#if" around calls to different implementations, and these being defined 
as "static inline" functions (completely within large "#if" blocks) is 
as readable, and it allows for optimization.


I have seen too many cases where developers changed one occurrence only 
(which they had tested) and forgot others (which "bk -r grep ..." would 
have immediately shown to them).


Regards,
Jörg

-- 
Joerg Bruehe,  MySQL Build Team,  joerg@stripped   (+49 30) 417 01 487
Sun Microsystems GmbH,   Sonnenallee 1,   D-85551 Kirchheim-Heimstetten
Geschaeftsfuehrer: Thomas Schroeder, Wolfgang Engels, Dr. Roland Boemer
Vorsitzender des Aufsichtsrates: Martin Haering     Muenchen: HRB161028

Thread
commit into mysql-6.0-wtf:mysql-6.0-wtf branch (vvaintroub:2653) Bug#24509,WL#3049Vladislav Vaintroub4 Jun
  • Re: commit into mysql-6.0-wtf:mysql-6.0-wtf branch (vvaintroub:2653)Bug#24509, WL#3049Davi Arnaut4 Jun
    • Re: commit into mysql-6.0-wtf:mysql-6.0-wtf branch(vvaintroub:2653) Bug#24509, WL#3049Konstantin Osipov4 Jun
    • Re: commit into mysql-6.0-wtf:mysql-6.0-wtf branch (vvaintroub:2653)Bug#24509, WL#3049Joerg Bruehe4 Jun
      • RE: commit into mysql-6.0-wtf:mysql-6.0-wtf branch (vvaintroub:2653) Bug#24509, WL#3049Vladislav Vaintroub4 Jun
        • Re: commit into mysql-6.0-wtf:mysql-6.0-wtf branch (vvaintroub:2653)Bug#24509, WL#3049Davi Arnaut4 Jun
          • RE: commit into mysql-6.0-wtf:mysql-6.0-wtf branch (vvaintroub:2653) Bug#24509, WL#3049Vladislav Vaintroub4 Jun
            • Re: commit into mysql-6.0-wtf:mysql-6.0-wtf branch (vvaintroub:2653)Bug#24509, WL#3049Davi Arnaut4 Jun
        • Re: commit into mysql-6.0-wtf:mysql-6.0-wtf branch (vvaintroub:2653)Bug#24509, WL#3049Joerg Bruehe4 Jun
          • Re: commit into mysql-6.0-wtf:mysql-6.0-wtf branch (vvaintroub:2653)Bug#24509, WL#3049 [ifdefery]Davi Arnaut5 Jun
      • Re: commit into mysql-6.0-wtf:mysql-6.0-wtf branch (vvaintroub:2653)Bug#24509, WL#3049Davi Arnaut4 Jun