Skip to content
This repository has been archived by the owner on Jun 21, 2024. It is now read-only.

Marshalling and unmarshalling #75

Closed
stedolan opened this issue Nov 23, 2016 · 7 comments
Closed

Marshalling and unmarshalling #75

stedolan opened this issue Nov 23, 2016 · 7 comments
Labels
bits-to-upstream Items to be upstreamed before multicore PR. Equivalent to "multicore-prerequisite" label in trunk. broken

Comments

@stedolan
Copy link
Contributor

stedolan commented Nov 23, 2016

Marshalling has various problems in multicore (the current implementation has just enough hacks to get the compiler to bootstrap). Specifically:

  • Marshalling of closures is broken
    Code pointers are treated specially by marshalling. In stock OCaml, code pointers are detected by using the page table. In multicore, there is no page table. Some changes to the closure representation are needed to allow the marshalling code to reliably detect code pointers.

  • Marshalling of Custom_tag objects is strange Fixed by Custom object intern fixes #224 and Make marshalled Custom_tag objects store their length. ocaml/ocaml#1683
    The API for marshalling Custom_tag objects is a bit odd: the format does not store the length and custom unmarshallers are passed a large memory region of unknown size in which to unmarshal. This works in stock OCaml because the unmarshaller knows about the heap format, but it doesn't work in multicore. Multicore requires either API changes (done, but not all clients updated), or a marshal format that stores length (probably a better option, since the old API can be kept)

  • Marshalling of concurrently-modified objects is unsafe
    There's no particular care taken at the moment to correctly marshal objects being mutated on a different domain. I don't expect the marshaller to give sensible output in this case, but it shouldn't segfault (the current marshaller can crash, in principle, if other domains modify objects being marshalled and a full GC cycle occurs during marshalling, but this is hard to trigger).

Some of the changes to marshalling might be worth upstreaming to stock OCaml independently of multicore.

@kayceesrk kayceesrk added this to the multicore-alpha milestone Jan 16, 2017
@dhil
Copy link
Collaborator

dhil commented May 2, 2017

I'd be interested in fixing at least marshalling of closures. In order to give me some idea could you elaborate on what changes to the closure representation are necessary?

@stedolan
Copy link
Contributor Author

stedolan commented May 2, 2017

The issue with marshalling closures is exactly the one being fixed by ocaml/ocaml#1156

@dhil
Copy link
Collaborator

dhil commented May 2, 2017

Great! I suppose we can back port it once it has been merged.

@kayceesrk
Copy link
Contributor

kayceesrk commented May 8, 2018

The PR ocaml/ocaml#1683 upstream is also necessary to fix marshalling. @stedolan have you gotten around to implementing the varint variant?

@kayceesrk kayceesrk removed this from the multicore-alpha milestone May 10, 2018
@kayceesrk kayceesrk added the bits-to-upstream Items to be upstreamed before multicore PR. Equivalent to "multicore-prerequisite" label in trunk. label May 10, 2018
@kayceesrk
Copy link
Contributor

PR ocaml/ocaml#9353 implements marshaling using hash tables instead of modifying GC colour bits in the header.

@kayceesrk
Copy link
Contributor

PR#9553 has been merged. The only item left to do is make marshaling safe when the objects are being modified concurrently.

@kayceesrk
Copy link
Contributor

Everything except

Marshalling of concurrently-modified objects is unsafe

is fixed in #612. I'll carve this into a new issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bits-to-upstream Items to be upstreamed before multicore PR. Equivalent to "multicore-prerequisite" label in trunk. broken
Projects
None yet
Development

No branches or pull requests

3 participants