feat: lazier runtime

[anmonteiro]

• ocaml-protoc-plugin generates a quite a few recursive module definitions
• generated runtime {de,}serializers that use a Spec.message pass first-class modules to it
• in Melange, accessing a recursive module eagerly like that results in Undefined_recursive_module exceptions
• Making the runtime a bit lazier sidesteps these issues, preserving semantics

I recently collaborated with @rauanmayemir to get his app ported to Melange, including running ocaml-protoc-plugin generated code, and this was the only required change.

[rauanmayemir]

I managed to upgrade my codebase to melange and latest main of the plugin, it’s a fairly large app using generated protobuf bindings for calling grpc-web from browser, everything works correctly now.

Runtime part also looked like a trivial change.

I’d love this change to get accepted so as to allow usage in melange.

[andersfugmann]

Thanks for the PR and for looking into this.

What exactly break melange? Is it the definition of the spec that causes problem (i.e. is the spec required to be lazy aswell?)

Making the functions less "eager" is something that I've tried before, but it really impacted speed, so I would be happy if this could be made local to the code gen. i.e. controlled by a flag to protoc

Something like

let serialize = lazy_opt ~lazy ~f:Internal'modules.Runtime.Serialize.serialize spec

(where the value of lazy is taken from code gen options)

The runtime can then define a function like:

let lazy_opt: lazy:bool -> f:('arg -> 'v -> 'r) -> 'arg -> 'v -> 'r = fun ~lazy ~f arg -> 
  match lazy with 
    | false -> f arg
    | true -> 
         let f' = Lazy.from_fun (fun () -> f arg) in
         fun v -> (Lazy.force f') v

(Edit: updated to allow the optimizer to do constant propagation)
The idea would be that if lazy is false the current behavior of eager evaluation is applied.

Also, consider reporting this to melange.

[rauanmayemir]

And codegen would be controlled by something like --opt=lazy=true? That would work, too.

[andersfugmann]

Exactly. In the same way as specifying other options controlling the generated code as documented here

[andersfugmann]

And I'd like to avoid marking the signature of serialize and deserialize take a lazy spec. If needed, the spec binding in the generated code could be a function, I.e.
let spec () = ... instead of let spec = ... .

[anmonteiro]

What exactly break melange? Is it the definition of the spec that causes problem (i.e. is the spec required to be lazy aswell?)

the smallest repro is something like the following, where you'll see serialize accesses Y eagerly.

module type Message = sig
  val f : unit -> unit
end

module rec X : sig
  val make : unit -> unit
  val serialize : unit -> unit
end = struct
  let make x = x

  let serialize =
    let f =
      let (module Msg) = (module Y : Message) in
      Msg.f
    in
    fun () -> f ()
end

and Y : sig
  val f : unit -> unit
end = struct
  let f = fun () -> ()
end

Also, consider reporting this to melange.

The underlying issue is that Melange compiles recursive modules to JS objects. It all works out if the modules don't have let bindings that refer to other modules. That's the error we're seeing

We're looking into this separately on the Melange side for sure, but I'd say it's a longer term, if ever, change. We added a test to track this behavior in melange-re/melange#1112.

[andersfugmann]

Thanks for the detailed description of the problem.

Instead of using a flag to control the code-gen, is there a way to determine if the code is running under melange? Will Sys.backend_type return Other _

If so, it would be possible to at runtime to apply lazy evaluation for backends other than Native or Bytecode.

[anmonteiro]

Indeed Sys.backend_type = Other "Melange". It's a good idea to use it to figure out whether the code should lazily evaluate. Thanks for helping dissect the problem!

[andersfugmann]

I can attempt to make a simple solution by wrapping serializers and deserializers in a lazy eval that switches on Sys.backend.

Is there an easy way to test if this is working? I have no experience with running Melange. What is a good staring point for generating a test case for doing test driven development on this?

[andersfugmann]

I'm trying to add a simple melange test here, following the example from the dune documentation. When adding a dependency to ocaml-protoc-plugin, melange complains that:

$ dune build @melange
Error: The library bytes was not compiled with Dune or it was compiled with
Dune but published with a META template. Such libraries are not compatible
with melange support
-> required by alias melange_test/melange
Error: The library ptime was not compiled with Dune or it was compiled with
Dune but published with a META template. Such libraries are not compatible
with melange support
-> required by alias melange_test/melange

Any idea on how to resolve this? As I said, this is my first attempt at using melange.

[anmonteiro]

Melange libraries need (modes melange) (or otherwise (modes :standard melange) to support multiple modes).

However, adding (modes melange) currently adds a non-optional dependency on melange, which will be paid by users.

I'm not sure that ocaml-protoc-plugin needs (modes melange) support. I'm also uncertain what's @rauanmayemir's workflow, but I suspect we may just need Melange compatibility in the generated modules.

[andersfugmann]

I create a branch, melange_compat, that inhibits eager evaluation for any backed other than Native or Bytecode. If you could test that is works for melange, that would be great.

[rauanmayemir]

@andersfugmann I'll try this asap, thank you.

[rauanmayemir]

@andersfugmann I tried your change and it works! Re-generated all bindings from that branch, tweaked the runtime, everything worked as expected.

[andersfugmann]

Thats great! I need to do some more testing (and solve a severe bug that I found in the process) before merging.

Btw. Did you need to make changes to the runtime of ocaml-protoc-plugin?

[rauanmayemir]

@andersfugmann Yes, at the moment I do have to do that. I vendored in Ptime module and for now ditched the Base64 dep (I don't need json serialization atm) because we need to add a runtime polyfill for nodejs/browser targets (server node and browser javascript work differently with base64).

Melange requires dune libraries to have (modes melange), so until most libraries will have melange support, we'd have to patch/vendor them.

I wanted to play more with OPP runtime before suggesting changes to the upstream.

[andersfugmann]

Thanks for the clarification. I've released 6.1.0, which includes the lazy binding. The code still depends on both ptime and base64. Both these libraries are only needed for json support.

I will close this PR, as I think its super-seeded by #31.
It should be possible to inhibit inclusion of these modules and provide dummy implementations and have dune switch on melange. I've opened #34 to track this.