Repository: weld-project/weld (language: rust)

Issue:
Weld Serialize/Deserialize
I am using Weld's `serialize(data)` and  `deserialize[T](data)` in a project. As most other libraries handle bytes as `Vec<u8>`, I am currently having to cast between i8/u8 while going from Weld to other libraries (e.g., RocksDB, Protobuf) and vice versa. Is there a motivation behind the choice of `Vec<i8>`?

I could look into creating a PR for this. Or if it would be breaking other projects relying on the current implementation, I could work on a fork instead.

Cheers


Discussion / hints:
Hi Max,

You can submit a PR for this if you’d like — I think u8 makes more sense
too and I don’t think it’ll break anything else.

Shoumik

On Fri, May 3, 2019 at 4:56 AM Max Meldrum <notifications@github.com> wrote:

> I am using Weld's serialize(data) and deserialize[T](data) in a project.
> As most other libraries handle bytes as Vec<u8>, I am currently having to
> cast between i8/u8 while going from Weld to other libraries (e.g., RocksDB,
> Protobuf) and vice versa. Is there a motivation behind the choice of
> Vec<i8>?
>
> I could look into creating a PR for this. Or if it would be breaking other
> projects relying on the current implementation, I could work on a fork
> instead.
>
> Cheers
>
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/weld-project/weld/issues/446>, or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AAKMEY3MVHD4LFGXD353JILPTP44PANCNFSM4HKSHVOA>
> .
>
-- 
Shoumik


Fix patch:
diff --git a/docs/language.md b/docs/language.md
index 1634d03ba..abb44d1dc 100644
--- a/docs/language.md
+++ b/docs/language.md
@@ -130,8 +130,8 @@ refer to types.
   value until the boolean it returns is `false`, and then return the `T` field in
   its output as the final value of the expression.
 * `cudf[name,ty](args)` to call arbitrary C-style functions (see a discussion of UDFs [below](#user-defined-functions)).
-* `serialize(data)` serializes `data` into a `vec[i8]`. The data in this vector can be written to disk, sent over the network, etc.
-* `deserialize[T](data)` deserializes `data` (a `vec[i8]`) into a value of type `T`.
+* `serialize(data)` serializes `data` into a `vec[u8]`. The data in this vector can be written to disk, sent over the network, etc.
+* `deserialize[T](data)` deserializes `data` (a `vec[u8]`) into a value of type `T`.
 * Casting: `T(data)` implements a cast between scalar types if `T` is a scalar and `data` is also a scalar type.
 * `broadcast(data)` takes a scalar value `data` and broadcasts the value into a SIMD type.
 * `assert(value)` takes a boolean value and checks that it is `true`. If so, the expression itself returns `true`. Otherwise, an error is thrown and the program terminates.
diff --git a/docs/serialization.md b/docs/serialization.md
index 58e2f7445..904f94df8 100644
--- a/docs/serialization.md
+++ b/docs/serialization.md
@@ -2,10 +2,10 @@
 
 Weld supports serialization of its data types. The language contains two operators to acheive this:
 
-* `serialize(data)` takes a value and returns a `vec[i8]`. This vector's data pointer contains a buffer with no
+* `serialize(data)` takes a value and returns a `vec[u8]`. This vector's data pointer contains a buffer with no
 pointers, so it can be written to disk or sent across the network. The length of the vector designates the number
 of bytes in the serialized buffer.
-* `deserialize[T](data)` takes as input a value of type `vec[i8]` and returns a Weld value of type `T`.
+* `deserialize[T](data)` takes as input a value of type `vec[u8]` and returns a Weld value of type `T`.
 
 Weld supports serialization of structs, vectors, scalars, and dictionaries, and
 *does not* support serialization of SIMD values and builders.
@@ -17,7 +17,7 @@ currently does not encode the type in the serialized buffer, so users who call
 `deserialize` must know the expected output type. This may be amended in a
 later release. Currently, passing an incorrect type to `deserialize` will
 result in undefined behavior, though if exactly every byte of the input
-`vec[i8]` is not consumed exactly by `deserialize`, the runtime will abort with
+`vec[u8]` is not consumed exactly by `deserialize`, the runtime will abort with
 a `DeserializationError`. Note that this check occurs after Weld attempts to
 deserialize the input buffer, so this does not prevent all unsafe
 behavior/protect against corrupt data. Each type in Weld is serialized as follows.
diff --git a/weld/src/ast/type_inference.rs b/weld/src/ast/type_inference.rs
index 1c08eee37..37753edc1 100644
--- a/weld/src/ast/type_inference.rs
+++ b/weld/src/ast/type_inference.rs
@@ -437,7 +437,7 @@ impl InferTypesInternal for Expr {
             CUDF { ref return_ty, .. } => self.ty.push(return_ty),
 
             Serialize(_) => {
-                let serialized_type = Vector(Box::new(Scalar(I8)));
+                let serialized_type = Vector(Box::new(Scalar(U8)));
                 self.ty.push_complete(serialized_type)
             }
 
diff --git a/weld/src/codegen/llvm/serde.rs b/weld/src/codegen/llvm/serde.rs
index 4e6c789a0..16802824c 100644
--- a/weld/src/codegen/llvm/serde.rs
+++ b/weld/src/codegen/llvm/serde.rs
@@ -21,8 +21,8 @@ use self::llvm_sys::prelude::*;
 use super::{CodeGenExt, FunctionContext, HasPointer, LlvmGenerator};
 
 lazy_static! {
-    /// The serialized type, which is a vec[i8].
-    static ref SER_TY: Type = Type::Vector(Box::new(Type::Scalar(ScalarKind::I8)));
+    /// The serialized type, which is a vec[u8].
+    static ref SER_TY: Type = Type::Vector(Box::new(Type::Scalar(ScalarKind::U8)));
     /// The type returned by the serialization function.
     static ref SER_RET_TY: Type = Type::Struct(vec![SER_TY.clone(), Scalar(ScalarKind::I64)]);
 }
@@ -428,7 +428,7 @@ impl SerHelper for LlvmGenerator {
                     let key_ser_fn = self.gen_serialize_fn(key)?;
                     let val_ser_fn = self.gen_serialize_fn(val)?;
                     let methods = self.dictionaries.get_mut(ty).unwrap();
-                    let buffer_vector = self.vectors.get_mut(&Scalar(ScalarKind::I8)).unwrap();
+                    let buffer_vector = self.vectors.get_mut(&Scalar(ScalarKind::U8)).unwrap();
                     methods.gen_serialize(
                         builder,
                         function,


Test patch:
diff --git a/examples/cpp/serialization/serialize_dictionary/serialize_test.cpp b/examples/cpp/serialization/serialize_dictionary/serialize_test.cpp
index 7dc07cc48..a32703440 100644
--- a/examples/cpp/serialization/serialize_dictionary/serialize_test.cpp
+++ b/examples/cpp/serialization/serialize_dictionary/serialize_test.cpp
@@ -37,8 +37,8 @@ const char *program_no_pointers = "|v:vec[{i32,i32}]| serialize(result(for(v, di
 const char *program_pointers = "|v:vec[{i32,i32}]| serialize(result(for(v, groupmerger[i32,i32], |b,i,e| merge(b, e))))";
 
 // Programs that deserialize outputs of the above programs.
-const char *deser_no_pointers = "|v:vec[i8]| tovec(deserialize[dict[i32,i32]](v))";
-const char *deser_pointers = "|v:vec[i8]| tovec(deserialize[dict[i32,vec[i32]]](v))";
+const char *deser_no_pointers = "|v:vec[u8]| tovec(deserialize[dict[i32,i32]](v))";
+const char *deser_pointers = "|v:vec[u8]| tovec(deserialize[dict[i32,vec[i32]]](v))";
 
 /** Runs a serialize test on a program that takes a vec[{i32,i32}] has input. */
 weld_value_t serialize_test(const char *program, SerializeCheckerFn checker) {
@@ -98,9 +98,9 @@ weld_value_t serialize_test(const char *program, SerializeCheckerFn checker) {
     return result;
 }
 
-/** Runs a serialize test on a program that takes a vec[i8] has input. */
+/** Runs a serialize test on a program that takes a vec[u8] has input. */
 void deserialize_test(const char *program,
-    weld_vec<int8_t> *a,
+    weld_vec<uint8_t> *a,
     SerializeCheckerFn checker) {
 
     weld_error_t e = weld_error_new();
@@ -143,9 +143,9 @@ void deserialize_test(const char *program,
 
 // Serialized as Length:i64, Key:i32, Value:i32, Key:i32, Value:i32, ...
 void check_nopointers(void *inp) {
-  weld_vec<int8_t> *res_vec = (weld_vec<int8_t> *)inp;
+  weld_vec<uint8_t> *res_vec = (weld_vec<uint8_t> *)inp;
   int64_t length = res_vec->length;
-  int8_t *data = res_vec->data;
+  uint8_t *data = res_vec->data;
 
   // Print the number of bytes in the serialized buffer.
   printf("Output data buffer length %lld\n", length);
@@ -161,7 +161,7 @@ void check_nopointers(void *inp) {
   }
 }
 
-int8_t *parse_veci32(int8_t *data) {
+uint8_t *parse_veci32(uint8_t *data) {
   int64_t length = *((int64_t *)data);
   data += sizeof(int64_t);
 
@@ -177,9 +177,9 @@ int8_t *parse_veci32(int8_t *data) {
 
 // Serialized as Length:i64, Key:i32, ValueLength:i64, Val1:i32, ..., Key:i32, ValLength:i64, Val1: i32, ...
 void check_pointers(void *inp) {
-  weld_vec<int8_t> *res_vec = (weld_vec<int8_t> *)inp;
+  weld_vec<uint8_t> *res_vec = (weld_vec<uint8_t> *)inp;
   int64_t length = res_vec->length;
-  int8_t *data = res_vec->data;
+  uint8_t *data = res_vec->data;
 
   int64_t size = *((int64_t *)data);
   data += sizeof(int64_t);
@@ -225,15 +225,16 @@ int main() {
   printf("Serializing Dictionary of type 'dict[i32,i32]':\n");
   weld_value_t no_ptrs = serialize_test(program_no_pointers, check_nopointers);
   printf("Deserializing Dictionary of type 'dict[i32,i32]':\n");
-  deserialize_test(deser_no_pointers, (weld_vec<int8_t> *)weld_value_data(no_ptrs), check_deserialize_nopointers);
+  deserialize_test(deser_no_pointers, (weld_vec<uint8_t> *)weld_value_data(no_ptrs), check_deserialize_nopointers);
   weld_value_free(no_ptrs);
   */
 
-  weld_set_log_level(WELD_LOG_LEVEL_INFO);
+  // set to INFO level
+  weld_set_log_level(3);
 
   weld_value_t ptrs = serialize_test(program_pointers, check_pointers);
   deserialize_test(deser_pointers,
-      (weld_vec<int8_t> *)weld_value_data(ptrs),
+      (weld_vec<uint8_t> *)weld_value_data(ptrs),
       check_deserialize_pointers);
   // weld_value_free(ptrs);
   printf("Success!\n");
diff --git a/examples/cpp/serialization/serialize_vec/serialize_test.cpp b/examples/cpp/serialization/serialize_vec/serialize_test.cpp
index 85a7e2875..835a43ec6 100644
--- a/examples/cpp/serialization/serialize_vec/serialize_test.cpp
+++ b/examples/cpp/serialization/serialize_vec/serialize_test.cpp
@@ -91,8 +91,8 @@ int main() {
         exit(1);
     }
 
-    weld_vec<int8_t> *res_vec = (weld_vec<int8_t> *)weld_value_data(result);
-    int8_t *result_data = res_vec->data;
+    weld_vec<uint8_t> *res_vec = (weld_vec<uint8_t> *)weld_value_data(result);
+    uint8_t *result_data = res_vec->data;
 
     printf("Output data buffer length %ld\n", res_vec->length);