Re: [Ocaml-lib-devel] a bug in BitSet?

[Bardur A. <oca...@sc...>]

On Sun, Dec 19, 2004 at 03:44:54PM +0200, Janne Hellsten wrote:

> 
> On Sun, 19 Dec 2004, Bardur Arantsson wrote:
> >I'm no expert on OCaml optimization, but I believe there
> >is a considerable performance benefit in using unsafe_get
> >(bget), unsafe_set (bset), etc. because you can avoid
> >bounds checking and always get proper inlining. Whether
> >this level of optimization is really necessary for BitSet
> >I'll leave as an exercise for the reader. :)
> 
> ...
> 
> >I don't think the compiler is currently clever enough (or
> >maybe the language semantics are not strict enough?) to do
> >all the needed inlining, and the bounds checking would
> >still slow you down.
> 
> I took the current BitSet module, modified bget and bset to just use 
> String.[] normally and added the couple of necessary char_of_ints and 
> int_of_chars.  I then went on a compiled the BitSet module with ocamlopt 
> -unsafe (to turn off bounds checking).  The resulting assembly is pretty 
> much the same as with the Obj.magic+unsafe_get trickery, except that 
> ocamlopt seems to be unable to inline int_of_char calls (which was a 
> surprise to me).

Generally I would say one should always compile without
-unsafe and use unsafe_get and companions in those
instances where it can be *guaranteed* to stay within the
bounds *AND* where it matters for performance.

Even in cases where it is easy to prove that the index is
always within bounds, I wouldn't use unsafe_get/set unless
it actually matters a great deal for performance. It
reduces readability and makes code harder to modify later.

> 
> >I think compiling without bounds checking for "release"
> >builds is unwise since it may obscure serious bugs which
> >haven't been caught by the test suite. It's basically rude
> >to anyone using the library to (possibly) obscure bugs
> >within the library this way.
> 
> Yes.  However, it seems that many parts of the library
> *are* already using unsafe_gets.

Yup. Personally, I'd like to see some instances of such
use removed from some ExtLib code, but I don't care enough
to submit patches :). Examples include Dbi.string_escaped
and UTF8.look where the performance difference is most
likely tiny.

In my opinion overusing unsafe_get/set sets a bad example
because people will assume they'll always write bug-free
code and just end up using unsafe_get/set "by default".

-- 
Bardur Arantsson
<ba...@im...>
<ba...@sc...>

- If you can remember drinking it, you obviously didn't drink
enough of it.
                                                               Me