subject:"\[issue43760\] The DISPATCH\(\) macro is not as efficient as it could be."



Pablo Galindo Salgado  added the comment:

Also, I checked the DWARF tree of all existing wheels for 3.10 on PyPI (there 
aren't many) and none had anything that uses the size of the struct.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43760] The DISPATCH() macro is not as efficient as it could be (move PyThreadState.use_tracing)



Pablo Galindo Salgado  added the comment:

> I'll just note that a change in struct size does technically break ABI, since 
> *arrays* of PyThreadState will break.

Not that matters now because we are not proceeding but just to clarify why I 
deemed this acceptable: arrays of PyThreadState is extremelly unlikely in 
extensions because we pass it by Pointer and is always manipulated by pointer. 
To place it in an array you either need to create one or copy one into an 
array, which I cannot see what would be the point because the fields are mainly 
pointers that would become useless as the interpreter will not update anything

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43760] The DISPATCH() macro is not as efficient as it could be (move PyThreadState.use_tracing)

2021-09-22 Thread Petr Viktorin



Petr Viktorin  added the comment:

> The ABI is not broken, the only thing that this PR change is the size of the 
> struct. All the offsets to the members are the same and therefore will be 
> valid in any compiled code.

I'll just note that a change in struct size does technically break ABI, since 
*arrays* of PyThreadState will break.

So the size shouldn't be changed in RCs or point releases. (But since it's not 
part of stable ABI, it was OK to change it for 3.10.)

> We will try to determine and open a discussion in the future about what is 
> considered public/private in these ambiguous cases and what can users expect 
> regarding stability and backwards compatibility.

Please keep me in the loop; I'm working on summarizing my understanding of this 
(in a form that can be added to the docs if approved).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43760] The DISPATCH() macro is not as efficient as it could be (move PyThreadState.use_tracing)



Pablo Galindo Salgado  added the comment:

I discussed this particular instance with the Steering Council and the 
conclusion was that this field (use_tracing) is considered an implementation 
detail and therefore its removal it's justified so we won't be restoring it.

I'm therefore closing PR28498

Notice that this decision only affects this particular issue and should not be 
generalized to other fields or structures. We will try to determine and open a 
discusion in the future about what is considered public/private in these 
ambiguous cases and what can users expect regarding stability and backwards 
compatibility.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43760] The DISPATCH() macro is not as efficient as it could be (move PyThreadState.use_tracing)



Pablo Galindo Salgado  added the comment:

I'm removing the release blocker as per above, feel free to close of there is 
nothing else to discuss or act on here.

--
priority: release blocker -> 

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43760] The DISPATCH() macro is not as efficient as it could be (move PyThreadState.use_tracing)



Pablo Galindo Salgado  added the comment:

Also, I personally thing there is absolutely no guarantee that Cython code 
generated for 3.9 should work for 3.10 and the thread state is a private 
structure that has undocumented fields and is not part of the stable API nor 
the limited API so, tstate->tracing disappearing is totally withing the 
guarantees between Python versions.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43760] The DISPATCH() macro is not as efficient as it could be (move PyThreadState.use_tracing)



Pablo Galindo Salgado  added the comment:

Also, just to clarify, I also opened PR 28498 to discuss the possibility of 
going ahead, I still don't want to move on without consensus.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43760] The DISPATCH() macro is not as efficient as it could be (move PyThreadState.use_tracing)



Pablo Galindo Salgado  added the comment:

>From PR 28498:

@vstinner @ambv The ABI is not broken, the only thing that this PR change is 
the size of the struct. All the offsets to the members are the same and 
therefore will be valid in any compiled code.

Any compiled wheels will still work. Look at the ABI report:

  [C]'function void PyEval_AcquireThread(PyThreadState*)' at ceval.c:447:1 has 
some indirect sub-type changes:
parameter 1 of type 'PyThreadState*' has sub-type changes:
  in pointed to type 'typedef PyThreadState' at pystate.h:20:1:
underlying type 'struct _ts' at pystate.h:62:1 changed:
  type size changed from 2240 to 2304 (in bits)
  1 data member insertion:
'int _ts::use_tracing', at offset 2240 (in bits) at pystate.h:151:1
  1 data member changes (2 filtered):
   type of 'PyInterpreterState* _ts::interp' changed:
 in pointed to type 'typedef PyInterpreterState' at pystate.h:22:1:
   underlying type 'struct _is' at pycore_interp.h:220:1 changed:
 type size hasn't changed
 1 data member changes (3 filtered):
  type of '_PyFrameEvalFunction _is::eval_frame' changed:
underlying type 'PyObject* (PyThreadState*, PyFrameObject*, 
int)*' changed:
  in pointed to type 'function type PyObject* 
(PyThreadState*, PyFrameObject*, int)':
parameter 1 of type 'PyThreadState*' has sub-type 
changes:
  in pointed to type 'typedef PyThreadState' at 
pystate.h:20:1:
underlying type 'struct _ts' at pystate.h:62:1 
changed:
  type size changed from 2240 to 2304 (in bits)
  1 data member insertion:
'int _ts::use_tracing', at offset 2240 (in 
bits) at pystate.h:151:1
  1 data member changes (2 filtered):
   type of '_ts* _ts::next' changed:
 in pointed to type 'struct _ts' at 
pystate.h:62:1:
   type size changed from 2240 to 2304 (in bits)
   1 data member insertion:
 'int _ts::use_tracing', at offset 2240 (in 
bits) at pystate.h:151:1
   no data member changes (2 filtered);




  [C]'function PyThreadState* PyGILState_GetThisThreadState()' at 
pystate.c:1455:1 has some indirect sub-type changes:
return type changed:
  in pointed to type 'typedef PyThreadState' at pystate.h:20:1:
underlying type 'struct _ts' at pystate.h:62:1 changed:
  type size changed from 2240 to 2304 (in bits)
  1 data member insertion:
'int _ts::use_tracing', at offset 2240 (in bits) at pystate.h:151:1
  no data member changes (4 filtered);

  [C]'function int _PyErr_CheckSignalsTstate(PyThreadState*)' at 
signalmodule.c:1767:1 has some indirect sub-type changes:
parameter 1 of type 'PyThreadState*' has sub-type changes:
  in pointed to type 'typedef PyThreadState' at pystate.h:20:1:
underlying type 'struct _ts' at pystate.h:62:1 changed:
  type size changed from 2240 to 2304 (in bits)
  1 data member insertion:
'int _ts::use_tracing', at offset 2240 (in bits) at pystate.h:151:1
  no data member changes (3 filtered);

  [C]'function void _PyErr_Clear(PyThreadState*)' at errors.c:453:1 has some 
indirect sub-type changes:
parameter 1 of type 'PyThreadState*' has sub-type changes:
  in pointed to type 'typedef PyThreadState' at pystate.h:20:1:
underlying type 'struct _ts' at pystate.h:62:1 changed:
  type size changed from 2240 to 2304 (in bits)
  1 data member insertion:
'int _ts::use_tracing', at offset 2240 (in bits) at pystate.h:151:1
  no data member changes (3 filtered);
As you can see, the leaves of the change is only type size changed from 2240 to 
2304. As the member is added

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43760] The DISPATCH() macro is not as efficient as it could be (move PyThreadState.use_tracing)



Change by Pablo Galindo Salgado :


--
pull_requests: +26893
pull_request: https://github.com/python/cpython/pull/28498

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43760] The DISPATCH() macro is not as efficient as it could be (move PyThreadState.use_tracing)

2021-09-20 Thread Guido van Rossum



Change by Guido van Rossum :


--
nosy: +gvanrossum

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43760] The DISPATCH() macro is not as efficient as it could be (move PyThreadState.use_tracing)

2021-09-20 Thread Miro Hrončok


Miro Hrončok  added the comment:

> It would mean that extensions compiled against the release candidates may not 
> be binary compatible with the final release

If that's true, I definitively argue not to do that. We've told everybody it 
won't happen.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43760] The DISPATCH() macro is not as efficient as it could be (move PyThreadState.use_tracing)

2021-09-20 Thread John Ehresman



John Ehresman  added the comment:

Is adding the field back an option at this point? It would mean that extensions 
compiled against the release candidates may not be binary compatible with the 
final release

My take is that use_tracing is an implementation and version dependent field, 
and that binary compatibility will be maintained for a specific release (e.g. 
3.10) but that there's no assurance that it will be there in the next release 
-- though these things tend not to change. I also regard generated cython code 
as only being valid for the releases that a specific cython version supports.

Code and API's change slowly, but eventually they do change.

--
nosy: +jpe

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43760] The DISPATCH() macro is not as efficient as it could be (move PyThreadState.use_tracing)

2021-09-20 Thread Stefan Behnel


Stefan Behnel  added the comment:

> The code in the examples given above are using `tstate->use_tracing` assuming 
> that its meaning is to determine whether tracing is turned on or not.

No, actually not. It is using the field in the same way as CPython, simply 
because most of this code was originally copied from CPython, and we also 
copied the optimisation of avoiding to check the other fields (for the obvious 
reason of being an optimisation).


> I propose adding back `tstate->use_tracing` as a copy of 
> `tstate->cframe->us_tracing`.

Cython 0.29.24 has already been adapted to this change and will use the new 
field in CPython 3.10b1+.


> Any code that reads `tstate->use_tracing` will work correctly.

Any code that reads and /writes/ the field would probably also continue to work 
correctly, which is what older Cython versions did.


> if_you_use_this_then_we_will_break_your_code_in_some_way_that_will_ruin_your_reputation_as_a_library_developer…

The thing is, new APIs can only be added to new CPython releases. Supporting 
features in older CPython versions (currently 2.7+) means that we always *have 
to* use the existing fields, and can only switch to new APIs by duplicating 
code based on a PY_VERSION_HEX preprocessor check. Even if a new low-latency 
profiling API was added in CPython 3.11, we'd have to wait until there is at 
least an alpha release that has it before enabling this code switch.

And if the new API proves to be slower, we may end up keeping the old code 
around and adding a C compile-time configuration option for users to enable (or 
disable) its use. Cython has lots of those these days, mostly to support the 
different C-API capabilities of different Python implementations, e.g. to take 
advantage of the PyLong or PyUnicode internals if available, and use generic 
C-API calls if not.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43760] The DISPATCH() macro is not as efficient as it could be (move PyThreadState.use_tracing)

2021-09-20 Thread Xtrem532



Change by Xtrem532 :


--
nosy: +Xtrem532

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43760] The DISPATCH() macro is not as efficient as it could be (move PyThreadState.use_tracing)

2021-09-20 Thread Mark Shannon



Change by Mark Shannon :


--
pull_requests: +26872
pull_request: https://github.com/python/cpython/pull/28474

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43760] The DISPATCH() macro is not as efficient as it could be (move PyThreadState.use_tracing)

2021-09-20 Thread Mark Shannon

Mark Shannon added the comment:

IMO those failures are bugs in the projects listed not in CPython.

Relying on the exact meaning, or even the existence of an undocumented field of
a C struct is not, nor ever has been, safe.
The user of the field is assuming a meaning that is not known to the developers
of the library/application, so there is no way for them to preserve that
meaning.
This is not specific to CPython, but applies to any C API.

The code in the examples given above are using `tstate->use_tracing` assuming
that its meaning is to determine whether tracing is turned on or not.
However, it has no such meaning. It is an internal performance optimization to
avoid checking both `tstate->c_tracefunc` and `tstate->c_profilefunc`.
It is `tstate->tracing` that determines whether tracing is active or not.

I propose adding back `tstate->use_tracing` as a copy of
`tstate->cframe->us_tracing`. Any writes to `tstate->use_tracing` will be
ignored, but any code that also sets `tstate->tracing`, as the Cython code
does, will work correctly. Any code that reads `tstate->use_tracing` will work
correctly.

I'm minded to prefix all the names of all fields in all C structs that happen
to be included by Python.h with
"if_you_use_this_then_we_will_break_your_code_in_some_way_that_will_ruin_your_reputation_as_a_library_developer__maybe_not_tomorrow_maybe_not_next_year_but_someday"
Although that might prove tricky with a 80 character line limit :)

My attempts to avoid this happening again next year, and the year after that,
and...
https://bugs.python.org/issue45247
https://github.com/cython/cython/issues/4382

___
Python tracker

___
___
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43760] The DISPATCH() macro is not as efficient as it could be (move PyThreadState.use_tracing)

2021-09-20 Thread Erlend E. Aasland



Change by Erlend E. Aasland :


--
nosy: +erlendaasland

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43760] The DISPATCH() macro is not as efficient as it could be (move PyThreadState.use_tracing)

2021-09-20 Thread STINNER Victor



STINNER Victor  added the comment:

> A Cython issue report: https://github.com/cython/cython/issues/4153

Cython 0.29.24 released at July 13, 2021 with a fix (2 commits):

* 
https://github.com/cython/cython/commit/be3b178296975b976107f41a6383291701e0297f
* 
https://github.com/cython/cython/commit/8d177f4aa51a663e8c51de3210ccb329d7629d36

The Cython master branch was fixed as well: see 
https://github.com/cython/cython/issues/4153

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43760] The DISPATCH() macro is not as efficient as it could be (move PyThreadState.use_tracing)

2021-09-19 Thread Miro Hrončok


Miro Hrončok  added the comment:

Also Numba is broken: https://bugzilla.redhat.com/show_bug.cgi?id=2005686

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43760] The DISPATCH() macro is not as efficient as it could be (move PyThreadState.use_tracing)

2021-09-19 Thread STINNER Victor



STINNER Victor  added the comment:

The commit 28d28e053db6b69d91c2dfd579207cd8ccbc39e7 caused a performance 
regression on Windows which is currently blocking the Python 3.10.0 final 
release: bpo-45116.

Moroever, this issue introduced a incompatible C API change which is not 
documented in What's New in Python 3.10, and it doesn't provide any solution 
for projects broken by these changes. So far, the following projects are known 
to be broken by the change:

* Cython
* greenlet (fixed)
* dipy
* yappi
* smartcols

Would it be possible to:

* Bare minimum: document the change in What's in Python 3.10?

* Provide a solution to broken project? If possible, solution working on old 
and new Python versions. Maybe a compatibility functions can added to 
https://github.com/pythoncapi/pythoncapi_compat if needed.

* Maybe revert the change in Python 3.10 since a full solution may require 
additional work.

By the way, I'm also disappointed that nothing was done to enhance the 
situation for 4 months (since the first known projects were reported here in 
May).

I raise the priority to release blocker to make more people aware of the 
situation.

--
nosy: +pablogsal
priority: normal -> release blocker

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43760] The DISPATCH() macro is not as efficient as it could be (move PyThreadState.use_tracing)

2021-05-14 Thread Stefan Behnel



Change by Stefan Behnel :


--
Removed message: https://bugs.python.org/msg393667

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43760] The DISPATCH() macro is not as efficient as it could be (move PyThreadState.use_tracing)

2021-05-14 Thread Stefan Behnel



Stefan Behnel  added the comment:

Should we allow empty CFrame entries (with a NULL Frame pointer) that only use 
their "use_tracing" flag? That would make it easy for code to append and remove 
such an entry in order to temporarily disable tracing.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43760] The DISPATCH() macro is not as efficient as it could be (move PyThreadState.use_tracing)

2021-05-14 Thread Stefan Behnel



Stefan Behnel  added the comment:

I just noticed that new C-API functions are probably useless for Cython since I 
think it will have to maintain the CFrame stack, so not to enable "use_tracing" 
for the (Python) caller but the current (Cython) function. This then means that 
we own The current CFrame as well as its "use_tracing" field and don't need any 
help from CPython in order to change the state.

I'm not sure if this is any different for other users of the "use_tracing" 
field.

--
type:  -> performance

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43760] The DISPATCH() macro is not as efficient as it could be (move PyThreadState.use_tracing)

2021-05-11 Thread Stefan Behnel



Stefan Behnel  added the comment:

For the same reason that motivated this ticket, I think the functions should be 
inline functions. They should also take the current thread-state as argument, 
because that's probably known on the caller side already.

I guess a macro would be fine, too. :)

Cython previously used "use_tracing" directly because it needs to implement the 
exact same tracing/profiling behaviour as CPython, regardless of who called a 
Cython implemented function (Cython or CPython).

Naming nit: Get/Is/UsesTracing?

Also, given that a common use case seems to be "make sure tracing is disabled, 
do something, enable tracing if it was enabled", I think DisableTracing() 
should return the previous state.

--
nosy: +scoder

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43760] The DISPATCH() macro is not as efficient as it could be (move PyThreadState.use_tracing)

2021-05-11 Thread Miro Hrončok


Miro Hrončok  added the comment:

A Cython issue report: https://github.com/cython/cython/issues/4153

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43760] The DISPATCH() macro is not as efficient as it could be (move PyThreadState.use_tracing)

2021-05-10 Thread Raymond Hettinger



Raymond Hettinger  added the comment:

+1 for Victor's suggestions.  It provides a reasonable way forward without 
locking in eval-loop implementation details that weren't intended to be public 
and frozen in time.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43760] The DISPATCH() macro is not as efficient as it could be (move PyThreadState.use_tracing)

2021-05-10 Thread STINNER Victor



Change by STINNER Victor :


--
components: +C API, Interpreter Core

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43760] The DISPATCH() macro is not as efficient as it could be (move PyThreadState.use_tracing)

2021-05-10 Thread STINNER Victor



Change by STINNER Victor :


--
title: The DISPATCH() macro is not as efficient as it could be. -> The 
DISPATCH() macro is not as efficient as it could be (move 
PyThreadState.use_tracing)
versions: +Python 3.10

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43760] The DISPATCH() macro is not as efficient as it could be.

2021-05-10 Thread STINNER Victor



STINNER Victor  added the comment:

I understand that some projects manually call the profile and/or trace 
functions, and temporarily set use_tracing 0 while calling these functions.

Some projects restore use_tracing to the correct value (compute the efficient 
value), some projects simply set use_tracing to 1.

I see 3 use cases:

* disable tracing temporarily (set use_tracing to 0)
* reenable tracing (compute use_tracing to the correct value)
* check if tracing is used (get use_tracing)

We can add 3 functions:

* PyThreadState_DisableTracing()
* PyThreadState_EnableTracing()
* PyThreadState_GetTracing()

PyThreadState_EnableTracing(tstate) would do something like:

tstate->cframe->use_tracing = (tstate->c_tracefunc || 
tstate->c_profilefunc);

If we added these functions, I can then add an implementation for Python 3.9 
and older to my https://github.com/pythoncapi/pythoncapi_compat project for 
backward compatibility.

The problem is that some projects also increase temporarily ts->tracing. Since 
I would like to make PyThreadState opaque, I would prefer to hide this access 
behind a function call as well. Maybe we need an API to call profile and/or 
trace functions?

--

According to the bugzilla compiler errors:

> greenlet: https://bugzilla.redhat.com/show_bug.cgi?id=1957784

It has already been fixed:

* 
https://github.com/python-greenlet/greenlet/commit/6c5f0963eb00eeb1cfb337c6edbd3efaf7d8eacc
* 
https://github.com/python-greenlet/greenlet/commit/352b974447bb489cb2778e07c3832d0cc60e0e4a

It uses:

* "tstate->use_tracing = 0;"
* "tstate->use_tracing = (tstate->tracing <= 0 && (...)"

> dipy: https://bugzilla.redhat.com/show_bug.cgi?id=1958203

It uses:

* "tstate->use_tracing = 0;"
* "tstate->use_tracing = 1;"
* "tstate->use_tracing = (tstate->c_profilefunc || (...)"
* "return tstate->use_tracing && retval;"
* "if (tstate->use_tracing) {"

> yappi: https://bugzilla.redhat.com/show_bug.cgi?id=1958896

* "ts->use_tracing = 1;"
* "ts->use_tracing = 0;"

> smartcols: https://bugzilla.redhat.com/show_bug.cgi?id=1958938

It uses "tstate->use_tracing = 0;".

> scikit-learn: https://bugzilla.redhat.com/show_bug.cgi?id=1958976

It uses:

* "tstate->use_tracing = 0;"
* "tstate->use_tracing = 1;"

> The usage comes from 
> https://github.com/cython/cython/blob/master/Cython/Utility/Profile.c

Simplified code:

--
static int __Pyx_TraceSetupAndCall(...)
{
...
tstate->tracing++;
tstate->use_tracing = 0;

if (tstate->c_tracefunc)
retval = tstate->c_tracefunc(tstate->c_traceobj, *frame, PyTrace_CALL, 
NULL) == 0;
if (retval && tstate->c_profilefunc)
retval = tstate->c_profilefunc(tstate->c_profileobj, *frame, 
PyTrace_CALL, NULL) == 0;

tstate->use_tracing = (tstate->c_profilefunc ||
   (CYTHON_TRACE && tstate->c_tracefunc));
tstate->tracing--;
...
}

  int __Pyx_use_tracing = 0;

  #define __Pyx_TraceCall(funcname, srcfile, firstlineno, nogil, goto_error)
 \
  if (nogil) {  
 \
  if (CYTHON_TRACE_NOGIL) { 
 \
  PyThreadState *tstate;
 \
  PyGILState_STATE state = PyGILState_Ensure(); 
 \
  tstate = __Pyx_PyThreadState_Current; 
 \
  if (unlikely(tstate->use_tracing) && !tstate->tracing &&  
 \
  (tstate->c_profilefunc || (CYTHON_TRACE && 
tstate->c_tracefunc))) {\
  __Pyx_use_tracing = __Pyx_TraceSetupAndCall(&$frame_code_cname, 
&$frame_cname, tstate, funcname, srcfile, firstlineno);  \
  } 
 \
  PyGILState_Release(state);
 \
  if (unlikely(__Pyx_use_tracing < 0)) goto_error;  
 \
  } 
 \
  } else {  
 \
  PyThreadState* tstate = PyThreadState_GET();  
 \
  if (unlikely(tstate->use_tracing) && !tstate->tracing &&  
 \
  (tstate->c_profilefunc || (CYTHON_TRACE && tstate->c_tracefunc))) 
{\
  __Pyx_use_tracing = __Pyx_TraceSetupAndCall(&$frame_code_cname, 
&$frame_cname, tstate, funcname, srcfile, firstlineno);  \
  if (unlikely(__Pyx_use_tracing < 0)) goto_error;  
 \
  } 
 \
  }
--

--

___
Python tracker

[issue43760] The DISPATCH() macro is not as efficient as it could be.

2021-05-10 Thread Raymond Hettinger



Raymond Hettinger  added the comment:

I don't think the PEP meant to restrict individual struct member such as this.  
For example, we were able to switch from byte code to word code without 
violating the intended rules.  Consider asking Brett and Benjamin for 
clarification.  I would think that if a new function were introduced to provide 
a reliable way to determine whether tracing was enabled, that would suffice for 
external packages to have a minimally disruptive migration path.

--
nosy: +rhettinger

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43760] The DISPATCH() macro is not as efficient as it could be.

2021-05-10 Thread Mark Shannon



Mark Shannon  added the comment:

But what does "use it" mean?
What does setting `tstate->use_tracing = 1` do?
There is no documented behavior, so how do we know what assumptions people are 
making about what happens when they set some field to 1?

As I said, we could keep the field and ignore it, but that seems worse.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43760] The DISPATCH() macro is not as efficient as it could be.

2021-05-10 Thread Petr Viktorin



Petr Viktorin  added the comment:

PEP 0497 is rejected; the active one is PEP 387, which says "backwards 
incompatibility" means preexisting code ceases to comparatively function after 
a change.
So, this does look like a backwards-incompatible change.

Unfortunately, not all of the C API is documented, so unless it's explicitly 
marked private, people will use it :(

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43760] The DISPATCH() macro is not as efficient as it could be.


Miro Hrončok  added the comment:

scikit-learn: https://bugzilla.redhat.com/show_bug.cgi?id=1958976

gcc: sklearn/cluster/_k_means_fast.c
In file included from 
/usr/lib64/python3.10/site-packages/numpy/core/include/numpy/ndarraytypes.h:1944,
 from 
/usr/lib64/python3.10/site-packages/numpy/core/include/numpy/ndarrayobject.h:12,
 from 
/usr/lib64/python3.10/site-packages/numpy/core/include/numpy/arrayobject.h:4,
 from sklearn/cluster/_k_means_fast.c:635:
/usr/lib64/python3.10/site-packages/numpy/core/include/numpy/npy_1_7_deprecated_api.h:17:2:
 warning: #warning "Using deprecated NumPy API, disable it with " "#define 
NPY_NO_DEPRECATED_API NPY_1_7_API_VERSION" [-Wcpp]
   17 | #warning "Using deprecated NumPy API, disable it with " \
  |  ^~~
sklearn/cluster/_k_means_fast.c: In function ‘__Pyx_call_return_trace_func’:
sklearn/cluster/_k_means_fast.c:1596:15: error: ‘PyThreadState’ {aka ‘struct 
_ts’} has no member named ‘use_tracing’; did you mean ‘tracing’?
 1596 |   tstate->use_tracing = 0;
  |   ^~~
  |   tracing
sklearn/cluster/_k_means_fast.c:1602:15: error: ‘PyThreadState’ {aka ‘struct 
_ts’} has no member named ‘use_tracing’; did you mean ‘tracing’?
 1602 |   tstate->use_tracing = 1;
  |   ^~~
  |   tracing



The usage comes from 
https://github.com/cython/cython/blob/master/Cython/Utility/Profile.c

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43760] The DISPATCH() macro is not as efficient as it could be.


Miro Hrončok  added the comment:

Disclaimer: I have not written the code nor do I understand what is trying to 
achieve. I merely collect the data and report the problems to the package 
maintainers.

It just seems to me that a non-underscored (and hence public) member variable 
on a non-underscored (and hence public) structure should not suddenly go 
missing. Although, I am not familiar with the rules that define what part of 
the API falls under https://www.python.org/dev/peps/pep-0497/

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43760] The DISPATCH() macro is not as efficient as it could be.

2021-05-10 Thread Mark Shannon



Mark Shannon  added the comment:

If there is no C-API function that supports your needs, feel free to suggest 
one.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43760] The DISPATCH() macro is not as efficient as it could be.

2021-05-10 Thread Mark Shannon



Mark Shannon  added the comment:

At yappi/_yappi.c:1261 sets an undocumented field on a CPython internal data 
structure.

What did you believe that was supposed to do? use_tracing is not documented 
anywhere.

We could add the field back and ignore it, but I doubt that would help you much.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43760] The DISPATCH() macro is not as efficient as it could be.


Miro Hrončok  added the comment:

Fedora packages affected (that we know of now):

greenlet: https://bugzilla.redhat.com/show_bug.cgi?id=1957784
dipy: https://bugzilla.redhat.com/show_bug.cgi?id=1958203
yappi: https://bugzilla.redhat.com/show_bug.cgi?id=1958896
smartcols: https://bugzilla.redhat.com/show_bug.cgi?id=1958938

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43760] The DISPATCH() macro is not as efficient as it could be.