More features in bench/tree.exe #1269

Ngoguey42 · 2021-01-28T16:25:53Z

Diff 1: --quick is now --mode quick, and --mode gives more choices.
Diff 2: The configurations for the quick and slow modes are now hard coded and can't be changed from the CLI. The other modes can be configured from the CLI.
Diff 3: Add --flatten for trace mode to collapse the 6 step-long hashes to single steps.
Diff 4: Add a progress bar using https://github.com/CraigFe/progress
Diff 5: Trace mode now tracks the time spent by the 8 unitary operation, using mtime and https://github.com/barko/bentov (more info in tree.ml).

I also have a python script to plot the histograms

icristescu

Thanks, looks great! Can you share your python script too?

bench/irmin-pack/tree.ml

icristescu · 2021-02-04T17:12:40Z

bench/irmin-pack/tree.ml

+    |> Hashtbl.of_seq
+
+  (** Find and print the largest directory. This may be useful in the future to
+      make sure that we are benching on million-sized directories. *)


I think we should not add code that we don't use right now. If you think it's useful you can add a cli argument to print this?

I'd have to refactor a big chunk of code to avoid printing that in the middle of the progress bar. My next PR on that file will most likely take care of it, so I'd like to keep it if that is OK for you.

bench/irmin-pack/tree.ml

Ngoguey42 · 2021-02-05T10:07:05Z

Where would be a good place to put my python script? Slack? Gist? Irmin's repo?

Ngoguey42 · 2021-02-05T12:26:03Z

The python script: https://gist.github.com/Ngoguey42/872ded9908593e1dc94ab0f28e8abbe8

Diff 1: `--quick` is now `--mode quick`, and `--mode` gives more choices. Diff 2: The configurations for the `quick` and `slow` modes are now hard coded and can't be changed from the CLI. The other modes can be configured from the CLI. Diff 3: Add `--flatten` for trace mode to collapse the 6 step-long hashes to single steps. Diff 4: Add a progress bar using https://github.com/CraigFe/progress Diff 5: Trace mode now tracks the time spent by the 8 unitary operation, using mtime and https://github.com/barko/bentov (more info in tree.ml).

Co-authored-by: Ioana Cristescu <icristescu@users.noreply.github.com>

craigfe · 2021-02-05T12:30:19Z

I keep this sort of thing in an irmin-notebooks repository. Now might be a good time to put that somewhere we can all have access too; maybe tarides/irmin-notebooks?

Ngoguey42 · 2021-02-05T12:42:45Z

Yes, we should centralise those scripts.

Although I don't feel like turning this one into a notebook, I find it more convenient to manipulate image files in order to compare 2 revisions or 2 variations of a parameter.

craigfe · 2021-02-05T12:47:42Z

Notebooks can run savefig too :-) I don't think it needs to be a case of one or the other.

Ngoguey42 · 2021-02-05T13:33:58Z

Okey I can do that. I also want to be able to run from command line, which is possible for a notebook after installing some more python tools.

Ngoguey42 · 2021-02-05T14:08:39Z

My latest commit writes to disk the json histogram.

If you have no more comments this PR may be merged today.

craigfe

LGTM. Just style nits.

CHANGES.md

craigfe · 2021-02-05T14:41:59Z

bench/irmin-pack/tree.ml

      |> Benchmark.run config
    in
    let+ () = Store.Repo.close repo in
-    (config, result)
+    fun fmt ->


I see this is already in this file in a few places, but conventionally fmt is a format-string, and ppf is a Format.formatter.

bench/irmin-pack/tree.ml

Co-authored-by: Craig Ferguson <me@craigfe.io>

@Ngoguey42

… irmin-chunk, irmin-pack, irmin-test, irmin-http, irmin-unix, ppx_irmin, irmin-bench, irmin-graphql, irmin-containers, irmin-mirage-git and irmin-mirage-graphql (2.5.0) CHANGES: ### Changed - **irmin** - `Store.Tree.remove` is now much faster when operating on large directories. The commits following removals are also much faster. (mirage/irmin#1289, @Ngoguey42) - Changed `Store.Tree.{of_hash, shallow}` to take kinded hashes, allowing the creation of unforced contents values. (mirage/irmin#1285, @craigfe) - Changed `Tree.destruct` to return _lazy_ contents values, which may be forced with `Tree.Contents.force`. (mirage/irmin#1285, @craigfe) - **irmin-bench** - New features in benchmarks for tree operations (mirage/irmin#1269, @Ngoguey42)

Ngoguey42 added the no-changelog-needed No changelog is needed here label Jan 28, 2021

Ngoguey42 force-pushed the profile_inodes branch from d661977 to dfb190b Compare January 31, 2021 14:49

Ngoguey42 mentioned this pull request Feb 1, 2021

Inode.v is inefficient #1272

Closed

Ngoguey42 marked this pull request as draft February 2, 2021 16:47

Ngoguey42 force-pushed the profile_inodes branch from dfb190b to 9a53e9c Compare February 3, 2021 10:58

Ngoguey42 changed the title ~~More options on tree.exe~~ WIP: More options on tree.exe Feb 3, 2021

Ngoguey42 force-pushed the profile_inodes branch 2 times, most recently from e0018e3 to bbba35e Compare February 4, 2021 12:02

Ngoguey42 changed the title ~~WIP: More options on tree.exe~~ More features in bench/tree.exe Feb 4, 2021

Ngoguey42 force-pushed the profile_inodes branch from bbba35e to 43f4970 Compare February 4, 2021 12:10

Ngoguey42 requested a review from icristescu February 4, 2021 12:12

Ngoguey42 marked this pull request as ready for review February 4, 2021 12:12

Ngoguey42 removed the no-changelog-needed No changelog is needed here label Feb 4, 2021

icristescu approved these changes Feb 4, 2021

View reviewed changes

Ngoguey42 and others added 6 commits February 5, 2021 13:27

Fix 1 docstring

f57c63f

Apply suggestions from code review

c849d21

Co-authored-by: Ioana Cristescu <icristescu@users.noreply.github.com>

Remove ununsed parameters in exec_* functions

b5a2bdd

Improve and format docstrings

08a69a8

Remove ununsed path to tarball parameter

80052a4

Update changes

67d8b83

Ngoguey42 force-pushed the profile_inodes branch from 1d6f2f7 to 67d8b83 Compare February 5, 2021 12:38

Write json histogram file

adf9318

craigfe approved these changes Feb 5, 2021

View reviewed changes

ngoguey and others added 3 commits February 5, 2021 16:51

Apply suggestions from code review

026f552

Co-authored-by: Craig Ferguson <me@craigfe.io>

Code cleaning/formatting

de958d3

Fix irmin-bench.opam

1516b5a

Ngoguey42 force-pushed the profile_inodes branch from 17e89fc to 1516b5a Compare February 8, 2021 09:59

Merge remote-tracking branch 'origin/master' into profile_inodes

1c1b7b7

Ngoguey42 merged commit c526079 into mirage:master Feb 8, 2021

craigfe mentioned this pull request Feb 16, 2021

[new release] irmin et al. (2.5.0) ocaml/opam-repository#18168

Merged

Ngoguey42 mentioned this pull request Feb 23, 2021

bench/irmin-pack/tree.exe: respect arguments from CLI #1306

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

More features in bench/tree.exe #1269

More features in bench/tree.exe #1269

Ngoguey42 commented Jan 28, 2021 •

edited

icristescu left a comment

icristescu Feb 4, 2021

Ngoguey42 Feb 5, 2021

Ngoguey42 commented Feb 5, 2021

Ngoguey42 commented Feb 5, 2021

craigfe commented Feb 5, 2021

Ngoguey42 commented Feb 5, 2021

craigfe commented Feb 5, 2021 •

edited

Ngoguey42 commented Feb 5, 2021

Ngoguey42 commented Feb 5, 2021

craigfe left a comment

craigfe Feb 5, 2021

More features in bench/tree.exe #1269

More features in bench/tree.exe #1269

Conversation

Ngoguey42 commented Jan 28, 2021 • edited

icristescu left a comment

Choose a reason for hiding this comment

icristescu Feb 4, 2021

Choose a reason for hiding this comment

Ngoguey42 Feb 5, 2021

Choose a reason for hiding this comment

Ngoguey42 commented Feb 5, 2021

Ngoguey42 commented Feb 5, 2021

craigfe commented Feb 5, 2021

Ngoguey42 commented Feb 5, 2021

craigfe commented Feb 5, 2021 • edited

Ngoguey42 commented Feb 5, 2021

Ngoguey42 commented Feb 5, 2021

craigfe left a comment

Choose a reason for hiding this comment

craigfe Feb 5, 2021

Choose a reason for hiding this comment

Ngoguey42 commented Jan 28, 2021 •

edited

craigfe commented Feb 5, 2021 •

edited