homepage

Py_MEMCPY() has a special case for small blocks on Windows to work around an ancient performance issue in MSVC. Can we safely assume that recent MSVC properly optimize memcpy()? See #28055
/* Py_MEMCPY can be used instead of memcpy in cases where the copied blocks
 * are often very short. While most platforms have highly optimized code for
 * large transfers, the setup costs for memcpy are often quite high. MEMCPY
 * solves this by doing short copies "in line".
 */
#if defined(_MSC_VER)
#define Py_MEMCPY(target, source, length) do { \
 size_t i_, n_ = (length); \
 char *t_ = (void*) (target); \
 const char *s_ = (void*) (source); \
 if (n_ >= 16) \
 memcpy(t_, s_, n_); \
 else \
 for (i_ = 0; i_ < n_; i_++) \
 t_[i_] = s_[i_]; \
 } while (0)
#else
#define Py_MEMCPY memcpy
#endif

Yes, memcpy (or preferably memcpy_s, which includes the size of the destination) are basically intrinsics that will inline short copies and call out to a range of implementations depending on alignment/overlap/etc.

Perfect, thanks! Should we keep the Py_MEMCPY() macro for now and define it to memcpy()? Or should I get rid of it and just define it for b/w compatibility?

I'm okay with removing it completely, if you're willing to change that much code. (Most of my additions already uses memcpy or memcpy_s directly, since I wasn't even aware of this macro :) )

Behold the power of Unix! :)
$ find -name '*.[ch]' | xargs sed -i s/Py_MEMCPY/memcpy/g
Victor, are you fine with the change? I'm going to keep Py_MEMCPY around.

The macro looks public, you cannot remove it.
Make it an alias to memcpy(), but explain that it's only kept for backward
compatibility with a reference to this issue.

As I said, I'm going to keep Py_MEMCPY around but replace it everywhere else.
/* Py_MEMCPY is kept for backwards compatibility,
 * see https://bugs.python.org/issue28126 */
#define Py_MEMCPY memcpy

New changeset bedce61ae0a0 by Christian Heimes in branch '3.6':
Issue #28126: Replace Py_MEMCPY with memcpy(). Visual Studio can properly optimize memcpy().
https://hg.python.org/cpython/rev/bedce61ae0a0
New changeset f5d32ed0f9c2 by Christian Heimes in branch 'default':
Issue #28126: Replace Py_MEMCPY with memcpy(). Visual Studio can properly optimize memcpy().
https://hg.python.org/cpython/rev/f5d32ed0f9c2 

Shouldn't the NEWS entity be in the C API section?

Isn't the C API section reserved for C API changes? I haven't changed the C API but rather optimized the core.

The Py_MEMCPY name doesn't start by an underscore, and it is not inside the "#ifndef Py_LIMITED_API" block. But it is not documented either. I don't know what is the status of this macro. If this is a part of public API, this change should be documented in the C API section. If it is inner macro, this change shouldn't be mentioned in Misc/NEWS.

I don't think that it's very useful to discuss the status of the
macro. Documenting the change doesn't harm ;-)

My change doesn't break or change 3rd party application. They might get a tiny bit faster on Windows, if they have used Py_MEMCPY() for up to 16 bytes. That's it.

> They might get a tiny bit faster on Windows, if they have used Py_MEMCPY() for up to 16 bytes
Even that's unlikely as the loop in the macro would have been unrolled in practically every case.
This is about removing an unnecessary macro. Certainly no API change unless we delete it entirely (which I'm okay with), and IMHO if it has an issue number it should go in NEWS. Maybe build is the right section?

Py_MEMCPY() has a special case for small blocks on Windows to work around an ancient performance issue in MSVC. Can we safely assume that recent MSVC properly optimize memcpy()? See #28055
/* Py_MEMCPY can be used instead of memcpy in cases where the copied blocks
 * are often very short. While most platforms have highly optimized code for
 * large transfers, the setup costs for memcpy are often quite high. MEMCPY
 * solves this by doing short copies "in line".
 */
#if defined(_MSC_VER)
#define Py_MEMCPY(target, source, length) do { \
 size_t i_, n_ = (length); \
 char *t_ = (void*) (target); \
 const char *s_ = (void*) (source); \
 if (n_ >= 16) \
 memcpy(t_, s_, n_); \
 else \
 for (i_ = 0; i_ < n_; i_++) \
 t_[i_] = s_[i_]; \
 } while (0)
#else
#define Py_MEMCPY memcpy
#endif