Attached are a first stab at some tests for native integer serialisation.
They pass on the JVM, and on MoarVM (tested 64 bit and 32 bit Linux) with
patching. Specifically the two commits from the branch maybe-varintfix plus
the attached patch that fixes some varint serialisation bugs.

The tests aren't correct *yet*. They fail like this on Parrot:

ok 65 - string array second element is correct
ok 66 - string array third element is correct
Can only use nqp_set_sc_for_object with a SixModelObject
current instr.: 'add_to_sc' pc 359 ((file unknown):597886677) 
(t/serialization/01-basic.t:5)
called from Sub 'round_trip_int_array' pc 439 ((file unknown):597886721) 
(t/serialization/01-basic.t:393)
called from Sub '' pc 5474 ((file unknown):597888559) 
(t/serialization/01-basic.t:418)
called from Sub '' pc 317 ((file unknown):597886655) 
(t/serialization/01-basic.t:1)
called from Sub '' pc 28306 (gen/parrot/stage2/NQPHLL.pir:11153) 
(gen/parrot/stage2/NQPHLL.nqp:1145)
called from Sub 'eval' pc 27960 (gen/parrot/stage2/NQPHLL.pir:11012) 
(gen/parrot/stage2/NQPHLL.nqp:1132)
called from Sub 'evalfiles' pc 31162 (gen/parrot/stage2/NQPHLL.pir:12374) 
(gen/parrot/stage2/NQPHLL.nqp:1338)
called from Sub 'command_eval' pc 29962 (gen/parrot/stage2/NQPHLL.pir:11900) 
(gen/parrot/stage2/NQPHLL.nqp:1267)
called from Sub 'command_line' pc 28998 (gen/parrot/stage2/NQPHLL.pir:11499) 
(gen/parrot/stage2/NQPHLL.nqp:1217)
called from Sub 'MAIN' pc 601 (gen/parrot/stage2/NQP.pir:204) 
(gen/parrot/stage2/NQP.nqp:3674)
called from Sub '' pc 578 (gen/parrot/stage2/NQP.pir:191) 
(gen/parrot/stage2/NQP.nqp:3669)
called from Sub '' pc 160656 (gen/parrot/stage2/NQP.pir:57324) 
(gen/parrot/stage2/NQP.nqp:3646)


The problem seems to be serialising an array. Is the correct trick to make
them work to refactor them to use an object with an array attribute?


I don't think that fixing the tests should hold up fixing MoarVM, if the
C code patch is acceptable.

Nicholas Clark
>From c1e8b27bb390b3aa1e129ac2b2f114e4628e0467 Mon Sep 17 00:00:00 2001
From: Nicholas Clark <n...@ccl4.org>
Date: Tue, 11 Feb 2014 09:12:17 +0100
Subject: [PATCH 1/3] Basic serialization tests for integers.

Start by testing that -258 .. 258 round trip correctly.
---
 t/serialization/01-basic.t | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/t/serialization/01-basic.t b/t/serialization/01-basic.t
index 6131955..e6d7805 100644
--- a/t/serialization/01-basic.t
+++ b/t/serialization/01-basic.t
@@ -1,6 +1,6 @@
 #! nqp
 
-plan(66);
+plan(585);
 
 sub add_to_sc($sc, $idx, $obj) {
     nqp::scsetobj($sc, $idx, $obj);
@@ -380,3 +380,34 @@ sub add_to_sc($sc, $idx, $obj) {
     ok(nqp::atpos_s(nqp::scgetobj($dsc, 0).a, 1) eq 'sheep', 'string array second element is correct');
     ok(nqp::atpos_s(nqp::scgetobj($dsc, 0).a, 2) eq 'pig',   'string array third element is correct');
 }
+
+# integers
+{
+    my @a;
+
+    my int $i := -258;
+    while ($i <= 258) {
+        nqp::push(@a, $i);
+        $i := $i + 1;
+    }
+    my $elems := nqp::elems(@a);
+
+    my $sc := nqp::createsc('TEST_SC_13_IN');
+    my $sh := nqp::list_s();
+
+    add_to_sc($sc, 0, @a);
+
+    my $serialized := nqp::serialize($sc, $sh);
+
+    my $dsc := nqp::createsc('TEST_SC_13_OUT');
+    nqp::deserialize($serialized, $dsc, $sh, nqp::list(), nqp::null());
+
+    ok(nqp::istype(nqp::scgetobj($dsc, 0), Array), 'deserialized object has correct type');
+    my $j := 0;
+    my @b := nqp::scgetobj($dsc, 0);
+    ok(nqp::elems(@b) == $elems, 'array came back with correct element count');
+    while ($j < $elems) {
+        ok(@b[$j] == @a[$j], 'integer ' ~ @a[$j] ~ ' serialization round trip (' ~ $j ~ ')');
+        ++$j;
+    }
+}
-- 
1.8.4.2

>From 9e752e68d15368cfc91acbc8cb949914cd0e246a Mon Sep 17 00:00:00 2001
From: Nicholas Clark <n...@ccl4.org>
Date: Tue, 11 Feb 2014 12:10:04 +0100
Subject: [PATCH 2/3] Serialization tests for integers up to 2**62.

---
 t/serialization/01-basic.t | 52 +++++++++++++++++++++++++++++++++-------------
 1 file changed, 38 insertions(+), 14 deletions(-)

diff --git a/t/serialization/01-basic.t b/t/serialization/01-basic.t
index e6d7805..6b6c0bb 100644
--- a/t/serialization/01-basic.t
+++ b/t/serialization/01-basic.t
@@ -1,6 +1,6 @@
 #! nqp
 
-plan(585);
+plan(1341);
 
 sub add_to_sc($sc, $idx, $obj) {
     nqp::scsetobj($sc, $idx, $obj);
@@ -382,32 +382,56 @@ sub add_to_sc($sc, $idx, $obj) {
 }
 
 # integers
-{
-    my @a;
-
-    my int $i := -258;
-    while ($i <= 258) {
-        nqp::push(@a, $i);
-        $i := $i + 1;
-    }
+sub round_trip_int_array($seq, $desc, @a) {
+    $seq := 'TEST_SC_' ~ $seq;
+    $desc := 'for ' ~ $desc ~ ', ';
     my $elems := nqp::elems(@a);
 
-    my $sc := nqp::createsc('TEST_SC_13_IN');
+    my $sc := nqp::createsc($seq ~ '_IN');
     my $sh := nqp::list_s();
 
     add_to_sc($sc, 0, @a);
 
     my $serialized := nqp::serialize($sc, $sh);
 
-    my $dsc := nqp::createsc('TEST_SC_13_OUT');
+    my $dsc := nqp::createsc($seq ~ '_OUT');
     nqp::deserialize($serialized, $dsc, $sh, nqp::list(), nqp::null());
 
-    ok(nqp::istype(nqp::scgetobj($dsc, 0), Array), 'deserialized object has correct type');
+    ok(nqp::istype(nqp::scgetobj($dsc, 0), Array), $desc ~ 'deserialized object has correct type');
     my $j := 0;
     my @b := nqp::scgetobj($dsc, 0);
-    ok(nqp::elems(@b) == $elems, 'array came back with correct element count');
+    ok(nqp::elems(@b) == $elems, $desc ~ 'array came back with correct element count');
     while ($j < $elems) {
-        ok(@b[$j] == @a[$j], 'integer ' ~ @a[$j] ~ ' serialization round trip (' ~ $j ~ ')');
+        ok(nqp::iseq_i(@b[$j], @a[$j]), $desc ~ 'integer ' ~ @a[$j] ~ ' serialization round trip (' ~ $j ~ ')');
         ++$j;
     }
 }
+
+{
+    my @a;
+
+    my int $i := -258;
+    while ($i <= 258) {
+        nqp::push(@a, $i);
+        $i := $i + 1;
+    }
+    round_trip_int_array(13, 'small integers', @a);
+}
+
+{
+    my $i := 9;
+    my int $b := 512;
+
+    while ($i < 63) {
+        my @a;
+        my int $j := nqp::sub_i($b, 4);
+        while (nqp::islt_i($j, nqp::add_i($b, 2))) {
+            nqp::push(@a, $j);
+            nqp::push(@a, nqp::neg_i($j));
+            $j := nqp::add_i($j, 1);
+        }
+        round_trip_int_array($i + 7, 'integers around 2 ** ' ~ $i, @a);
+        ++$i;
+        $b := nqp::add_i($b, $b);
+    }
+}
-- 
1.8.4.2

>From 7ff40c7225a7970461dae47e4ac750d57e039c18 Mon Sep 17 00:00:00 2001
From: Nicholas Clark <n...@ccl4.org>
Date: Tue, 11 Feb 2014 15:25:10 +0100
Subject: [PATCH 3/3] Serialization tests for integers around 2**63, and other
 interesting values.

---
 t/serialization/01-basic.t | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/t/serialization/01-basic.t b/t/serialization/01-basic.t
index 6b6c0bb..50d9b71 100644
--- a/t/serialization/01-basic.t
+++ b/t/serialization/01-basic.t
@@ -1,6 +1,6 @@
 #! nqp
 
-plan(1341);
+plan(1355);
 
 sub add_to_sc($sc, $idx, $obj) {
     nqp::scsetobj($sc, $idx, $obj);
@@ -435,3 +435,31 @@ sub round_trip_int_array($seq, $desc, @a) {
         $b := nqp::add_i($b, $b);
     }
 }
+
+{
+    # values around 2 ** 63, and interesting bit patterns.
+    # Need to do these as BigInts parsing strings due to a bug in nqp::radix
+    my @s := (
+               '9223372036854775805', # 0x7FFFFFFFFFFFFFFD
+               '9223372036854775806', # 0x7FFFFFFFFFFFFFFE
+               '9223372036854775807', # 0x7FFFFFFFFFFFFFFF
+              '-9223372036854775808', # 0x8000000000000000
+              '-9223372036854775807', # 0x8000000000000001
+              '-9223372036854775806', # 0x8000000000000002
+                '-81985529216486896', # 0xFEDCBA9876543210
+               '1147797409030816545', # 0x0FEDCBA987654321
+              '-6148914691236517206', # 0xAAAAAAAAAAAAAAAA
+               '6148914691236517205', # 0x5555555555555555
+              '-6510615555426900571', # 0xA5A5A5A5A5A5A5A5
+               '6510615555426900570', # 0x5A5A5A5A5A5A5A5A
+             );
+    my $bi_type := nqp::knowhow().new_type(:name("TestBigInt"), :repr("P6bigint"));
+    $bi_type.HOW.compose($bi_type);
+    my @a;
+    for (@s) {
+        my $bi := nqp::fromstr_I($_, $bi_type);
+        nqp::push(@a, nqp::unbox_i($bi));
+    }
+
+    round_trip_int_array(70, 'special case integers', @a);
+}
-- 
1.8.4.2

>From b20f8f328c6155019702cf44d27ed10a4251ac21 Mon Sep 17 00:00:00 2001
From: Nicholas Clark <n...@ccl4.org>
Date: Tue, 11 Feb 2014 17:40:54 +0100
Subject: [PATCH] Fix various errors in varint serialisation code.

write_varint9() was writing a 9th byte when only 8 should have been written.
read_varint9() was reading a 9th byte when only 8 should have been read.

It was also failing to perform its bit shifting operations using 64 bit types,
and had an off-by one error in the shifting if a 9th byte was read.
---
 src/6model/serialization.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/src/6model/serialization.c b/src/6model/serialization.c
index 8ed67e6..380d1b9 100644
--- a/src/6model/serialization.c
+++ b/src/6model/serialization.c
@@ -231,7 +231,8 @@ static size_t write_varint9(MVMuint8 *buffer, size_t offset, int64_t value) {
         if (position != needed_bytes - 1) buffer[offset + position] = buffer[offset + position] | 0x80;
         value = value >> 7;
     }
-    if (position == 8) {
+    if (needed_bytes == 9) {
+        assert(position == 8);
         buffer[offset + position] = value;
     }
     return needed_bytes;
@@ -1213,8 +1214,8 @@ static size_t read_varint9(MVMuint8 *buffer, size_t offset, int64_t *value) {
     int read_on = !!(buffer[offset] & 0x80) + 1;
     *value = 0;
     while (read_on && inner_offset != 8) {
-        *value = *value | ((buffer[offset + inner_offset] & 0x7F) << shift_amount);
-        negation_mask = negation_mask | (0x7F << shift_amount);
+        *value = *value | ((int64_t)(buffer[offset + inner_offset] & 0x7F) << shift_amount);
+        negation_mask = negation_mask | ((int64_t)0x7F << shift_amount);
         if (read_on == 1 && buffer[offset + inner_offset] & 0x80) {
             read_on = 2;
         }
@@ -1223,10 +1224,10 @@ static size_t read_varint9(MVMuint8 *buffer, size_t offset, int64_t *value) {
         shift_amount += 7;
     }
     // our last byte will be a full byte, so that we reach the full 64 bits
-    if (inner_offset == 8) {
-        shift_amount += 1;
-        *value = *value | (buffer[offset + inner_offset] << shift_amount);
-        negation_mask = negation_mask | (0x7F << shift_amount);
+    if (read_on && inner_offset == 8) {
+        *value = *value | ((int64_t)buffer[offset + inner_offset] << shift_amount);
+        negation_mask = negation_mask | ((int64_t)0xFF << shift_amount);
+        ++inner_offset;
     }
     negation_mask = negation_mask >> 1;
     // do we have a negative number so far?
-- 
1.8.4.2

Reply via email to