Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wish: configure ocaml so that old caml_modify is used #6084

Closed
vicuna opened this issue Jul 22, 2013 · 18 comments
Closed

Wish: configure ocaml so that old caml_modify is used #6084

vicuna opened this issue Jul 22, 2013 · 18 comments

Comments

@vicuna
Copy link

@vicuna vicuna commented Jul 22, 2013

Original bug ID: 6084
Reporter: gerd
Status: closed (set by @xavierleroy on 2015-12-11T18:23:57Z)
Resolution: fixed
Priority: normal
Severity: feature
Fixed in version: 4.01.0+dev
Category: runtime system and C interface
Monitored by: @gasche @hcarty @yakobowski @alainfrisch

Bug description

This actually refers to the upcoming 4.01 version: Apparently, caml_modify was recently changed in the svn tree so that no out-of-heap values are supported anymore. I guess this gives better performance, as the page table lookup can be avoided. So far I understand the new code, the avoided Is_in_heap check makes it impossible to have out-of-heap values (i.e. neither in the minor nor the major heap).

I have a siginificant code base using out-of-heap values for managing shared memory, and it is non-trivial to change it so that it complies again to the rules (which will require some form of alternate code generator). This is not impossible, but I need some time to do it, and it would be nice to have a configure-time option for building ocaml with the old caml_modify behavior (maybe "-out-of-heap-values"), in order to smoothen the transition.

Steps to reproduce

The patch changing caml_modify: http://caml.inria.fr/cgi-bin/viewvc.cgi?view=revision&revision=13723

Additional information

This affects Netmulticore (a multi-processing manager supporting shared memory), and software using it, e.g. Plasma Map/Reduce.

I'd happily provide a patch if the suggestion is accepted.

@vicuna
Copy link
Author

@vicuna vicuna commented Jul 22, 2013

Comment author: @alainfrisch

Just to be clear: the new implementation requires the modified block to be in the heap, not the target pointer. If you indeed use caml_modify to modify blocks outside the heap, couldn't you just update the point directly instead?

@vicuna
Copy link
Author

@vicuna vicuna commented Jul 22, 2013

Comment author: gerd

If I wrote C code, yes, I could do what Alain suggests. But the problem is that Netmulticore is almost completely written in OCaml, and this is part of its elegance. This means that I'm talking about OCaml-level assignments (x.field <- y). Just to show an example:

https://godirepo.camlcity.org/wwwsvn/trunk/code/src/netmulticore/netmcore_queue.ml?rev=1679&root=lib-ocamlnet2&view=auto

This is a queue implemented in shared memory (what's called "(shared) heap" in the code is actually referring to memory outside the normal OCaml heap). The updates of the values are simply done in-place, with some helpers like [add] (copy a new value to the shared heap) or [modify] (get a lock).

Nevertheless, thanks for the hint that only the left side needs to be in the heap, and the right can be anywhere. This allows it to call a C subroutine doing the field update (e.g. I could redefine := inside the modify block so that a C external is called instead), and fortunately it is harmless when out-of-ocaml-heap values are stored into ocaml-heap variables. So the new caml_modify is not as catastrophic as I feared.

(Well, this approach will still ruin the style somewhat, so the bigger solution of having a separate code generator for such blocks accessing shared memory would still be advantageous. Maybe with a ppx driver to get better syntax.)

@vicuna
Copy link
Author

@vicuna vicuna commented Jul 22, 2013

Comment author: @alainfrisch

Yikes!

It is a known fact that many libraries around depend on the fact that out-of-heap pointers can be seen as OCaml values, and there will be ample communication before this is outlawed. But we (at least I, and probably Xavier who did this change) were not aware of the use of regular OCaml code used to modify out-of-heap values...

One could indeed imagine some attributes (as in the extension_points branch) to inform the code generator that some block needs to be compiled with calls to the old 'modify', but this will not happen soon.

I'll let Xavier and others comment about the configure switch, but I'm not a big fan of this solution (your library would require this flag, which would give incentive e.g. to GODI to configure ocaml by default with this flag). How much code do you have which would require to be rewritten in a more low-level style? (Redefining := is not enough, you also have <- assignments.)

@vicuna
Copy link
Author

@vicuna vicuna commented Jul 23, 2013

Comment author: @gasche

I think we could have something more fine-grained than a configure-time setting. Given that this only impact the implementation of out-of-heap-accessing modules, not their interfaces, you could have a compiler command-line option activated on those specific modules to generate a test for out-of-heap pointers before primitive mutations with caml_modify.

PS: of course pragmas using extension_points could allow to enable this setting more locally inside a module.

@vicuna
Copy link
Author

@vicuna vicuna commented Jul 23, 2013

Comment author: @xavierleroy

Thanks to all who participated in the discussion; it makes the issue clearer. Indeed, we are still supporting out-of-heap pointers as valid OCaml values anywhere. (Getting rid of them would speed up the run-time system, but it would break quite a lot of code.) What the new caml_modify requires is that the "value *" being modified resides in one of the OCaml heaps. Note that the old code was already unsafe if called with a "value *" that is out of heap and a right-hand side that is within the heap...

@vicuna
Copy link
Author

@vicuna vicuna commented Jul 23, 2013

Comment author: gerd

gasche: the compiler switch would be of course better. For my use case, there is still the danger that a code section modifying out-of-heap values calls a function that was not compiled with the switch, but that's something one can deal with. Btw, how would inlined functions be handled (thinking of := and Array.set especially)?

xleroy: fortunately I do not run into the case where lhs is out of heap, and rhs in the heap (it would probably break with the next GC cycle anyway). Or better said, while developing my code, this was one of the errors I ran into frequently. I wished it raised an exception in this case when debugging the code. Taking this thought one step further: if we add a new switch per compilation unit, maybe caml_modify should also check for this case, and fail? i.e. it's a caml_safe_modify we are turning on. I could imagine this switch could be useful for debugging C wrappers too, as bad pointers could be detected quicker.)

@vicuna
Copy link
Author

@vicuna vicuna commented Jul 23, 2013

Comment author: @gasche

Gerd, do not get too high hopes about the compiler switch idea. The changes to make would not very dangerous, but not simple either (in particular that would require duplicating a handful of runtime functions that ultimately call "caml_modify"), and I am not sure the people in charge will appreciate the idea this late in the release cycle -- Alain for example does not seem enamored with your use-case.

@vicuna
Copy link
Author

@vicuna vicuna commented Jul 23, 2013

Comment author: gerd

No problem, that was just an idea to maximize the usefulness if the switch path is taken.

Off topic - If I had several wishes free, I would even go further, because what I really need for out-of-heap shared memory is the ability to redirect memory allocations, so that values created in a special section are directly created in the target memory region. That would make programming with explicit shared memory almost as convenient as with the implicit version (i.e. normal multi-threading). Just mentioning this as I have currently your attention, no idea whether the ocaml team is still considering true parallelism as future option.

@vicuna
Copy link
Author

@vicuna vicuna commented Jul 24, 2013

Comment author: @alainfrisch

Gerd: are you comfortable with the fact that OCaml 4.01 will break the current version of your library? Will you able to adapt it with reasonable efforts (probably by using some custom C code)?

@vicuna
Copy link
Author

@vicuna vicuna commented Jul 24, 2013

Comment author: gerd

Alain: I think so. There are maybe 20-30 shared data structures, and all fortunately simple (ususally shared caches of some sort). I'll have to convert the code from <- to :=, but this should be possible. The most work is probably to change the core of Netmulticore. Maybe a day's work, if I'm fully focused to it.

@vicuna
Copy link
Author

@vicuna vicuna commented Jul 24, 2013

Comment author: @xavierleroy

On the OCaml side, the best we could do (as far as I can think of right now) would be to declare caml_modify() as a "weak" function, so that Gerd's libraries can override it with their own implementation of caml_modify(), similar to the old implementation, perhaps with extra checks. This would work on ELF/Unix and MacOS X platforms at least, but not under Windows. Gerd, let us know if you think this could help.

@vicuna
Copy link
Author

@vicuna vicuna commented Jul 24, 2013

Comment author: @xavierleroy

PS:

maybe caml_modify should also check for this case, and fail?

Good idea, but beware: it is not safe to raise an exception from caml_modify(). So, that would be a fatal error.

@vicuna
Copy link
Author

@vicuna vicuna commented Jul 24, 2013

Comment author: gerd

I haven't worked with weak functions yet, so I cannot say whether it will work or not. What I've read is that static linking will require --whole-archive, or there is the risk that the lookup doesn't find the overriding strong definition. But I haven't checked. The definition in ocamlrun will not be overridable anyway, but there is of course the alternative of creating a custom toploop.

So if that works, it's an interesting abbreviation, and I could have a specialized version of caml_modify. In particular, it is probably faster to check for my special address range than using Is_in_heap.

Guess this is just a #pragma?

@vicuna
Copy link
Author

@vicuna vicuna commented Jul 24, 2013

Comment author: @xavierleroy

Guess this is just a #pragma?

In GCC, it's either a #pragma or an attribute. I guess Clang accepts both as well.

@vicuna
Copy link
Author

@vicuna vicuna commented Jul 29, 2013

Comment author: @xavierleroy

In GCC, it's either a #pragma or an attribute. I guess Clang accepts both as well.

Apparently, clang doesn't implement this #pragma, so attribute it should be.

What are the platforms of interest to you? ELF is fine, MacOS X might work but needs testing, and Win32 doesn't have anything resembling weak symbols. Finding the right #ifdef is a bit of a challenge...

@vicuna
Copy link
Author

@vicuna vicuna commented Jul 29, 2013

Comment author: gerd

Primary target is Linux, and OSX secondary. Win32 is out of interest anyway, as my multi-processing approach is hard to port to an OS without fork().

Thanks for considering this.

@vicuna
Copy link
Author

@vicuna vicuna commented Aug 1, 2013

Comment author: @xavierleroy

Declare caml_modify() and caml_initialize() as weak symbols on ELF platforms and under MacOS X. Commits r13957 in version/4.01 and r13958 in trunk/. Will be part of release 4.01.

@vicuna
Copy link
Author

@vicuna vicuna commented Aug 30, 2013

Comment author: gerd

Just wanting to report success: I can use my libraries with ocaml-4.01-beta1. The only little problem was that weak symbols can only be overridden by object files on the linker command-line, and not implicitly by archives. Once I figured this out, the whole thing worked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant