Chris00 / ocaml-rope

Ropes ("heavyweight strings") for OCaml
Other
49 stars 3 forks source link

Make Rope.Iterator bounds check more optimistic #6

Open Simn opened 5 years ago

Simn commented 5 years ago

This is the current implementation of peek:

  let peek itr i =
    if i < 0 || i >= itr.len then raise(Out_of_bounds "Rope.Iterator.peek")
    else (
      if itr.current_g0 <= i && i < itr.current_g1 then
        itr.current.[i + itr.current_offset]
      else
        get itr.rope i (* rope get *)
    )

This checks the more general constraint (out-of-bounds) before checking the good path. Given that we can assume current_g0 and current_g1 to be within the rope's bounds, we can prioritize their check without risking out-of-bounds:

  let peek itr i =
    if itr.current_g0 <= i && i < itr.current_g1 then
      itr.current.[i + itr.current_offset]
    else if i < 0 || i >= itr.len then
      raise(Out_of_bounds "Rope.Iterator.peek")
    else
      get itr.rope i (* rope get *)

This should bring a small performance benefit when algorithms are within the local bounds a lot, which is generally why iterators are used in the first place.

By the same token, get can also check the local bounds first.


On a related note, what is the reason for peek to not update the current leaf like get does? I ran into a bit of a performance trap with an algorithm that would just call peek a lot and thus never update the current leaf. My intuition was that get itr = peek itr (pos itr).