Created: (HADOOP-1089) The c++ version of write and read v-int don't agree with the java versions

classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

Created: (HADOOP-1089) The c++ version of write and read v-int don't agree with the java versions

JIRA jira@apache.org
The c++ version of write and read v-int don't agree with the java versions
--------------------------------------------------------------------------

                 Key: HADOOP-1089
                 URL: https://issues.apache.org/jira/browse/HADOOP-1089
             Project: Hadoop
          Issue Type: Bug
          Components: record
    Affects Versions: 0.12.0
            Reporter: Owen O'Malley
         Assigned To: Owen O'Malley
             Fix For: 0.13.0


The serialization of vints is inconsistent between C++ and Java. Since the Java has been fixed recently, I'll move the C++ implementation to match the current Java implementation.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

Updated: (HADOOP-1089) The c++ version of write and read v-int don't agree with the java versions

JIRA jira@apache.org

     [ https://issues.apache.org/jira/browse/HADOOP-1089?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Milind Bhandarkar updated HADOOP-1089:
--------------------------------------

    Fix Version/s:     (was: 0.13.0)
                   0.12.1
           Status: Patch Available  (was: Open)

> The c++ version of write and read v-int don't agree with the java versions
> --------------------------------------------------------------------------
>
>                 Key: HADOOP-1089
>                 URL: https://issues.apache.org/jira/browse/HADOOP-1089
>             Project: Hadoop
>          Issue Type: Bug
>          Components: record
>    Affects Versions: 0.12.0
>            Reporter: Owen O'Malley
>         Assigned To: Milind Bhandarkar
>             Fix For: 0.12.1
>
>         Attachments: jute-patch.txt
>
>
> The serialization of vints is inconsistent between C++ and Java. Since the Java has been fixed recently, I'll move the C++ implementation to match the current Java implementation.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

Assigned: (HADOOP-1089) The c++ version of write and read v-int don't agree with the java versions

JIRA jira@apache.org
In reply to this post by JIRA jira@apache.org

     [ https://issues.apache.org/jira/browse/HADOOP-1089?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Milind Bhandarkar reassigned HADOOP-1089:
-----------------------------------------

    Assignee: Milind Bhandarkar  (was: Owen O'Malley)

> The c++ version of write and read v-int don't agree with the java versions
> --------------------------------------------------------------------------
>
>                 Key: HADOOP-1089
>                 URL: https://issues.apache.org/jira/browse/HADOOP-1089
>             Project: Hadoop
>          Issue Type: Bug
>          Components: record
>    Affects Versions: 0.12.0
>            Reporter: Owen O'Malley
>         Assigned To: Milind Bhandarkar
>             Fix For: 0.12.1
>
>         Attachments: jute-patch.txt
>
>
> The serialization of vints is inconsistent between C++ and Java. Since the Java has been fixed recently, I'll move the C++ implementation to match the current Java implementation.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

Updated: (HADOOP-1089) The c++ version of write and read v-int don't agree with the java versions

JIRA jira@apache.org
In reply to this post by JIRA jira@apache.org

     [ https://issues.apache.org/jira/browse/HADOOP-1089?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Milind Bhandarkar updated HADOOP-1089:
--------------------------------------

    Attachment: jute-patch.txt

This patch fixes the c++ vint serialization.
In addition it fixes issues in generated C++ code:
Getters and setters for map were not correctly generated.

> The c++ version of write and read v-int don't agree with the java versions
> --------------------------------------------------------------------------
>
>                 Key: HADOOP-1089
>                 URL: https://issues.apache.org/jira/browse/HADOOP-1089
>             Project: Hadoop
>          Issue Type: Bug
>          Components: record
>    Affects Versions: 0.12.0
>            Reporter: Owen O'Malley
>         Assigned To: Owen O'Malley
>             Fix For: 0.12.1
>
>         Attachments: jute-patch.txt
>
>
> The serialization of vints is inconsistent between C++ and Java. Since the Java has been fixed recently, I'll move the C++ implementation to match the current Java implementation.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

Commented: (HADOOP-1089) The c++ version of write and read v-int don't agree with the java versions

JIRA jira@apache.org
In reply to this post by JIRA jira@apache.org

    [ https://issues.apache.org/jira/browse/HADOOP-1089?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12480832 ]

Sameer Paranjpye commented on HADOOP-1089:
------------------------------------------

Looks good, but produces some warnings when compiled with -Wall. It'd be better if these were fixed.

> The c++ version of write and read v-int don't agree with the java versions
> --------------------------------------------------------------------------
>
>                 Key: HADOOP-1089
>                 URL: https://issues.apache.org/jira/browse/HADOOP-1089
>             Project: Hadoop
>          Issue Type: Bug
>          Components: record
>    Affects Versions: 0.12.0
>            Reporter: Owen O'Malley
>         Assigned To: Milind Bhandarkar
>             Fix For: 0.12.1
>
>         Attachments: jute-patch.txt
>
>
> The serialization of vints is inconsistent between C++ and Java. Since the Java has been fixed recently, I'll move the C++ implementation to match the current Java implementation.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

Updated: (HADOOP-1089) The c++ version of write and read v-int don't agree with the java versions

JIRA jira@apache.org
In reply to this post by JIRA jira@apache.org

     [ https://issues.apache.org/jira/browse/HADOOP-1089?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Milind Bhandarkar updated HADOOP-1089:
--------------------------------------

    Attachment:     (was: jute-patch.txt)

> The c++ version of write and read v-int don't agree with the java versions
> --------------------------------------------------------------------------
>
>                 Key: HADOOP-1089
>                 URL: https://issues.apache.org/jira/browse/HADOOP-1089
>             Project: Hadoop
>          Issue Type: Bug
>          Components: record
>    Affects Versions: 0.12.0
>            Reporter: Owen O'Malley
>         Assigned To: Milind Bhandarkar
>             Fix For: 0.12.1
>
>
> The serialization of vints is inconsistent between C++ and Java. Since the Java has been fixed recently, I'll move the C++ implementation to match the current Java implementation.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

Updated: (HADOOP-1089) The c++ version of write and read v-int don't agree with the java versions

JIRA jira@apache.org
In reply to this post by JIRA jira@apache.org

     [ https://issues.apache.org/jira/browse/HADOOP-1089?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Milind Bhandarkar updated HADOOP-1089:
--------------------------------------

    Attachment: jute-patch.txt

Fixed  warnings with -Wall. (In fact there were two real bugs in deserialization of buffer in XML and CSV, that -Wall pointed out.)

> The c++ version of write and read v-int don't agree with the java versions
> --------------------------------------------------------------------------
>
>                 Key: HADOOP-1089
>                 URL: https://issues.apache.org/jira/browse/HADOOP-1089
>             Project: Hadoop
>          Issue Type: Bug
>          Components: record
>    Affects Versions: 0.12.0
>            Reporter: Owen O'Malley
>         Assigned To: Milind Bhandarkar
>             Fix For: 0.12.1
>
>         Attachments: jute-patch.txt
>
>
> The serialization of vints is inconsistent between C++ and Java. Since the Java has been fixed recently, I'll move the C++ implementation to match the current Java implementation.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

Commented: (HADOOP-1089) The c++ version of write and read v-int don't agree with the java versions

JIRA jira@apache.org
In reply to this post by JIRA jira@apache.org

    [ https://issues.apache.org/jira/browse/HADOOP-1089?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12480860 ]

Sameer Paranjpye commented on HADOOP-1089:
------------------------------------------

Looks good.

+1

> The c++ version of write and read v-int don't agree with the java versions
> --------------------------------------------------------------------------
>
>                 Key: HADOOP-1089
>                 URL: https://issues.apache.org/jira/browse/HADOOP-1089
>             Project: Hadoop
>          Issue Type: Bug
>          Components: record
>    Affects Versions: 0.12.0
>            Reporter: Owen O'Malley
>         Assigned To: Milind Bhandarkar
>             Fix For: 0.12.1
>
>         Attachments: jute-patch.txt
>
>
> The serialization of vints is inconsistent between C++ and Java. Since the Java has been fixed recently, I'll move the C++ implementation to match the current Java implementation.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

Commented: (HADOOP-1089) The c++ version of write and read v-int don't agree with the java versions

JIRA jira@apache.org
In reply to this post by JIRA jira@apache.org

    [ https://issues.apache.org/jira/browse/HADOOP-1089?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12480864 ]

David Bowen commented on HADOOP-1089:
-------------------------------------


Scanning this patch I noticed a typo in an error message in a couple of files: csvarchive.cc and xmlarchive.cc both contain multiple occurrences of "Errror".

I suppose it is a bit late to question the format of negative Vints (again)?  There seems to be no necessity to do the ones-complement.  We could instead store bytes of negative ints up to the point where all the more-significant bytes are 255.  This would make the code a bit simpler and faster.

E.g. the current Java code for writing a Vint is this:

  public static void writeVLong(DataOutput stream, long i) throws IOException {
      if (i >= -112 && i <= 127) {
          stream.writeByte((byte)i);
          return;
      }
      int len = -112;
      if (i < 0) {
          i ^= -1L; // take one's complement'
          len = -120;
      }
      long tmp = i;
      while (tmp != 0) {
          tmp = tmp >> 8;
          len--;
      }
      stream.writeByte((byte)len);
      len = (len < -120) ? -(len + 120) : -(len + 112);
      for (int idx = len; idx != 0; idx--) {
          int shiftbits = (idx - 1) * 8;
          long mask = 0xFFL << shiftbits;
          stream.writeByte((byte)((i & mask) >> shiftbits));
      }
  }

It could be:

  public static void writeVLong(DataOutput stream, long i) throws IOException {
      if (i >= -120 && i <= 127) {
          stream.writeByte((byte)i);
          return;
      }
      int len = -120;
      long done = ( i < 0 ? -1L : 0L);
      long tmp = i;
      while (tmp != done) {
          tmp = tmp >> 8;
          len--;
      }
      stream.writeByte((byte)len);
      len += 120;
      for (int idx = len; idx != 0; idx--) {
          int shiftbits = (idx - 1) * 8;
          long mask = 0xFFL << shiftbits;
          stream.writeByte((byte)((i & mask) >> shiftbits));
      }
  }






> The c++ version of write and read v-int don't agree with the java versions
> --------------------------------------------------------------------------
>
>                 Key: HADOOP-1089
>                 URL: https://issues.apache.org/jira/browse/HADOOP-1089
>             Project: Hadoop
>          Issue Type: Bug
>          Components: record
>    Affects Versions: 0.12.0
>            Reporter: Owen O'Malley
>         Assigned To: Milind Bhandarkar
>             Fix For: 0.12.1
>
>         Attachments: jute-patch.txt
>
>
> The serialization of vints is inconsistent between C++ and Java. Since the Java has been fixed recently, I'll move the C++ implementation to match the current Java implementation.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

Commented: (HADOOP-1089) The c++ version of write and read v-int don't agree with the java versions

JIRA jira@apache.org
In reply to this post by JIRA jira@apache.org

    [ https://issues.apache.org/jira/browse/HADOOP-1089?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12480867 ]

David Bowen commented on HADOOP-1089:
-------------------------------------


Never mind.  That code is wrong.  (Consider 128.)  Although there might be a simpler fix than the one chosen...  it is probably not worth worrying about.  

> The c++ version of write and read v-int don't agree with the java versions
> --------------------------------------------------------------------------
>
>                 Key: HADOOP-1089
>                 URL: https://issues.apache.org/jira/browse/HADOOP-1089
>             Project: Hadoop
>          Issue Type: Bug
>          Components: record
>    Affects Versions: 0.12.0
>            Reporter: Owen O'Malley
>         Assigned To: Milind Bhandarkar
>             Fix For: 0.12.1
>
>         Attachments: jute-patch.txt
>
>
> The serialization of vints is inconsistent between C++ and Java. Since the Java has been fixed recently, I'll move the C++ implementation to match the current Java implementation.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

Updated: (HADOOP-1089) The c++ version of write and read v-int don't agree with the java versions

JIRA jira@apache.org
In reply to this post by JIRA jira@apache.org

     [ https://issues.apache.org/jira/browse/HADOOP-1089?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Tom White updated HADOOP-1089:
------------------------------

    Resolution: Fixed
        Status: Resolved  (was: Patch Available)

I've just committed this. Thanks Milind!

> The c++ version of write and read v-int don't agree with the java versions
> --------------------------------------------------------------------------
>
>                 Key: HADOOP-1089
>                 URL: https://issues.apache.org/jira/browse/HADOOP-1089
>             Project: Hadoop
>          Issue Type: Bug
>          Components: record
>    Affects Versions: 0.12.0
>            Reporter: Owen O'Malley
>         Assigned To: Milind Bhandarkar
>             Fix For: 0.12.1
>
>         Attachments: jute-patch.txt
>
>
> The serialization of vints is inconsistent between C++ and Java. Since the Java has been fixed recently, I'll move the C++ implementation to match the current Java implementation.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.