Protobuf [WIP] #12
Protobuf [WIP] #12
Conversation
|
Depend on #10 currently ... Just see share my work. |
|
Some compilation errors: |
lambda.opam
Outdated
| @@ -9,6 +9,7 @@ bug-reports: "https://github.com/samoht/mirage-lambda/issues" | |||
| build: [ "jbuilder" "build" "-p" name "-j" jobs ] | |||
| depends: [ | |||
| "jbuilder" {build} | |||
| "lambda-protobuf" | |||
There was a problem hiding this comment.
why do you need this in the opam file?
There was a problem hiding this comment.
and if you do need it, don't forget to pin it in .travis.yml too!
proto/lambda.proto
Outdated
| required string name = 1; | ||
| repeated Type arguments = 2; | ||
| required Type return = 3; | ||
| required int32 smartptr = 4; |
There was a problem hiding this comment.
What value should the Haskell side provide for the smartptr field? Maybe this field should not be serialized, but resolved by the Mirage server based on the name field?
There was a problem hiding this comment.
The idea behind smartptr is to bind (on the MirageOS/OCaml side) an int64 with functions/primitives. Then, when we deserialize, we try to find this identifier in an internal map of the unikernel. So, currently the identifier is an int64 to be close to the idea of how to share a function between two processes. However, we can choose another identifier (like a string if you want).
On the haskell side, you just need to have a Map int64 ([value] -> value) (or, if you prefer a string, a Map String ([value] -> value).
There was a problem hiding this comment.
I see, so the smartptr is just the index into the array of primitives? E.g. here it would be index 4 for Block.read? The name field is ignored in that case? I feel a mapping from strings to primitives might be easier to maintain.
There was a problem hiding this comment.
Exactly, so I will change the key to be a string 👍
| message Int64 { } | ||
| message Bool { } | ||
| message String { } | ||
| message Lwt { } |
There was a problem hiding this comment.
How to handle values of Lwt type? E.g. some block operations have lwt return types. Is the value serialization not implemented, yet, or am I overlooking something?
There was a problem hiding this comment.
You need to use Apply and Lwt (like Apply (Int, Lwt)). This about a limitation of the OCaml type-system (and the higher kind polymorphism). We can provide a sugar about that if you want.
There was a problem hiding this comment.
Okay, thanks. Just to verify, would the following be a correct encoding of the program
λ (output_block : page_aligned_buffer).
Block.read 42 output_block
(using the definitions from here)
lam {
typ { abstract { witness: "Cstruct.t" } }
var: "output_block"
expr {
app {
a {
app {
a {
prm {
value {
name: "Block.read"
arguments { int64 { } }
arguments { list { value { abstract { witness: "Cstruct.t" } } } }
return {
apply {
a {
result { a { unit { } } b { abstract { witness: "Block.error" } } }
}
b { lwt { } }
}
}
smartptr: 4
}
}
}
b { val { value { int64 { value: 42 } } } }
}
}
b { var { var: 0 } }
}
}
}
There was a problem hiding this comment.
Exactly 👍 , as I said, we can provide a sugar and translate lwt { something } to apply { a { something } b { lwt { } } internally if it's better for you.
| @@ -0,0 +1,195 @@ | |||
| message Type { | |||
There was a problem hiding this comment.
protoc complains about a missing line syntax = "proto2";.
|
Thanks for the updates. I have another question: There are two parts to this. |
|
As let info = Type.abstract "Mirage_block.info" in
let primitives = [
primitive "read_write" [info] Type.bool (fun x -> x.Mirage_block.read_write);
...
] in |
|
I've rebased on top of master -- my plan is to fix the CI and then merge the PR. We can improve it (use first-class bind/return + syntax for records) in subsequent PRs. |
Sounds good, defining dedicated primitives is a viable work-around for a start. I tried compiling the code on revision 5c5fcad. This is the compilation output with the error message at the end: |
|
This error is very strange. I've just pushed a Dockerfile (and pushed |
Yes, that would be great! You can find a description of the wire-protocol I have in mind here. The corresponding protobufs definition is here. Let me know if there's something that you think should be changed. |
|
@aherrmann the protocol looks great to me! Do you mind adding a copy in that repo too (with the description + request.proto)? |
No description provided.