On Sep 24, 2009, at 3:37 PM, Lisandro Dalcin wrote:
> On Thu, Sep 24, 2009 at 7:16 PM, Robert Bradshaw
> <[email protected]> wrote:
>> On Sep 24, 2009, at 2:50 PM, Lisandro Dalcin wrote:
>>
>>> Sorry, I was not clear enough... if 'profile' directive is of type
>>> 'bool', but initialized to None, it seems that at some point Dag's
>>> transform machinery puts a 'False' there,
>>
>> Ah.
>>
>>> so the
>>> "directives['profile'] is None" does not work as expected...
>>>
>>> So, in short, the 'profile' directive should be 'False' by default;
>>> but if enabled, the macro in the C code should definitely
>>> default to
>>> 1, as currently done.
>>
>> Actually, None (the default) was a special case--enabled for non-
>> inline non-nogil functions. It requires the GIL, so we need that at
>> least, and I disabled it for inline functions for performance
>> reasons.
>>
>
> Ah! I see... but that way is broken...
>
>> Maybe rather than True/False, it should be an int with different
>> thresholds?
>
> I think so.
>
>>
>> If so, what should they be?
>>
>
> What about this?:
>
> 0: do no profile at all
> 1: profile only 'def' functions
> 2: profile 'def' and 'cdef' but not cdef+inline
> =3: profile all, i.e def, cdef, and cdef+inline.
>
> You could also merge 1 and 2 in case the distinction is not worth
> enough.
I think that's a good breakdown. Were you thinking cpdef would get
grouped with def? Another possibility is to do a breakdown based on
lines of code, but that seems rather arbitrary (as well as messier).
I still like being able to specify booleans, so
@cython.profile(True):
cdef inline foo(x):
...
would always add profiling code (e.g. True = infinity) Maybe this
should be a separate directive (i.e. one would have cython.profile
(boolean) and cython.profile_level(int), where the former would
override the latter.)
> BTW, we could also support the support C code being generated, but
> with the define below by default:
>
> #ifndef CYTHON_TRACING
> #define CYTHON_TRACING 0
> #endif
>
> So we could be able to define a default level of tracing (1,2,3) and
> how to compile the C code by default..
Hmm... so which would override which? Also, the cython directives can
be locally applied, unlike C macros (at least as we use them). I do
like being able to explicitly disable all tracing from via C compile
environment if desired.
> Just throwing some ideas...
Thanks for doing so!
>
>>
>> Should any level be on by
>> default? (I think so.)
>>
>
> I personally prefer that profile be off by default (level 0 in my
> proposal above), but as always, I do not bother too much about
> defaults provided that I can change it :-)
>
> Anyway, until you can have some more benchmarking (or in the case you
> do not have the time to do the benchmarking), I think you should
> disable the beast, just in case do not have it unintentionally enabled
> in final release 0.11.3...
Yes, lets have it disabled for 0.11.3 by default, and when I have
time to do some benchmarks we'll think about what to do for 0.12. If
it's cheap enough, it's nice to be able to not have to re-compile to
enable profiling (especially if it's not your code, e.g. it'd be nice
to have it enable for most of Sage). Maybe def methods would be a
nice compromise (it's much less overhead than, e.g., argument
unpacking). I'd like to get input from more people on this too.
- Robert
_______________________________________________
Cython-dev mailing list
[email protected]
http://codespeak.net/mailman/listinfo/cython-dev
