Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Py_LOCAL_INLINE(type) doesn't actually inline except using MSC #49803

Closed
stutzbach mannequin opened this issue Mar 24, 2009 · 9 comments
Closed

Py_LOCAL_INLINE(type) doesn't actually inline except using MSC #49803

stutzbach mannequin opened this issue Mar 24, 2009 · 9 comments
Labels
performance Performance or resource usage

Comments

@stutzbach
Copy link
Mannequin

stutzbach mannequin commented Mar 24, 2009

BPO 5553
Nosy @loewis, @pitrou, @benjaminp, @ned-deily
Files
  • inline.patch
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
closed_at = <Date 2010-08-31.19:52:02.631>
created_at = <Date 2009-03-24.14:49:18.380>
labels = ['performance']
title = "Py_LOCAL_INLINE(type) doesn't actually inline except using MSC"
updated_at = <Date 2010-09-05.04:43:23.365>
user = 'https://bugs.python.org/stutzbach'

    bugs.python.org fields:

    activity = <Date 2010-09-05.04:43:23.365>
actor = 'stutzbach'
assignee = 'stutzbach'
closed = True
closed_date = <Date 2010-08-31.19:52:02.631>
closer = 'stutzbach'
components = []
creation = <Date 2009-03-24.14:49:18.380>
creator = 'stutzbach'
dependencies = []
files = ['17149']
hgrepos = []
issue_num = 5553
keywords = ['patch']
message_count = 9.0
messages = ['84089', '104470', '104476', '104641', '104644', '105940', '115275', '115621', '115625']
nosy_count = 6.0
nosy_names = ['loewis', 'pitrou', 'benjamin.peterson', 'ned.deily', 'stutzbach', 'rpetrov']
pr_nums = []
priority = 'low'
resolution = 'accepted'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'performance'
url = 'https://bugs.python.org/issue5553'
versions = ['Python 3.2']
    @stutzbach
    Copy link
    Mannequin Author

    stutzbach mannequin commented Mar 24, 2009

    Below is the relevant snippet from pyport.h. There are two reasons that
    Py_LOCAL_INLINE doesn't actually emit the "inline" keyword (unless
    compiling with MSC).

    First, "configure" does not have code to test the compiler and define
    USE_INLINE if appropriate.

    Second, the code undefines USE_INLINE even if defined! (oops? ;) )

    The snippet is replicated with slightly different names near the top of
    _sre.c.

    #undef USE_INLINE /* XXX - set via configure? */

    #if defined(_MSC_VER)
#if defined(PY_LOCAL_AGGRESSIVE)
/* enable more aggressive optimization for visual studio */
#pragma optimize("agtw", on)
#endif
/* ignore warnings if the compiler decides not to inline a function */
#pragma warning(disable: 4710)
/* fastest possible local call under MSVC */
#define Py_LOCAL(type) static type __fastcall
#define Py_LOCAL_INLINE(type) static __inline type __fastcall
#elif defined(USE_INLINE)
#define Py_LOCAL(type) static type
#define Py_LOCAL_INLINE(type) static inline type
#else
#define Py_LOCAL(type) static type
#define Py_LOCAL_INLINE(type) static type
#endif

    @stutzbach stutzbach mannequin self-assigned this Apr 27, 2010
    @stutzbach stutzbach mannequin added the performance Performance or resource usage label Apr 27, 2010
    @rpetrov
    Copy link
    Mannequin

    rpetrov mannequin commented Apr 28, 2010

    Configure could call macro to define inline - cf. autoconf manuals.

    @pitrou
    Copy link
    Member

    pitrou commented Apr 29, 2010

    Py_LOCAL_INLINE is also not used a lot. Usually, the compiler will inline small static functions by itself. Most of the time, we used #defines rather than functions when we want to inline short snippets of code.

    @stutzbach
    Copy link
    Mannequin Author

    stutzbach mannequin commented Apr 30, 2010

    Attached is a patch.

    The diff to configure is just what autoconf produces as a result of the diff to configure.in.

    With the patch, pyconfig.h will:

    • do nothing if the compiler supports the "inline" keyword
    • #define inline to __inline or __inline__ if that's the compiler supports
    • #define inline to nothing if the compiler doesn't support inlining

    The patch also updates pyport.h and _sre.c appropriately.

    @stutzbach
    Copy link
    Mannequin Author

    stutzbach mannequin commented Apr 30, 2010

    I should add that the patch is against the py3k branch, since it's too late for performance improvements to land in trunk.

    @pitrou
    Copy link
    Member

    pitrou commented May 17, 2010

    The patch looks ok to me.

    @stutzbach
    Copy link
    Mannequin Author

    stutzbach mannequin commented Aug 31, 2010

    Committed in r84379

    @stutzbach stutzbach mannequin closed this as completed Aug 31, 2010
    @ned-deily
    Copy link
    Member

    Note: autoreconf failure caused by r84379 was fixed by r84512.

    @stutzbach
    Copy link
    Mannequin Author

    stutzbach mannequin commented Sep 5, 2010

    Ned, thanks for bringing that to my attention. I might have missed it otherwise.

    I had run "autoconf" but had not known to run "autoreconf", so I had missed the failure there. *embarrassed*

    Benjamin, thanks for fixing my error!

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    performance Performance or resource usage
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    2 participants
    @pitrou @ned-deily