ZooKeeper
  1. ZooKeeper
  2. ZOOKEEPER-1635

ZooKeeper C client doesn't compile on 64 bit Windows

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Invalid
    • Affects Version/s: None
    • Fix Version/s: 3.5.0
    • Component/s: None
    • Labels:
      None
    • Environment:

      Windows x64 systems.

      Description

      x64 target does not support _asm inline (See: http://msdn.microsoft.com/en-us/library/4ks26t93(v=vs.80).aspx)

      The proposal is to use native windows function which still valid for i386 and x64 architecture.

      In order to avoid any potential break, a compilation directive has been added. But, the best should be the removal of the asm part.

      -----------
      sample code
      -----------

      int32_t fetch_and_add(volatile int32_t* operand, int incr)
      {
      #ifndef WIN32
      int32_t result;
      asm _volatile_(
      "lock xaddl %0,%1\n"
      : "=r"(result), "=m"(*(int *)operand)
      : "0"(incr)
      : "memory");
      return result;
      #else

      #ifdef WIN32_NOASM
      InterlockedExchangeAdd(operand, incr);
      return *operand;
      #else
      volatile int32_t result;
      _asm

      { mov eax, operand; //eax = v; mov ebx, incr; // ebx = i; mov ecx, 0x0; // ecx = 0; lock xadd dword ptr [eax], ecx; lock xadd dword ptr [eax], ebx; mov result, ecx; // result = ebx; }

      return result;*/
      #endif

      #endif
      }

        Activity

        Tomas Gutierrez created issue -
        Tomas Gutierrez made changes -
        Field Original Value New Value
        Description x64 target does not support _asm inline (See: http://msdn.microsoft.com/en-us/library/4ks26t93(v=vs.80).aspx)

        The proposal is to use native windows function which still valid for i386 and x64 architecture.

        In order to avoid any potential break, a compilation directive has been added. But, the best should be the removal of the asm part.
        x64 target does not support _asm inline (See: http://msdn.microsoft.com/en-us/library/4ks26t93(v=vs.80).aspx)

        The proposal is to use native windows function which still valid for i386 and x64 architecture.

        In order to avoid any potential break, a compilation directive has been added. But, the best should be the removal of the asm part.


        -----------
        sample code
        -----------


        int32_t fetch_and_add(volatile int32_t* operand, int incr)
        {
        #ifndef WIN32
            int32_t result;
            asm __volatile__(
                 "lock xaddl %0,%1\n"
                 : "=r"(result), "=m"(*(int *)operand)
                 : "0"(incr)
                 : "memory");
           return result;
        #else

        #ifdef WIN32_NOASM
                        InterlockedExchangeAdd(operand, incr);
                        return *operand;
        #else
            volatile int32_t result;
            _asm
            {
                mov eax, operand; //eax = v;
               mov ebx, incr; // ebx = i;
                mov ecx, 0x0; // ecx = 0;
                lock xadd dword ptr [eax], ecx;
               lock xadd dword ptr [eax], ebx;
                mov result, ecx; // result = ebx;
             }
             return result;*/
        #endif

        #endif
        }
        Tomas Gutierrez made changes -
        Fix Version/s 3.5.0 [ 12316644 ]
        Hide
        Timothy Chen added a comment -

        Does this issue prevent ZK to run on x64 windows?

        Show
        Timothy Chen added a comment - Does this issue prevent ZK to run on x64 windows?
        Hide
        Flavio Junqueira added a comment -

        Tomas Gutierrez, Could you provide a patch, please?

        Show
        Flavio Junqueira added a comment - Tomas Gutierrez , Could you provide a patch, please?
        Hide
        Joe Gamache added a comment -

        Can anyone tell the rest of us, does 'support' in the title mean production support or development support?

        thanks!

        Show
        Joe Gamache added a comment - Can anyone tell the rest of us, does 'support' in the title mean production support or development support? thanks!
        Hide
        Tomas Gutierrez added a comment -

        It should be both I think. Not really sure why this matter

        Show
        Tomas Gutierrez added a comment - It should be both I think. Not really sure why this matter
        Hide
        Joe Gamache added a comment -

        As for why it matters, read

        here: http://mail-archives.apache.org/mod_mbox/zookeeper-user/201307.mbox/%3CCANLc_9LTYw7Q2Zte1vXdJMG_VTuXXK1Un2kvtny1LY=Ah3kB4A@mail.gmail.com%3E

        and here: http://osdir.com/ml/java-hadoop-zookeeper-user/2010-03/msg00116.html

        So for some people when the Admin Guide says it is not production ready there is understandable recalcitrance to usage in a production mode...

        Show
        Joe Gamache added a comment - As for why it matters, read here: http://mail-archives.apache.org/mod_mbox/zookeeper-user/201307.mbox/%3CCANLc_9LTYw7Q2Zte1vXdJMG_VTuXXK1Un2kvtny1LY=Ah3kB4A@mail.gmail.com%3E and here: http://osdir.com/ml/java-hadoop-zookeeper-user/2010-03/msg00116.html So for some people when the Admin Guide says it is not production ready there is understandable recalcitrance to usage in a production mode...
        Hide
        Flavio Junqueira added a comment -

        when the Admin Guide says it is not production ready there is understandable recalcitrance to usage in a production mode

        I'm not sure which admin guide you're referring to, I can only see e-mail pointers. In any case, we have worked on a couple of patches to fix the windows build on both trunk and 3.4 branch.

        Show
        Flavio Junqueira added a comment - when the Admin Guide says it is not production ready there is understandable recalcitrance to usage in a production mode I'm not sure which admin guide you're referring to, I can only see e-mail pointers. In any case, we have worked on a couple of patches to fix the windows build on both trunk and 3.4 branch.
        Hide
        Michi Mutsuzaki added a comment -

        We don't use _asm any more. The current fetch_and_add code looks like this:

        int32_t fetch_and_add(volatile int32_t* operand, int incr)
        {
        #ifndef WIN32
            return __sync_fetch_and_add(operand, incr);
        #else
            return InterlockedExchangeAdd(operand, incr);
        #endif
        }
        

        This should work on 64 bit windows, no?

        Show
        Michi Mutsuzaki added a comment - We don't use _asm any more. The current fetch_and_add code looks like this: int32_t fetch_and_add( volatile int32_t* operand, int incr) { #ifndef WIN32 return __sync_fetch_and_add(operand, incr); # else return InterlockedExchangeAdd(operand, incr); #endif } This should work on 64 bit windows, no?
        Hide
        Salabhanjika added a comment -

        I'm not sure which admin guide you're referring to, I can only see e-mail pointers. In any case, we have worked on a couple of patches to fix the windows build on both trunk and 3.4 branch.

        Flavio Junqueira, supported platforms section under ZooKeeper admin guide claims "Development Only" support only for Windows (Win32 only)
        http://zookeeper.apache.org/doc/trunk/zookeeperAdmin.html#sc_supportedPlatforms

        Show
        Salabhanjika added a comment - I'm not sure which admin guide you're referring to, I can only see e-mail pointers. In any case, we have worked on a couple of patches to fix the windows build on both trunk and 3.4 branch. Flavio Junqueira , supported platforms section under ZooKeeper admin guide claims "Development Only" support only for Windows (Win32 only) http://zookeeper.apache.org/doc/trunk/zookeeperAdmin.html#sc_supportedPlatforms
        Hide
        Michi Mutsuzaki added a comment -

        The client now uses InterlockedExchangeAdd.

        Show
        Michi Mutsuzaki added a comment - The client now uses InterlockedExchangeAdd.
        Michi Mutsuzaki made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Resolution Invalid [ 6 ]
        Hide
        Joe Gamache added a comment -

        While the description of this bug clearly calls for one type of implementation, the title does not. The remarks made simply state something, probably unbeknown to the vast majority of readers, is being used. The resolution is "invalid". One branch of logic would then state that "Support x64 architecture for Windows" is "Invalid". While this seems clear because the link above to supported platforms still clearly states that windows is for "Development Only", can some from ZooKeeper just confirm?

        Thank you.

        Show
        Joe Gamache added a comment - While the description of this bug clearly calls for one type of implementation, the title does not. The remarks made simply state something, probably unbeknown to the vast majority of readers, is being used. The resolution is "invalid". One branch of logic would then state that "Support x64 architecture for Windows" is "Invalid". While this seems clear because the link above to supported platforms still clearly states that windows is for "Development Only", can some from ZooKeeper just confirm? Thank you.
        Hide
        Michi Mutsuzaki added a comment -

        That's a fair point. I'll change the title to something like "zookeeper c client doesn't compile on 64 bit windows".

        I think we should update the doc to say "Win32/Win64 is supported as a development platform only for both server and client." Having said that, I think there are people using ZooKeeper on windows in production. Also note the a lot of work has gone in to make ZooKeeper on windows more robust. Maybe somebody more experienced with windows deployment can chime in?

        https://issues.apache.org/jira/browse/ZOOKEEPER-1833

        Show
        Michi Mutsuzaki added a comment - That's a fair point. I'll change the title to something like "zookeeper c client doesn't compile on 64 bit windows". I think we should update the doc to say "Win32/Win64 is supported as a development platform only for both server and client." Having said that, I think there are people using ZooKeeper on windows in production. Also note the a lot of work has gone in to make ZooKeeper on windows more robust. Maybe somebody more experienced with windows deployment can chime in? https://issues.apache.org/jira/browse/ZOOKEEPER-1833
        Michi Mutsuzaki made changes -
        Summary Support x64 architecture for Windows ZooKeeper C client doesn't compile on 64 bit Windows
        Hide
        Michi Mutsuzaki added a comment -

        I opened a new JIRA to add windows 64 bit as a supported platform for development.

        https://issues.apache.org/jira/browse/ZOOKEEPER-1918

        Show
        Michi Mutsuzaki added a comment - I opened a new JIRA to add windows 64 bit as a supported platform for development. https://issues.apache.org/jira/browse/ZOOKEEPER-1918
        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open Resolved Resolved
        448d 1h 1 Michi Mutsuzaki 23/Apr/14 23:02

          People

          • Assignee:
            Unassigned
            Reporter:
            Tomas Gutierrez
          • Votes:
            10 Vote for this issue
            Watchers:
            8 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development