Re: Thread-safety issues in QtHandles

[Michael Goffioul]

On Fri, Nov 4, 2011 at 7:51 PM, John W. Eaton <address@hidden> wrote:
> I wrote the octave_allocator class early on in Octave's history
> thinking that it would help performance and memory usage some to not
> call new directly for many small allocations.  I don't really know
> whether it is helpful or just excess baggage.  Probably the easiest
> way to experiment is to make a small change like the following to
> disable it:
>
> diff --git a/liboctave/oct-alloc.h b/liboctave/oct-alloc.h
> --- a/liboctave/oct-alloc.h
> +++ b/liboctave/oct-alloc.h
> @@ -62,6 +62,7 @@
>   bool grow (void);
>  };
>
> +#if 0
>  #if defined (HAVE_PLACEMENT_DELETE)
>  #define DECLARE_OCTAVE_ALLOCATOR_PLACEMENT_DELETE \
>     void operator delete (void *p, void *) \
> @@ -87,5 +88,10 @@
>
>  #define DEFINE_OCTAVE_ALLOCATOR2(t, s) \
>   octave_allocator t::allocator (sizeof (t), s)
> +#else
> +#define DECLARE_OCTAVE_ALLOCATOR
> +#define DEFINE_OCTAVE_ALLOCATOR(t)
> +#define DEFINE_OCTAVE_ALLOCATOR2(t, s)
> +#endif
>
>  #endif

I tried it and got finally rid of the random crashes. My test case is
to start the mirone GUI (which is a pretty heavy user of ui* stuff)
and before all the changes, I could get a crash 50% of the time. Now,
I've launched the GUI 10-15 times and didn't get a single crash.

For the record, although I tested it on a Linux box, remotely through
VNC, my first impression is that octave is actually faster. This is
just a feeling, I didn't time it. But for instance, mirone GUI takes
about 1s to appear (in QtHandles, on a E5300 Intel Dual Core), while
it used to be more in the range 1.5-2s, when it didn't crash. Given
the complexity of mirone's GUI, I think it's pretty good :)

I'll test the changes on Windows and do more tests, I want to sure
QtHandles is stable now. In the coming days, I'll wrap up all the
changes I've made (there are still a few classes using
hand-made/unsafe refcount, I guess it'd be good to convert those as
well), then I'll post the patch on the list for discussion. There are
a lot of changes and you might not like them all. If there's an
agreement, as a first step, maybe I could apply the patch on the gui
branch only.

2 questions, though:
1) what should I do with octave_allocator class?
2) same question for octave_refcount?

Michael.