Skip to content

Commit

Permalink
WIP: Port to Eio
Browse files Browse the repository at this point in the history
Most of the complexity in the previous version was about managing the
child worker processes. With OCaml 5 this all goes away, and we just
use multiple domains instead.

Notes:

- opam is not thread-safe, so we need a lock around all opam file
  parsing operations. See ocaml/opam#5591

- We now load opam files lazily, which makes testing much faster.
  This reduces the time for `stress/stress.exe` from 97.4s to 11.5s for
  me.

- Epoch_lock is gone. It was just for managing sub-processes, but a
  simple Eio.Mutex.t will do now. This should also be much faster when
  switching between opam-repository commits.

- The main executable is now in the `bin` directory. Simplifies the dune
  file.

- test_service.ml is gone. It was only testing parsing of the
  communications between parent and child processes, which no longer
  exist. This also removed the need to functorise over opam repository.

- Default to `recommended_domain_count - 1` workers.

- Removed uses of (non-threadsafe) Str module.

TODO:

- Bring back parent/child pipe mode.

- Integrate worker mode?

- Add full stress test (i.e. solving for many packages over many hours).
  • Loading branch information
talex5 committed Jul 5, 2023
1 parent 96a35e1 commit e783e4c
Show file tree
Hide file tree
Showing 38 changed files with 620 additions and 1,166 deletions.
5 changes: 4 additions & 1 deletion .gitmodules
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,7 @@
[submodule "ocaml-dockerfile"]
path = ocaml-dockerfile
url = https://github.com/ocurrent/ocaml-dockerfile.git
branch = master
branch = master
[submodule "opam-0install-solver"]
path = opam-0install-solver
url = https://github.com/ocaml-opam/opam-0install-solver.git
7 changes: 7 additions & 0 deletions bin/dune
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
(executable
(name main)
(package solver-service)
(public_name solver-service)
(preprocess
(pps ppx_deriving.std ppx_deriving_yojson))
(libraries solver-service logs.cli capnp-rpc-unix eio_main dune-build-info logs.fmt logs.threaded fmt.cli fmt.tty))
57 changes: 24 additions & 33 deletions service/main.ml → bin/main.ml
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
open Solver_service
open Lwt.Syntax
module Service = Service.Make (Opam_repository)
module Worker_process = Internal_worker.Worker_process
open Eio.Std
module Service = Solver_service

let pp_timestamp f x =
let open Unix in
Expand Down Expand Up @@ -37,6 +35,7 @@ let setup_log style_renderer level =
()

let export service ~on:socket =
let open Lwt.Syntax in
let restore =
Capnp_rpc_net.Restorer.single
(Capnp_rpc_net.Restorer.Id.public "solver")
Expand All @@ -60,38 +59,36 @@ let export service ~on:socket =
in
crashed

let start_server address ~n_workers =
let start_server ~sw ~process_mgr ~domain_mgr address ~n_workers =
let open Lwt.Syntax in
Lwt_eio.run_lwt @@ fun () ->
let config =
Capnp_rpc_unix.Vat_config.create ~secret_key:(`File "server.pem") address
in
let service_id =
Capnp_rpc_unix.Vat_config.derived_id config "solver-service"
in
let create_worker commits =
let cmd =
("", [| Sys.argv.(0); "--worker"; Remote_commit.list_to_string commits |])
in
Worker_process.create cmd
in
let* service = Service.v ~n_workers ~create_worker in
let service = Service.create ~sw ~domain_mgr ~process_mgr ~n_workers in
let service = Service.capnp_service service in
let restore = Capnp_rpc_net.Restorer.single service_id service in
let+ vat = Capnp_rpc_unix.serve config ~restore in
Capnp_rpc_unix.Vat.sturdy_uri vat service_id

let main () hash address sockpath n_workers =
match hash with
| Some commits_str -> Solver.main (Remote_commit.list_of_string_or_fail commits_str)
let main () address sockpath n_workers =
Eio_main.run @@ fun env ->
Switch.run @@ fun sw ->
let process_mgr = env#process_mgr in
Lwt_eio.with_event_loop ~clock:env#clock @@ fun () ->
match address with
| Some address ->
(* Run with a capnp address as the endpoint *)
let uri = start_server ~sw ~process_mgr ~domain_mgr:env#domain_mgr address ~n_workers in
Fmt.pr "Solver service running at: %a@." Uri.pp_hum uri;
Fiber.await_cancel ()
| None ->
Eio_main.run @@ fun env ->
Lwt_eio.with_event_loop ~clock:env#clock @@ fun () ->
Lwt_eio.run_lwt @@ fun () ->
match address with
| Some address ->
(* Run with a capnp address as the endpoint *)
let* uri = start_server address ~n_workers in
Fmt.pr "Solver service running at: %a@." Uri.pp_hum uri;
fst @@ Lwt.wait ()
| None ->
ignore (sockpath, export);
assert false
(*
let socket =
match sockpath with
| Some path ->
Expand All @@ -112,6 +109,7 @@ let main () hash address sockpath n_workers =
in
let* service = Service.v ~n_workers ~create_worker in
export service ~on:socket
*)

(* Command-line parsing *)

Expand All @@ -122,15 +120,9 @@ let setup_log =
Term.(
const setup_log $ Fmt_cli.style_renderer ~docs () $ Logs_cli.level ~docs ())

let worker_commits =
Arg.value
@@ Arg.opt Arg.(some string) None
@@ Arg.info ~doc:"The hash commits of the worker." ~docv:"COMMITS"
[ "worker" ]

let internal_workers =
Arg.value
@@ Arg.opt Arg.int 20
@@ Arg.opt Arg.int (Domain.recommended_domain_count () - 1)
@@ Arg.info ~doc:"The number of sub-process solving requests in parallel"
~docv:"N" [ "internal-workers" ]

Expand Down Expand Up @@ -164,7 +156,6 @@ let cmd =
Term.(
const main
$ setup_log
$ worker_commits
$ address
$ sockpath
$ internal_workers)
Expand Down
File renamed without changes.
File renamed without changes.
2 changes: 1 addition & 1 deletion dune
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
(dirs :standard \ var)

(vendored_dirs ocluster ocurrent ocaml-dockerfile)
(vendored_dirs ocluster ocurrent ocaml-dockerfile opam-0install-solver)
1 change: 1 addition & 0 deletions examples/dune
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
(names main submit)
(package solver-service)
(libraries
eio_main
current_git
capnp-rpc-unix
solver-service-api
Expand Down
33 changes: 19 additions & 14 deletions examples/main.ml
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,11 @@ let opam_template arch =
|}
arch

let get_vars ~ocaml_package_name ~ocaml_version ?arch () =
let+ vars =
Solver_service.Process.pread
("", [| "opam"; "config"; "expand"; opam_template arch |])
let get_vars ~process_mgr ~ocaml_package_name ~ocaml_version ?arch () =
Lwt_eio.run_eio @@ fun () ->
let vars =
Eio.Process.parse_out process_mgr Eio.Buf_read.take_all
["opam"; "config"; "expand"; opam_template arch]
in
let json =
match Yojson.Safe.from_string vars with
Expand All @@ -55,15 +56,16 @@ let get_vars ~ocaml_package_name ~ocaml_version ?arch () =
in
Result.get_ok @@ Solver_service_api.Worker.Vars.of_yojson json

let get_opam_file pv =
Solver_service.Process.pread ("", [| "opam"; "show"; "--raw"; pv |])
let get_opam_file ~process_mgr pv =
Lwt_eio.run_eio @@ fun () ->
Eio.Process.parse_out process_mgr Eio.Buf_read.take_all ["opam"; "show"; "--raw"; pv]

let run_client ~package ~version ~ocaml_version ~opam_commit service =
let run_client ~process_mgr ~package ~version ~ocaml_version ~opam_commit service =
let pv = package ^ "." ^ version in
let* platform =
get_vars ~ocaml_package_name:"ocaml-base-compiler" ~ocaml_version ()
get_vars ~process_mgr ~ocaml_package_name:"ocaml-base-compiler" ~ocaml_version ()
in
let* opam_file = get_opam_file pv in
let* opam_file = get_opam_file ~process_mgr pv in
let request =
Solver_service_api.Worker.Solve_request.
{
Expand All @@ -88,11 +90,14 @@ let run_client ~package ~version ~ocaml_version ~opam_commit service =
| Error `Cancelled -> Fmt.failwith "Job Cancelled"

let connect package version ocaml_version opam_commit uri =
Lwt_main.run
(let client_vat = Capnp_rpc_unix.client_only_vat () in
let sr = Capnp_rpc_unix.Vat.import_exn client_vat uri in
Capnp_rpc_unix.with_cap_exn sr
(run_client ~package ~version ~ocaml_version ~opam_commit))
Eio_main.run @@ fun env ->
let process_mgr = env#process_mgr in
Lwt_eio.with_event_loop ~clock:env#clock @@ fun () ->
Lwt_eio.run_lwt @@ fun () ->
let client_vat = Capnp_rpc_unix.client_only_vat () in
let sr = Capnp_rpc_unix.Vat.import_exn client_vat uri in
Capnp_rpc_unix.with_cap_exn sr
(run_client ~process_mgr ~package ~version ~ocaml_version ~opam_commit)

open Cmdliner

Expand Down
30 changes: 4 additions & 26 deletions service/dune
Original file line number Diff line number Diff line change
@@ -1,36 +1,14 @@
(library
(name solver_service)
(public_name solver-service)
(preprocess
(pps ppx_deriving.std ppx_deriving_yojson))
(libraries
eio_main
eio
lwt_eio
solver-service-api
ppx_deriving_yojson.runtime
prometheus-app
opam-0install
capnp-rpc-unix
capnp-rpc-net
git-unix
ocaml-version
dune-build-info
str
fmt.cli
fmt.tty)
(modules
epoch_lock
git_context
opam_repository
opam_repository_intf
process
remote_commit
internal_worker
service
solver
solver_service))

(executable
(name main)
(package solver-service)
(public_name solver-service)
(libraries solver-service logs.cli)
(modules main))
ocluster-api))
60 changes: 0 additions & 60 deletions service/epoch_lock.ml

This file was deleted.

22 changes: 0 additions & 22 deletions service/epoch_lock.mli

This file was deleted.

Loading

0 comments on commit e783e4c

Please sign in to comment.