Re: [PATCH v1 0/6] tools/ocaml/xenstored: simplify code

[Christian Lindig <christian.lindig@xxxxxxxxxx>]

________________________________________
From: Edwin Torok
Sent: 14 August 2020 23:11
To: xen-devel@xxxxxxxxxxxxxxxxxxxx
Cc: Edwin Torok; Christian Lindig; David Scott; Ian Jackson; Wei Liu
Subject: [PATCH v1 0/6] tools/ocaml/xenstored: simplify code

Fix warnings, and delete some obsolete code.
oxenstored contained a hand-rolled GC to perform hash-consing:
this can be done with a lot fewer lines of code by using the built-in Weak 
module.

The choice of data structures for trees/tries is not very efficient: they are 
just
lists. Using a map improves lookup and deletion complexity, and replaces 
hand-rolled
recursion with higher-level library calls.

There is a lot more that could be done to optimize socket polling:
an epoll backend with a poll fallback,but API structured around event-based 
polling
would be better. But first lets drop the legacy select based code: I think every
modern *nix should have a working poll(3) by now.

This is a draft series, in need of more testing.

Edwin Török (6):
  tools/ocaml/libs/xc: Fix ambiguous documentation comment
  tools/ocaml/xenstored: fix deprecation warning
  tools/ocaml/xenstored: replace hand rolled GC with weak GC references
  tools/ocaml/xenstored: drop select based
  tools/ocaml/xenstored: use more efficient node trees
  tools/ocaml/xenstored: use more efficient tries

 tools/ocaml/libs/xc/xenctrl.mli               |  2 +
 tools/ocaml/xenstored/connection.ml           |  3 -
 tools/ocaml/xenstored/connections.ml          |  2 +-
 tools/ocaml/xenstored/disk.ml                 |  2 +-
 tools/ocaml/xenstored/history.ml              | 14 ----
 tools/ocaml/xenstored/parse_arg.ml            |  7 +-
 tools/ocaml/xenstored/{select.ml => poll.ml}  | 14 +---
 .../ocaml/xenstored/{select.mli => poll.mli}  | 12 +---
 tools/ocaml/xenstored/store.ml                | 49 ++++++-------
 tools/ocaml/xenstored/symbol.ml               | 70 +++++--------------
 tools/ocaml/xenstored/symbol.mli              | 22 ++----
 tools/ocaml/xenstored/trie.ml                 | 61 +++++++---------
 tools/ocaml/xenstored/trie.mli                | 26 +++----
 tools/ocaml/xenstored/xenstored.ml            | 20 +-----
 14 files changed, 98 insertions(+), 206 deletions(-)
 rename tools/ocaml/xenstored/{select.ml => poll.ml} (85%)
 rename tools/ocaml/xenstored/{select.mli => poll.mli} (58%)

--
2.25.1

This all looks good - I left a small comment on one of the patches and I agree 
that this needs testing. I also wonder about compatibility with earlier OCaml 
releases that we support but I see no real obstacles.

-- 
Acked-by: Christian Lindig <christian.lindig@xxxxxxxxxx>