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

Add json output and make target for benchmarking layered store. #1146

Merged
merged 4 commits into from
Nov 20, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
- **irmin-bench** (_new_):
- Created a new package to contain benchmarks for Irmin and its various
backends. (#1142, @CraigFe)
- Added ability to get json output and a make target to run layers benchmark.
(#1146, @gs0510)

- **irmin**
- Added `Tree.{Contents,Node}` modules exposing operations over lazy tree
Expand Down
7 changes: 5 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
.PHONY: all clean test fuzz bench-mem bench-pack bench doc examples
.PHONY: all clean test fuzz bench-mem bench-pack bench-layers bench doc examples

all:
dune build
Expand All @@ -12,7 +12,10 @@ bench-mem:
bench-pack:
dune exec ./test/irmin-pack/bench.exe

bench: bench-mem bench-pack
bench-layers:
dune exec -- ./bench/irmin-pack/layers.exe -n 2005 -b 2 -j

bench: bench-mem bench-pack bench-layers

fuzz:
dune build @fuzz --no-buffer
Expand Down
5 changes: 4 additions & 1 deletion bench/irmin-pack/dune
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
(executables
(names main layers)
(libraries irmin-pack irmin-test.bench irmin-layers lwt unix cmdliner logs))
(preprocess
(pps ppx_deriving_yojson))
(libraries irmin-pack irmin-test.bench irmin-layers lwt unix cmdliner logs
yojson ppx_deriving_yojson))

;; Require the above executables to compile during tests

Expand Down
52 changes: 43 additions & 9 deletions bench/irmin-pack/layers.ml
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ let first_5_cycles config repo =
print_commit_stats config c 0 0.0;
let rec aux i c =
add_min c;
if i >= 4 then Lwt.return c
if i > 4 then Lwt.return c
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only 4 cycles were running before, fixed to run 5 cycles as the function name suggests 😸

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One day, we'll live in an Lwt utopia with helper functions for these things...

else write_cycle config repo c >>= fun c -> aux (i + 1) c
in
aux 0 c
Expand Down Expand Up @@ -267,7 +267,28 @@ let run config =
Fmt.epr "After freeze thread finished : ";
FSHelper.print_size config.root)

let main () ncommits ncycles depth clear no_freeze show_stats =
type result = {
total_time : float;
time_per_commit : float;
commits_per_sec : float;
}
[@@deriving yojson]

let get_json_str total_time time_per_commit commits_per_sec =
let res = { total_time; time_per_commit; commits_per_sec } in
let obj =
`Assoc
[
( "results",
`Assoc
[
("name", `String "benchmarks"); ("metrics", result_to_yojson res);
] );
]
in
Yojson.Safe.to_string obj

let main () ncommits ncycles depth clear no_freeze show_stats json =
let config =
{
ncommits;
Expand All @@ -281,13 +302,15 @@ let main () ncommits ncycles depth clear no_freeze show_stats =
in
init config;
let d, _ = Lwt_main.run (with_timer (fun () -> run config)) in
let all_commits = ncommits * ncycles in
let all_commits = ncommits * (ncycles + 5) in
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems odd to always run 5 more cycles than requested. Perhaps we could run max(5, ncycles) and update the CLI doc to say that at least 5 runs are necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first 5 cycles are run in addition to the number of cycles passed as arg.

https://github.com/mirage/irmin/blob/master/bench/irmin-pack/layers.ml#L263 runs the first 5 cycles followed by run_cycles. Maybe we should remove the first_5_cycles and change the doc to say running at least 5 is necessary?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, I think it's better to have a more explicit behaviour.

Out of interest, why is it necessary to have at least 5 cycles? It seems that I might want to just run ~1 cycle when working on the benchmarks, for instance.

Copy link
Contributor Author

@gs0510 gs0510 Nov 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know! Paging @icristescu 😁

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The benchmarks are meant to be somewhat similar to the tezos usecase where every freeze copies the last 5 cycles. So we need to have the first 5 cycles before calling the first freeze. Then each cycle we call freeze again (done by run_cycles). You are free to change the benchmarks, just I think the default behaviour should be this one (and more documentation is always welcomed:) )

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merge when ready then, @gs0510 🙂

let rate = d /. float all_commits in
let freq = 1. /. rate in
Logs.app (fun l ->
l
"%d commits completed in %.2fs.\n\
[%.3fs per commit, %.0f commits per second]" all_commits d rate freq)
if json then Logs.app (fun l -> l "%s" (get_json_str d rate freq))
else
Logs.app (fun l ->
l
"%d commits completed in %.2fs.\n\
[%.3fs per commit, %.0f commits per second]" all_commits d rate freq)

open Cmdliner

Expand All @@ -296,7 +319,13 @@ let ncommits =
Arg.(value @@ opt int 4096 doc)

let ncycles =
let doc = Arg.info ~doc:"Number of cycles." [ "b"; "ncycles" ] in
let doc =
Arg.info
~doc:
"Number of cycles. This will be in addition to the 5 cycles that run \
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added this documentation here, hopefully it makes it clearer that it will be 5 + ncycles running. cc @craigfe @icristescu

to emulate freeze."
[ "b"; "ncycles" ]
in
Arg.(value @@ opt int 10 doc)

let depth =
Expand All @@ -317,6 +346,10 @@ let stats =
let doc = Arg.info ~doc:"Show performance stats." [ "s"; "stats" ] in
Arg.(value @@ flag doc)

let json =
let doc = Arg.info ~doc:"Json output on command line." [ "j"; "json" ] in
Arg.(value @@ flag doc)

let setup_log style_renderer level =
Fmt_tty.setup_std_outputs ?style_renderer ();
Logs.set_level level;
Expand All @@ -335,7 +368,8 @@ let main_term =
$ depth
$ clear
$ no_freeze
$ stats)
$ stats
$ json)

let () =
let info = Term.info "Benchmarks for layered store" in
Expand Down
2 changes: 2 additions & 0 deletions irmin-bench.opam
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ depends: [
"cmdliner"
"logs"
"lwt"
"ppx_deriving_yojson"
"yojson"
]

synopsis: "Irmin benchmarking suite"
Expand Down