You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Madan U Sreenivasan <ma...@collab.net> on 2006/07/06 11:23:31 UTC
question about range_intersect() and range_contains() functions
Hi,
In the merge-tracking branch, I notice that the range_intersect() and
the range_contains() functions in the
subversion/libsvn_subr/mergeinfo.c file are single line functions. Am
thinking why not make them a macro. Is there any specific reason these are
functions?
Regards,
Madan.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: question about range_intersect() and range_contains() functions
Posted by Daniel Berlin <db...@dberlin.org>.
Madan U Sreenivasan wrote:
> On Thu, 06 Jul 2006 17:38:57 +0530, Malcolm Rowe
> <ma...@farside.org.uk> wrote:
>
>> On Thu, Jul 06, 2006 at 05:44:45PM +0530, Madan U Sreenivasan wrote:
>>> On Thu, 06 Jul 2006 17:08:03 +0530, Malcolm Rowe
>>> <ma...@farside.org.uk> wrote:
> [snip]
>>>> Are these public functions? (I haven't looked). If so, making them
>>>> macros would mean that we couldn't change the implementation, which
>>>> might be a good reason to keep them as functions.
>>> no these are static functions.
>>>
>> Ok, so what's the benefit of using a macro here? (which is what I should
>> have asked first).
>>
>> The compiler can easily inline a static function if it thinks that's
>> the right thing to do (given the current optimisation settings), and
>> functions have argument type-checking and obvious side effects, which
>> macros don't.
>
> I think its better to have single line functions as macros, than rely on
> the compiler to detect it.
>
It is not a matter of detection. GCC knows better than you do what it
should be inlining.
Also, macros are still impossible to debug without using special options
to the compiler to generate macro debug info (in gcc's case, -g3).
There is simply no advantage to making it harder to debug by using
macros here. This is not performance critical code, and the compiler is
going to inline it anyway.
--Dan
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: question about range_intersect() and range_contains() functions
Posted by Malcolm Rowe <ma...@farside.org.uk>.
On Thu, Jul 06, 2006 at 06:44:33PM +0530, Madan U Sreenivasan wrote:
> >Ok, so what's the benefit of using a macro here? (which is what I should
> >have asked first).
> >
> >The compiler can easily inline a static function if it thinks that's
> >the right thing to do (given the current optimisation settings), and
> >functions have argument type-checking and obvious side effects, which
> >macros don't.
>
> I think its better to have single line functions as macros, than rely on
> the compiler to detect it.
>
The compiler is smarter than us, and this isn't performance-critical code
(the callers aren't in tight loops, for example). And what if the user
wants to optimise for space rather than speed? (though admittedly in
this case it's likely that the inlined version of the function might
be smaller).
> >Really, the rule should be that we only use macros where we absolutely
> >have to.
>
> and when do you think it is? (am asking as a matter of trying to
> understand :)
>
Apart from constants, which we need to use macros for in C, I say that
we should use macros only when we have no alternative, when we actually
need to use the macro functionality, not for simple function.
Good examples in our codebase would be: the error code functionality
(include/svn_error_codes.h), where we change the macro definition to
generate different output; SVN_ERR(), which generates inline code
that we can't replicate with a function; the token-pasting macros
in e.g. mod_dav_svn/liveprops.c (SVN_RO_DAV_PROP et al); and the RA
compatibility wrappers in libsvn_ra\wrapper_template.h.
Regards,
Malcolm
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: question about range_intersect() and range_contains() functions
Posted by Madan U Sreenivasan <ma...@collab.net>.
On Thu, 06 Jul 2006 17:38:57 +0530, Malcolm Rowe
<ma...@farside.org.uk> wrote:
> On Thu, Jul 06, 2006 at 05:44:45PM +0530, Madan U Sreenivasan wrote:
>> On Thu, 06 Jul 2006 17:08:03 +0530, Malcolm Rowe
>> <ma...@farside.org.uk> wrote:
[snip]
>> >Are these public functions? (I haven't looked). If so, making them
>> >macros would mean that we couldn't change the implementation, which
>> >might be a good reason to keep them as functions.
>>
>> no these are static functions.
>>
>
> Ok, so what's the benefit of using a macro here? (which is what I should
> have asked first).
>
> The compiler can easily inline a static function if it thinks that's
> the right thing to do (given the current optimisation settings), and
> functions have argument type-checking and obvious side effects, which
> macros don't.
I think its better to have single line functions as macros, than rely on
the compiler to detect it.
> Really, the rule should be that we only use macros where we absolutely
> have to.
and when do you think it is? (am asking as a matter of trying to
understand :)
Regards,
Madan.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: question about range_intersect() and range_contains() functions
Posted by Malcolm Rowe <ma...@farside.org.uk>.
On Thu, Jul 06, 2006 at 05:44:45PM +0530, Madan U Sreenivasan wrote:
> On Thu, 06 Jul 2006 17:08:03 +0530, Malcolm Rowe
> <ma...@farside.org.uk> wrote:
>
> >On Thu, Jul 06, 2006 at 04:53:31PM +0530, Madan U Sreenivasan wrote:
> >> In the merge-tracking branch, I notice that the range_intersect() and
> >>the range_contains() functions in the
> >>subversion/libsvn_subr/mergeinfo.c file are single line functions. Am
> >>thinking why not make them a macro. Is there any specific reason these
> >>are
> >>functions?
> >>
> >
> >Are these public functions? (I haven't looked). If so, making them
> >macros would mean that we couldn't change the implementation, which
> >might be a good reason to keep them as functions.
>
> no these are static functions.
>
Ok, so what's the benefit of using a macro here? (which is what I should
have asked first).
The compiler can easily inline a static function if it thinks that's
the right thing to do (given the current optimisation settings), and
functions have argument type-checking and obvious side effects, which
macros don't.
Really, the rule should be that we only use macros where we absolutely
have to. (Implying that the MAX() macro at the top of mergeinfo.c should
probably be a function too, although MAX is sometime defined by the C
library, so maybe that's okay).
Regards,
Malcolm
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: question about range_intersect() and range_contains() functions
Posted by Madan U Sreenivasan <ma...@collab.net>.
On Thu, 06 Jul 2006 17:08:03 +0530, Malcolm Rowe
<ma...@farside.org.uk> wrote:
> On Thu, Jul 06, 2006 at 04:53:31PM +0530, Madan U Sreenivasan wrote:
>> In the merge-tracking branch, I notice that the range_intersect() and
>> the range_contains() functions in the
>> subversion/libsvn_subr/mergeinfo.c file are single line functions. Am
>> thinking why not make them a macro. Is there any specific reason these
>> are
>> functions?
>>
>
> Are these public functions? (I haven't looked). If so, making them
> macros would mean that we couldn't change the implementation, which
> might be a good reason to keep them as functions.
no these are static functions.
Regards,
Madan.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: question about range_intersect() and range_contains() functions
Posted by Malcolm Rowe <ma...@farside.org.uk>.
On Thu, Jul 06, 2006 at 04:53:31PM +0530, Madan U Sreenivasan wrote:
> In the merge-tracking branch, I notice that the range_intersect() and
> the range_contains() functions in the
> subversion/libsvn_subr/mergeinfo.c file are single line functions. Am
> thinking why not make them a macro. Is there any specific reason these are
> functions?
>
Are these public functions? (I haven't looked). If so, making them
macros would mean that we couldn't change the implementation, which
might be a good reason to keep them as functions.
Regards,
Malcolm
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org