octave-maintainers
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Thread-safety issues in QtHandles


From: John W. Eaton
Subject: Re: Thread-safety issues in QtHandles
Date: Mon, 7 Nov 2011 14:34:06 -0500

On  7-Nov-2011, Michael Goffioul wrote:

| On Mon, Nov 7, 2011 at 8:25 AM, John W. Eaton <address@hidden> wrote:
| > On  5-Nov-2011, Michael Goffioul wrote:
| >
| > | I'm fine with your change. I suggest you apply it, so I can build my
| > | patch on it. AFAIK, if valgrind reports uninitialized values, they
| > | were also uninitialized before removing the octave_allocator, as that
| > | class doesn't initialize the allocated memory. And if the memory was
| > | initialized from a previous allocation, then the content is wrong
| > | anyway. In that regards, I don't think removing octave_allocator
| > | changed anything.
| >
| > Right, I didn't mean to imply that things were correct previously.  I
| > was just thinking that the implementation of the octave_allocator
| > class could explain why valgrind doesn't see errors about
| > uninitialized data.
| 
| I didn't mean you meant that either :) I was just thinking out loud.
| 
| BTW, the patch I sent contained 2 errors that prevent compilation. The
| attached patch fix that: compilation OK, test suite OK. There's no
| functional difference from the previous one, but at least you can test
| this one.

OK, I tried the allocator patch and your changeset and did some
timings.

First, with neither patch applied, or with just the allocator patch, I
get about 61 seconds to run the tests (cd to the tests directory in my
build tree and run them directly with /usr/bin/time ../run-octave ...).

After applying your patch in addition to the allocator patch, I get
about 76 seconds.  That's a hefty performance penalty.

I used the default build flags in all cases, so I have -O2
in CXXFLAGS so I don't think this problem is due to not inlining
functions.

jwe


reply via email to

[Prev in Thread] Current Thread [Next in Thread]