Open thurstond opened 3 years ago
These are amazing (a/k/a awful) performance issues, thanks for finding them!
Another source of inefficiency in parse_tilde
is that OCaml by default has eager evaluation, which means implode
will be called at every per-character iteration of parse_tilde
, instead of just once when the ret
value is actually needed
i.e., for an n-character input, implode
will be about n times, leading to O(n^2) runtime.
Code snippet where this happens in ast.ml:
parse_tilde acc =
let ret = if acc = [] then None else Some (implode acc) in
function
| [] -> (ret , [])
...
| c::s' -> parse_tilde (acc @ [c]) s'
To show this, I changed dash.ast
to add a printf
to the implode
function:
let implode l =
let s = Bytes.create (List.length l) in
let rec imp i l =
match l with
| [] -> ()
| (c::l) -> (Bytes.set s i c; imp (i+1) l)
in
imp 0 l;
Printf.printf "implode created: %s\n" s;
Bytes.unsafe_to_string s
Output:
/pash/compiler/parser# echo "~lovecraft" | ./parse_to_json.native
implode created: l
implode created: lo
implode created: lov
implode created: love
implode created: lovec
implode created: lovecr
implode created: lovecra
implode created: lovecraf
implode created: lovecraft
["Command",[1,[],[[["T",["Some","lovecraft"]]]],[]]]
There is a "Lazy" module in OCaml, or just use Haskell 🙃
😬 😅
Linear-time versions of parse_tilde
and to_assign
(changing list append to list prepend + one-off reverse, and removing the eager implode in parse_tilde):
and maybe_implode_rev acc =
if acc = [] then None else Some (implode (List.rev acc))
and parse_tilde acc =
function
| [] -> (maybe_implode_rev acc, [])
(* CTLESC *)
| '\129'::_ as s -> None, s
(* CTLQUOTEMARK *)
| '\136'::_ as s -> None, s
(* terminal: CTLENDVAR, /, : *)
| '\131'::_ as s -> maybe_implode_rev acc, s
| ':'::_ as s -> maybe_implode_rev acc, s
| '/'::_ as s -> maybe_implode_rev acc, s
(* ordinary char *)
(* TODO 2019-01-03 only characters from the portable character set *)
| c::s' -> parse_tilde (c :: acc) s'
maybe_implode_rev acc
is not as pretty as having ret
(or a lazy version of it), but it's fast.and to_assign v = function
| [] -> failwith ("Never found an '=' sign in assignment, got " ^ implode (List.rev v))
| C '=' :: a -> (implode (List.rev v),a)
| C c :: a -> to_assign (c :: v) a
| _ -> failwith "Unexpected special character in assignment"
Relatedly, parse_arg
and arg_char
are using the call stack when they shouldn't need to.
Parsing and unparsing have super-linear runtime, largely because OCaml list append and string concatenation are not optimized.
Parsing: parse_tilde quadratic (/cubic?) behavior
(real/sys runtime omitted for brevity)
Notes:
| c::s' -> parse_tilde (acc @ [c]) s'
. It's not obvious to me why it seems to be cubic (rather than quadratic) runtime though.Parsing: to_assign cubic (?) behavior
This also has an expensive list append operation: ast.ml
| C c :: a -> to_assign (v @ [c]) a
.Unparsing: ^ string concatenation considered harmful
OCaml's "^" string operator is not optimized; concatenating n strings one at a time can take O(n^2) runtime (https://discuss.ocaml.org/t/whats-the-fastest-way-to-concatenate-strings/2616/7). This is arguably a compiler issue e.g., CPython optimizes for common cases (https://mail.python.org/pipermail/python-dev/2013-February/124031.html).
ast.ml's unparsing is essentially a series of "^" operations, hence everything is going to have a worst-case runtime that's super-linear.
Here's an example of a long pipeline, showing quadratic runtime for json_to_shell:
I also tried removing the Ast.to_string operation in json_to_shell.ml, to show that the JSON deserialization by itself is fast (linear); thus, the quadratic runtime is due to the libdash core.
Unparsing: fresh_marker for heredocs is slow on adversarial inputs
fresh_marker tries to find increasingly long-variants of {EOF, EOFF, EOFFF ..}, until it can find a marker that is not contained in the heredoc. This is slow for adversarial inputs that deliberately use all those markers: