Re: src/ov.h; src/oc.cc: make static member thread-safe

[Jaroslav Hajek]

On Wed, Jan 19, 2011 at 6:56 PM, CdeMills <address@hidden> wrote:
>
> Hello,
>
> I include a patch to make the class octave_base_value thread-safe; meaning:
> - making nil_ref a private static member
> - adding a serializing mechanism if there is no support for gcc atomic ops
> - initialising nil_ref and the mutex once
> - rewriting all access to ref->count either as atomic, either as
> mutex-protected.
>
> The issue is that, when dealing with octave_value::rep, it is not known if
> it use the class-wide, default  octave_base_value object or a dynamically
> allocated one.
>
> 'Octave' does compile, and all tests are OK with this patch applied. Could
> other people review and test the impact on performances ?
>


I think there's been a similar patch before (and it had been on my
TODO list as well), so I may be repeating part of the comments.

1. Injecting the code into the refcounting methods like you did is a
bad(TM) approach. The good(TM) one: There should be a class,
octave_atomic_count (or sth like that) that will encapsulate the count
and support the thread-safe pre/post increment/decrement operators and
typecasting to an integer type. With that, hopefully, the rest of
changes will be just changing the type of the count member fields
declarations.

Random spots:
A. Why would you want to lock the mutex/use the sync primitive in
get_count? Even if it made sense as such, that method is aimed at
low-level optimizations that are thread-unsafe by nature. Instead, we
may need a way to synchronize on the refcount explicitly from outside.
B. Yes, static block-scoped vars are not thread safe, but I don't
believe that getting rid of them is the right approach.
Static class members have other problems (order of initialization).
The right approach is, IMHO, to create some primitives/macros for
thread-safe static initializations.

2. As I said, measuring (at least some) of the performance impact is a
vital part of any such change. It may be simpler than you think; for
instance, you may try to time creation/deletion/copying of a large
cell array using tic/toc. That alone should tell us a lot. For
something more hardcore, you can try adding a benchmark to
OctaveForge/benchmarks.

I hope this didn't discourage you :)