compare_length_with

[cfcs]

This PR provides a compare_length_with in the style of List.compare_length_with (hence the name).
It serves the same purpose as List.compare_length_with: It can be used to replace instances like

if 6 = M.cardinal t then

(which runs in O(n) time and may be very slow with a large t) with

if 0 = M.compare_length_with t 6 then

(which runs in O( min(cardinal t, 6+1) ) time).

Two examples of where this can be useful, but I suspect this is a fairly common use-case in code:

• https://github.com/roburio/udns/blob/4493b1ca50c749d3dbeb013f0a2d2bf6fb9b2fbc/server/dns_server.ml#L130
• https://github.com/roburio/udns/blob/4493b1ca50c749d3dbeb013f0a2d2bf6fb9b2fbc/server/dns_trie.ml#L78

[hannesm]

I'm not at all a fan of that API in the Lists module (neither of adding more functions and mutable state to Gmap). The first call to cardinal checks an invariant for tsig-keys (which contain a single dnskey, thus cardinal is rather fast); the second call is a workaround as commented in the todo, and should be fixed properly.

are there any other cardinal uses you have found that would benefit from compare_length_with?

[cfcs]

Why do you dislike the API in the List module?

Here's another example, similarly guarded with a "TODO" comment about LRU, but probably a better example of where this would shine:
https://github.com/roburio/udns/blob/4493b1ca50c749d3dbeb013f0a2d2bf6fb9b2fbc/server/dns_server.ml#L676-L677

• https://github.com/cfcs/ofetch/blob/master/ofetch_cli.ml#L137 my own implementation of a LRU cache does exactly the same, so if we're anything to go by, this is a fairly intuitive way to implement an inefficient LRU cache ;)

I couldn't find other examples in udns, and not sure what other users of gmap there are.

I did a quick grep through my locally checked out code and found some uses of Map.cardinal where this might make sense in an attempt to backup my argument that partial comparison makes sense to have for map:

• https://github.com/ocaml/opam/blob/master/src/core/opamStd.ml#L194
  • https://github.com/ocaml/opam/blob/master/src/core/opamStd.ml#L288
  • here they need to return the cardinal though, so they're just inefficiently calling it twice - not a good example for my argument
• https://github.com/hannesm/conex/blob/master/src/conex.ml#L32
  • https://github.com/hannesm/conex/blob/master/src/conex_resource.ml#L526
  • I think this is a good example

Most of the other instances I saw was either

• code that could/should be replaced with is_empty t, not (is_empty t)
• code that compared cardinal t = 1, which makes me wonder if an is_singleton function would be sufficient

So basically no conclusive evidence that this is needed.

It also seems like it is counter-intuitive to a lot of people that cardinal is O(n) in the stdlib, so maybe that is an indicator that it should be added to the documentation of Map.Make, but again that is irrelevant to this PR.

Anyway, here's a take on it without mutable state, but it's a lot more verbose than the mutable state version:

let compare_with_length t num =
  let rec loop seen = function
  | Seq.Nil -> seen - num
  | Seq.Cons _ when seen > num -> 1
  | Seq.Cons ( _ , next) -> loop (succ seen) (next ())
  in loop 0 (M.to_seq t ())