Re: [Python-Dev] General concerns about C API changes

2018-11-23 Thread Victor Stinner
Le dim. 18 nov. 2018 à 17:54, Stefan Behnel  a écrit :
> It's also slower to compile, given that function inlining happens at a much
> later point in the compiler pipeline than macro expansion. The C compiler
> won't even get to see macros in fact, whereas whether to inline a function
> or not is a dedicated decision during the optimisation phase based on
> metrics collected in earlier stages. For something as ubiquitous as
> Py_INCREF/Py_DECREF, it might even be visible in the compilation times.

I ran a benchmark: there is no significant slowdown (+4 seconds, 6%
slower, in the worst case).
https://bugs.python.org/issue35059#msg330316


> Now imagine that you have an inline function that executes several
> Py_INCREF/Py_DECREF call cycles, and the C compiler happens to slightly
> overestimate the weights of these two. Then it might end up deciding
> against inlining the function now, whereas it previously might have decided
> for it since it was able to see the exact source code expanded from the
> macros. I think that's what Raymond meant with his concerns regarding
> changing macros into inline functions. C compilers might be smart enough to
> always inline CPython's new inline functions themselves, but the style
> change can still have unexpected transitive impacts on code that uses them.

I ran the performance benchmark suite to compare C macros to static
inline functions: there is no significant impact on performance.
https://bugs.python.org/issue35059#msg330302


> I agree with Raymond that as long as there is no clear gain in this code
> churn, we should not underestimate the risk of degarding code on user side.

I don't understand how what you mean with "degarding code on user
side". If you are talking about performance, again, my changes have no
significant impact on performance (not on compilation time nor runtime
performance).


> "there is no clear gain in this code churn"

There are multiple advantages:

* Better development and debugging experience: tools understand
inlined functions much better than C macros: gdb, Linux perf, etc.

* Better API: arguments now have a type and the function has a return
type. In practice, some macros still cast their argument to PyObject*
to not introduce new compiler warnings in Python 3.8. For example,
even if Py_INCREF() is documented (*) as a function expecting
PyObject*, it accepts any pointer type (PyTupleObject*,
PyUnicodeObject*, etc.). Technically, it also accepts PyObject** which
is a bug, but that's a different story ;-)

* Much better code, just plain regular C. C macros are ugly: "do { ...
} while (0)" workaround, additional parenthesis around each argument,
strange "expr1, expr2" syntax of "macro expression" which returns a
value (inline function just uses regular "return" and ";" at the end
of instructions), strange indentation, etc.

* No more "macro pitfals":
https://gcc.gnu.org/onlinedocs/cpp/Macro-Pitfalls.html

* Local variables no longer need a magic name to avoid risk of name
conflict, and have a clearly defined scope. Py_DECREF() and
_Py_XINCREF() no longer need a local variable since it's argument
already has a clearly defined type: PyObject*. I introduced a new
variable in _Py_Dealloc() to fix a possible race condition.
Previously, the variable was probably avoided because it's tricky use
variables in macros.

* #ifdef can now be used inside the inline function: it makes the code
easier to understand.

* etc.


Are you aware that Python had macros like:

#define _Py_REF_DEBUG_COMMA ,
#define _Py_CHECK_REFCNT(OP) /* a semicolon */;

I let you judge the quality of this macro:

#define _Py_NewReference(op) (  \
_Py_INC_TPALLOCS(op) _Py_COUNT_ALLOCS_COMMA \
_Py_INC_REFTOTAL  _Py_REF_DEBUG_COMMA   \
Py_REFCNT(op) = 1)

Is it an expression? Can it be used in "if (test)
_Py_NewReference(op);"? It doesn't use the "do { ... } while (0)"
protection against macro pitfals.

(*) Py_INCREF doc:
https://docs.python.org/dev/c-api/refcounting.html#c.Py_INCREF

Victor
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


Re: [Python-Dev] General concerns about C API changes

2018-11-19 Thread Antoine Pitrou
On Sun, 18 Nov 2018 13:53:54 -0800
Nathaniel Smith  wrote:
> On Sun, Nov 18, 2018 at 8:52 AM Stefan Behnel  wrote:
> >
> > Gregory P. Smith schrieb am 15.11.18 um 01:03:  
> > > From my point of view: A static inline function is a much nicer modern 
> > > code
> > > style than a C preprocessor macro.  
> >
> > It's also slower to compile, given that function inlining happens at a much
> > later point in the compiler pipeline than macro expansion. The C compiler
> > won't even get to see macros in fact, whereas whether to inline a function
> > or not is a dedicated decision during the optimisation phase based on
> > metrics collected in earlier stages. For something as ubiquitous as
> > Py_INCREF/Py_DECREF, it might even be visible in the compilation times.  
> 
> Have you measured this? I had the opposite intuition, that macros on
> average will be slower to compile because they increase the amount of
> code that the frontend has to process. But I've never checked...

It will certainly depend on how much code the macro expands to.
Py_INCREF is an extremely simple macro, so expanding everywhere doesn't
sound like a problem.

On the other hand, modern "macros" that are C++ templates can inline
vast amounts of code at the call site, and that's a common cause of
slow C++ compiles.

Regards

Antoine.


___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


Re: [Python-Dev] General concerns about C API changes

2018-11-18 Thread Nathaniel Smith
On Sun, Nov 18, 2018 at 8:52 AM Stefan Behnel  wrote:
>
> Gregory P. Smith schrieb am 15.11.18 um 01:03:
> > From my point of view: A static inline function is a much nicer modern code
> > style than a C preprocessor macro.
>
> It's also slower to compile, given that function inlining happens at a much
> later point in the compiler pipeline than macro expansion. The C compiler
> won't even get to see macros in fact, whereas whether to inline a function
> or not is a dedicated decision during the optimisation phase based on
> metrics collected in earlier stages. For something as ubiquitous as
> Py_INCREF/Py_DECREF, it might even be visible in the compilation times.

Have you measured this? I had the opposite intuition, that macros on
average will be slower to compile because they increase the amount of
code that the frontend has to process. But I've never checked...

-n

-- 
Nathaniel J. Smith -- https://vorpus.org
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


Re: [Python-Dev] General concerns about C API changes

2018-11-18 Thread Stefan Behnel
Gregory P. Smith schrieb am 15.11.18 um 01:03:
> From my point of view: A static inline function is a much nicer modern code
> style than a C preprocessor macro.

It's also slower to compile, given that function inlining happens at a much
later point in the compiler pipeline than macro expansion. The C compiler
won't even get to see macros in fact, whereas whether to inline a function
or not is a dedicated decision during the optimisation phase based on
metrics collected in earlier stages. For something as ubiquitous as
Py_INCREF/Py_DECREF, it might even be visible in the compilation times.

Oh, BTW, I don't know if this was mentioned in the discussion before, but
transitive inlining can easily be impacted by the switch from a macro to an
inline function. Since inlining happens long before the final CPU code
generation, the C compiler needs to uses heuristics for estimating the
eventual "code weight" of an inline function, and then sums up all weights
within a calling function to decide whether to also inline that calling
function into the transitive callers or not.

Now imagine that you have an inline function that executes several
Py_INCREF/Py_DECREF call cycles, and the C compiler happens to slightly
overestimate the weights of these two. Then it might end up deciding
against inlining the function now, whereas it previously might have decided
for it since it was able to see the exact source code expanded from the
macros. I think that's what Raymond meant with his concerns regarding
changing macros into inline functions. C compilers might be smart enough to
always inline CPython's new inline functions themselves, but the style
change can still have unexpected transitive impacts on code that uses them.

I agree with Raymond that as long as there is no clear gain in this code
churn, we should not underestimate the risk of degarding code on user side.

Stefan

___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


Re: [Python-Dev] General concerns about C API changes

2018-11-14 Thread Victor Stinner
Le jeu. 15 nov. 2018 à 01:06, Gregory P. Smith  a écrit :
> I expect the largest visible impact may be that a profiler may now attribute 
> CPU cycles takes by these code snippets to the function from the .h file 
> rather than directly to the functions the macro expanded in in the past due 
> to additional debug symbol info attribution. Just more data. Consider that a 
> win.

Oh. That's very interesting.

I just tried gdb and I confirm that gdb understands well inlined
function. When I debug Python, gdb moves into Py_INCREF() or
Py_DECREF() when I use "next".

I also tried perf record/perf report: if I annotate a function
(assembler code of the function), perf shows me the C code of inlined
Py_INCREF and Py_DECREF!

That's nice!

Victor
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


Re: [Python-Dev] General concerns about C API changes

2018-11-14 Thread Gregory P. Smith
On Tue, Nov 13, 2018 at 7:06 PM Raymond Hettinger <
raymond.hettin...@gmail.com> wrote:

> Overall, I support the efforts to improve the C API, but over the last few
> weeks have become worried.  I don't want to hold up progress with fear,
> uncertainty, and doubt.  Yet, I would like to be more comfortable that
> we're all aware of what is occurring and what are the potential benefits
> and risks.
>
> * Inline functions are great.  They provide true local variables, better
> separation of concerns, are far less kludgy than text based macro
> substitution, and will typically generate the same code as the equivalent
> macro.  This is good tech when used with in a single source file where it
> has predictable results.
>
> However, I'm not at all confident about moving these into header files
> which are included in multiple target .c files which need be compiled into
> separate .o files and linked to other existing libraries.
>
> With a macro, I know for sure that the substitution is taking place.  This
> happens at all levels of optimization and in a debug mode.  The effects are
> 100% predictable and have a well-established track record in our mature
> battle-tested code base.  With cross module function calls, I'm less
> confident about what is happening, partly because compilers are free to
> ignore inline directives and partly because the semantics of inlining are
> less clear when the crossing module boundaries.
>
> * Other categories of changes that we make tend to have only a shallow
> reach.  However, these C API changes will likely touch every C extension
> that has ever been written, some of which is highly tuned but not actively
> re-examined.  If any mistakes are make, they will likely be pervasive.
> Accordingly, caution is warranted.
>
> My expectation was that the changes would be conducted in experimental
> branches. But extensive changes are already being made (or about to be
> made) on the 3.8 master. If a year from now, we decide that the changes
> were destabilizing or that the promised benefits didn't materialize, they
> will be difficult to undo because there are so many of them and because
> they will be interleaved with other changes.
>
> The original motivation was to achieve a 2x speedup in return for
> significantly churning the C API. However, the current rearranging of the
> include files and macro-to-inline-function changes only give us churn.  At
> the very best, they will be performance neutral.  At worst, formerly cheap
> macro calls will become expensive in places that we haven't thought to run
> timings on.  Given that compilers don't have to honor an inline directive,
> we can't really know for sure -- perhaps today it works out fine, and
> perhaps tomorrow the compilers opt for a different behavior.
>
> Maybe everything that is going on is fine.  Maybe it's not. I am not
> expert enough to know for sure, but we should be careful before
> green-lighting such an extensive series of changes directly to master.
> Reasonable questions to ask are: 1) What are the risks to third party
> modules, 2) Do we really know that the macro-to-inline-function
> transformations are semantically neutral. 3) If there is no performance
> benefit (none has been seen so far, nor is any promised in the pending
> PRs), is it worth it?
>
> We do know that PyPy folks have had their share of issues with the C API,
> but I'm not sure that we can make any of this go away without changing the
> foundations of the whole ecosystem.  It is inconvenient for a full GC
> environment to interact with the API for a reference counted environment --
> I don't think we can make this challenge go away without giving up
> reference counting.  It is inconvenient for a system that manifests objects
> on demand to interact with an API that assumes that objects have identity
> and never more once they are created -- I don't think we can make this go
> away either.  It is inconvenient to a system that uses unboxed data to
> interact with our API where everything is an object that includes a type
> pointer and reference count -- We have provided an API for boxing and
> boxing, but the trip back-and-forth is inconveniently expensive -- I don't
> think we can make that go away either because too much of the ecosystem
> depends on that API.  There are some things that ca
>  n be mitigated such as challenges with borrowed references but that
> doesn't seem to have been the focus on any of the PRs.
>
> In short, I'm somewhat concerned about the extensive changes that are
> occurring.  I do know they will touch substantially every C module in the
> entire ecosystem.  I don't know whether they are safe or whether they will
> give any real benefit.
>
> FWIW, none of this is a criticism of the work being done.  Someone needs
> to think deeply about the C API or else progress will never be made.  That
> said, it is a high risk project with many PRs going directly into master,
> so it does warrant having buy in that the churn 

Re: [Python-Dev] General concerns about C API changes

2018-11-14 Thread Jeroen Demeyer

On 2018-11-14 04:06, Raymond Hettinger wrote:

With cross module function calls, I'm less confident about what is happening


If the functions are "static inline" (as opposed to plain "inline"), 
those aren't really cross-module function calls. Because the functions 
are "static" and defined in a header file, every module has its own copy 
of the function.


If the function is not inlined in the end, this would inflate the 
compiled size because you end up with multiple compilations of the same 
code in the CPython library. It would not affect correct functioning in 
any way though. If the function *is* inlined, then the result should be 
no different from using a macro.



Jeroen.
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


Re: [Python-Dev] General concerns about C API changes

2018-11-14 Thread Victor Stinner
> First, I attempted for "force inlining" (ex:
> __attribute__((always_inline)) for GCC/Clang), but it has been decided
> to not do that. Please read the discussion on the issue for the
> rationale.
>
> https://bugs.python.org/issue35059

It has been decided to use "static inline" syntax since it's the one
required by the PEP 7, but I'm open to switch to "inline" (without
static) or something else. Honestly, I don't understand exactly all
consequences of the exact syntax, so I relied on other core devs who
understand C99 that better than me :-)

Join the discussion on the issue ;-)

Victor
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


Re: [Python-Dev] General concerns about C API changes

2018-11-14 Thread Victor Stinner
Hi,

I made many different kinds of changes to the C API last weeks. None
of them should impact the backward compatibility. If it's the case,
the changes causing the backward compatibility should be reverted. The
most controversial changes are the conversion of macros to static
inline functions, but these changes have been discussed in length and
approved by multiple core developers.


(*) Move private internal API outside Include/: continue the work
started in Python 3.7 to move it to Include/internal/

This work is almost complete. The remaining issue is the
_PyObject_GC_TRACK() function.

The main change is that an explicit #include "pycore_.h" is
now required in C code.

https://bugs.python.org/issue35081
https://mail.python.org/pipermail/python-dev/2018-October/155587.html
https://mail.python.org/pipermail/python-dev/2018-November/155688.html


(*) Move "unstable" API outside Include/: move "#ifndef
Py_LIMITED_API" code to a new subdirectory

This work didn't start. It's still being discussed to see how we
should do it. I chose to try to finish Include/internal/ first.

https://bugs.python.org/issue35134


(*) Add a *new* C API which doesn't leak implementation details

This work didn't start. It's still under discussion, I'm not sure that
it's going to happen in the master branch in the short term.

https://pythoncapi.readthedocs.io/
https://bugs.python.org/issue35206
https://mail.python.org/pipermail/python-dev/2018-November/155702.html


(*) Convert macros to static inline functions

I guess that Raymond is worried by the impact on performance of these changes.

So far, the following macros have been converted to static inline functions:

* PyObject_INIT(), PyObject_INIT_VAR()
* _Py_NewReference(), _Py_ForgetReference()
* Py_INCREF(), Py_DECREF()
* Py_XINCREF(), Py_XDECREF()
* _Py_Dealloc()

First, I attempted for "force inlining" (ex:
__attribute__((always_inline)) for GCC/Clang), but it has been decided
to not do that. Please read the discussion on the issue for the
rationale.

https://bugs.python.org/issue35059

I modified the Visual Studio project to ask the compiler to respect
"inline" *hint* when CPython is compiled in debug mode: "Set
InlineFunctionExpansion to OnlyExplicitInline ("/Ob1" option) on all
projects (in pyproject.props) in Debug mode on Win32 and x64 platforms
to expand functions marked as inline." This change spotted a bug in
_decimal ("Add missing EXTINLINE in mpdecimal.h").

Macros have many pitfalls, and the intent here is to prevent these pitfalls.:
https://gcc.gnu.org/onlinedocs/cpp/Macro-Pitfalls.html


_Py_Dealloc() example:

#ifdef COUNT_ALLOCS
#define _Py_INC_TPFREES(OP) dec_count(Py_TYPE(OP))
#define _Py_COUNT_ALLOCS_COMMA  ,
#else
#define _Py_INC_TPFREES(OP)
#define _Py_COUNT_ALLOCS_COMMA
#endif /* COUNT_ALLOCS */

#define _Py_Dealloc(op) (   \
_Py_INC_TPFREES(op) _Py_COUNT_ALLOCS_COMMA  \
(*Py_TYPE(op)->tp_dealloc)((PyObject *)(op)))

_Py_Dealloc() produced something like "dealloc()" or "dec_count(),
dealloc()" depending on compiler options on Python 3.7. On Python 3.8,
it's now a well defined function call which returns "void" (no
result): "[static inline] void _Py_Dealloc(PyObject *);"


Another example:

#define Py_DECREF(op)   \
do {\
PyObject *_py_decref_tmp = (PyObject *)(op);\
if (_Py_DEC_REFTOTAL  _Py_REF_DEBUG_COMMA   \
--(_py_decref_tmp)->ob_refcnt != 0) \
_Py_CHECK_REFCNT(_py_decref_tmp)\
else\
_Py_Dealloc(_py_decref_tmp);\
} while (0)

This macro required a temporary "_py_decref_tmp" variable in Python
3.7, to cast the argument to PyObject*. It's gone in Python 3.8:

static inline void _Py_DECREF(const char *filename, int lineno,
  PyObject *op)
{
_Py_DEC_REFTOTAL;
if (--op->ob_refcnt != 0) {
#ifdef Py_REF_DEBUG
if (op->ob_refcnt < 0) {
_Py_NegativeRefcount(filename, lineno, op);
}
#endif
}
else {
_Py_Dealloc(op);
}
}

#define Py_DECREF(op) _Py_DECREF(__FILE__, __LINE__, (PyObject *)(op))

The cast is now done in Py_DECREF() macro.

The Py_DECREF() contract is that it accepts basically any pointer. I
chose to not change that (always require the exact PyObject* type and
nothing else), since it would create a compiler warning on basically
every usage of Py_DECREF() for no real benefit.

Victor
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


Re: [Python-Dev] General concerns about C API changes

2018-11-13 Thread Nathaniel Smith
To me, the "new C API" discussion and the "converting macros into
inline functions" discussions are very different, almost unrelated.
There are always lots of small C API changes happening, and AFAIK the
macros->inline changes fall into that category. It sounds like you
want to discuss whether inline functions are a good idea? Or are there
other changes happening that you're worried about? Or is there some
connection between inline functions and API breakage that I'm not
aware of? Your email touches on a lot of different topics and I'm
having trouble understanding how they fit together. (And I guess like
most people here, I'm not watching every commit to the master branch
so I may not even know what changes you're referring to.)

On Tue, Nov 13, 2018 at 7:06 PM, Raymond Hettinger
 wrote:
> Overall, I support the efforts to improve the C API, but over the last few 
> weeks have become worried.  I don't want to hold up progress with fear, 
> uncertainty, and doubt.  Yet, I would like to be more comfortable that we're 
> all aware of what is occurring and what are the potential benefits and risks.
>
> * Inline functions are great.  They provide true local variables, better 
> separation of concerns, are far less kludgy than text based macro 
> substitution, and will typically generate the same code as the equivalent 
> macro.  This is good tech when used with in a single source file where it has 
> predictable results.
>
> However, I'm not at all confident about moving these into header files which 
> are included in multiple target .c files which need be compiled into separate 
> .o files and linked to other existing libraries.
>
> With a macro, I know for sure that the substitution is taking place.  This 
> happens at all levels of optimization and in a debug mode.  The effects are 
> 100% predictable and have a well-established track record in our mature 
> battle-tested code base.  With cross module function calls, I'm less 
> confident about what is happening, partly because compilers are free to 
> ignore inline directives and partly because the semantics of inlining are 
> less clear when the crossing module boundaries.
>
> * Other categories of changes that we make tend to have only a shallow reach. 
>  However, these C API changes will likely touch every C extension that has 
> ever been written, some of which is highly tuned but not actively 
> re-examined.  If any mistakes are make, they will likely be pervasive.  
> Accordingly, caution is warranted.
>
> My expectation was that the changes would be conducted in experimental 
> branches. But extensive changes are already being made (or about to be made) 
> on the 3.8 master. If a year from now, we decide that the changes were 
> destabilizing or that the promised benefits didn't materialize, they will be 
> difficult to undo because there are so many of them and because they will be 
> interleaved with other changes.
>
> The original motivation was to achieve a 2x speedup in return for 
> significantly churning the C API. However, the current rearranging of the 
> include files and macro-to-inline-function changes only give us churn.  At 
> the very best, they will be performance neutral.  At worst, formerly cheap 
> macro calls will become expensive in places that we haven't thought to run 
> timings on.  Given that compilers don't have to honor an inline directive, we 
> can't really know for sure -- perhaps today it works out fine, and perhaps 
> tomorrow the compilers opt for a different behavior.
>
> Maybe everything that is going on is fine.  Maybe it's not. I am not expert 
> enough to know for sure, but we should be careful before green-lighting such 
> an extensive series of changes directly to master.  Reasonable questions to 
> ask are: 1) What are the risks to third party modules, 2) Do we really know 
> that the macro-to-inline-function transformations are semantically neutral. 
> 3) If there is no performance benefit (none has been seen so far, nor is any 
> promised in the pending PRs), is it worth it?
>
> We do know that PyPy folks have had their share of issues with the C API, but 
> I'm not sure that we can make any of this go away without changing the 
> foundations of the whole ecosystem.  It is inconvenient for a full GC 
> environment to interact with the API for a reference counted environment -- I 
> don't think we can make this challenge go away without giving up reference 
> counting.  It is inconvenient for a system that manifests objects on demand 
> to interact with an API that assumes that objects have identity and never 
> more once they are created -- I don't think we can make this go away either.  
> It is inconvenient to a system that uses unboxed data to interact with our 
> API where everything is an object that includes a type pointer and reference 
> count -- We have provided an API for boxing and boxing, but the trip 
> back-and-forth is inconveniently expensive -- I don't think we can make that 
> go away