rdicosmo / parmap

Parmap is a minimalistic library allowing to exploit multicore architecture for OCaml programs with minimal modifications.
http://rdicosmo.github.io/parmap/
Other
94 stars 20 forks source link

Investigate naked pointers and wrap them before OCaml 5.0 #102

Closed rdicosmo closed 2 years ago

rdicosmo commented 3 years ago

The marshalling/unmarshalling code exposes naked pointers, that may be deprecated in OCaml 5.0. We need to look at how to properly wrap them, see https://discuss.ocaml.org/t/ann-a-dynamic-checker-for-detecting-naked-pointers/5805 and https://ocaml.org/manual/intfc.html#ss:c-outside-head

Thanks @kit-ty-kate for spotting this.

moyodiallo commented 2 years ago

Hi, we found the issue about the naked pointers and It's caused by casting Bigarray to Array that make the header tag no longer valid for the Gc in OCaml 5.00.

This is the a diff of my fork on the branch fix-naked-pointers: we just change Array.unsafe_set to Bigarray.Array1.unsafe_set at parmap.ml line 725

diff --git a/src/parmap.ml b/src/parmap.ml
index edc043a..3b997e8 100644
--- a/src/parmap.ml
+++ b/src/parmap.ml
@@ -718,11 +718,11 @@ let array_float_parmapi
       array: the data in Bigarray is placed at offset 1 w.r.t. a normal array,
       so we get a pointer to that zone into arr_out_as_array, and have it typed
       as a float array *)
-   let barr_out_as_array = Array.unsafe_get (Obj.magic barr_out) 1 in
+   (*let barr_out_as_array = Array.unsafe_get (Obj.magic barr_out) 1 in*)
    let compute _ lo hi _ exc_handler =
      try
        for i=lo to hi do
-         Array.unsafe_set barr_out_as_array i (f i (Array.unsafe_get al i))
+         Bigarray.Array1.unsafe_set barr_out i (f i (Array.unsafe_get al i))
        done
      with e -> exc_handler e lo
    in

The question at end: Is this fix can be good for your purpose ?

rdicosmo commented 2 years ago

Thanks @moyodiallo for proposing this fix! May you open up a pull request, so I can merge it?

moyodiallo commented 2 years ago

OK I'll do it

rdicosmo commented 2 years ago

Thanks @moyodiallo, this is now in release 1.2.4