Re: [Libreoffice] [Patch][EasyHacks] Replace ScfRef with boost::shared_ptr in calc filter code

Hi Nigel,

On Fri, 2010-11-19 at 18:29 +0000, Nigel Hawkins wrote:
> Hi,
>
> Attacking another item on the easy hacks page.
>
> The attached patches should remove ScfRef (which was used more than I
> thought it would be) and replace it with boost::shared_ptr.

Great work!  I've applied all your patches and committed as a single
commit.  I hope that's okay.  I've applied your patches mostly as-is,
except for one part which I explain below.

> Most of them are very simple. The last one is to replace all uses of the
> ScfRef.is() function.

Here, you replaced all instances of xFoo.is() with xFoo.get(), which is
itself not incorrect.  But boost::shared_ptr overloads the operator
bool() which returns the wrapped pointer value when a boolean value is
expected, so in this example simply replacing xFoo.is() with xFoo is
sufficient (and cleaner).  I've made this change while applying your
last patch.

> ScfRef had a note about being used instead of shared_ptr because it was
> faster. Some simple profiling suggested it was about two and a half
> times faster, but that unless we're throwing these around in their
> millions, there wouldn't be any significant (or even noticeable)
> performance hit.

Agreed.  That was my idea as well.  In case using boost::shared_ptr
causes any significant performance hit, we could easily replace that
with boost::intrusive_ptr for better performance.

Thanks a lot for your work.  Really appreciated. :-)

Kohei

-- 
Kohei Yoshida, LibreOffice hacker, Calc
<kyoshida@novell.com>