ocaml-dune / csexp

Minimal support for Canonical S-expressions
MIT License
27 stars 7 forks source link

[WIP] Add feed API #9

Closed ghost closed 4 years ago

ghost commented 4 years ago

Alternative to #8. This PR proposes to add a direct parsing API (no functors involved) where the user can feed characters to the parser one by one. It is similar to the API of Parsexp.

  module Parser : sig
    (** Low-level parser *)

    type t

    val create : unit -> t

    (** Feed a character to the parser. The states and stack are updated
        accordingly. Note that the stack is passed explicitely rather than
        stored in the state for performance reason; it avoids calls to
        [caml_modify] which are slow. *)
    val feed : t -> char -> Stack.t -> Stack.t

    (** Feed the end of input to the parser. Return the parsed S-expressions
        contained in the stack. This functions raise if we were in the middle of
        parsing an atom or list. *)
    val feed_eoi : t -> Stack.t -> Sexp.t list
  end

It also fixes input_opt which couldn't return None before.

ghost commented 4 years ago

I added a benchmark in master and parse_string got quite a bit slower in this PR. After a bit of work, the version in this PR is now about 33% and allocates about 73% less on the example in the benchmark:

Branch Time/Run mWd/Run mjWd/Run Prom/Run
master 22.71ms 9.44Mw 1.82Mw 1.82Mw
this PR 15.71ms 2.49Mw 1.75Mw 1.75Mw
ghost commented 4 years ago

I just need to add a bit of doc and this will be good to go.

ghost commented 4 years ago

I added documentation. @rgrinberg and @mefyl could you try it in ocaml-lsp/dune-cache and give some feedback? The documentation is quite extensive, so hopefully it should be straightforward.

ghost commented 4 years ago

I'll make a 1.3 release after that.

rgrinberg commented 4 years ago

This seems fine to me, but I'm not using csexp in LSP at the moment. So we'll have to wait a bit before feedback.