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.

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

Use computed gotos by default #53449

Closed
pitrou opened this issue Jul 8, 2010 · 9 comments
Closed

Use computed gotos by default #53449

pitrou opened this issue Jul 8, 2010 · 9 comments
Assignees
@pitrou
Labels
build The build process and cross-build interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage

Comments

@pitrou
Copy link
Member

pitrou commented Jul 8, 2010

BPO 9203
Nosy @malemburg, @loewis, @pitrou

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 = 'https://github.com/pitrou'
closed_at = <Date 2010-08-13.22:26:56.833>
created_at = <Date 2010-07-08.17:35:03.007>
labels = ['interpreter-core', 'build', 'performance']
title = 'Use computed gotos by default'
updated_at = <Date 2010-08-13.22:26:56.833>
user = 'https://github.com/pitrou'

bugs.python.org fields:

activity = <Date 2010-08-13.22:26:56.833>
actor = 'pitrou'
assignee = 'pitrou'
closed = True
closed_date = <Date 2010-08-13.22:26:56.833>
closer = 'pitrou'
components = ['Build', 'Interpreter Core']
creation = <Date 2010-07-08.17:35:03.007>
creator = 'pitrou'
dependencies = []
files = []
hgrepos = []
issue_num = 9203
keywords = []
message_count = 9.0
messages = ['109558', '109605', '109609', '109612', '109618', '109623', '109633', '109690', '113832']
nosy_count = 4.0
nosy_names = ['lemburg', 'loewis', 'pitrou', 'jyasskin']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'performance'
url = 'https://bugs.python.org/issue9203'
versions = ['Python 3.2']
@pitrou
Copy link
Member Author

pitrou commented Jul 8, 2010

Now that the option has probably been extensively tested, it would be nice to enable computed gotos by default on systems that support them.
Perhaps this needs a dedicated test in the configure script.

@pitrou pitrou added build The build process and cross-build interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage labels Jul 8, 2010
@malemburg
Copy link
Member

Antoine Pitrou wrote:

New submission from Antoine Pitrou <pitrou@free.fr>:

Now that the option has probably been extensively tested, it would be nice to enable computed gotos by default on systems that support them.
Perhaps this needs a dedicated test in the configure script.

I doubt that it has been tested enough, but enabling it
for those platform/compiler where we know it works, should be
fine.

Have you tried testing this using the buildbots ?

@malemburg
Copy link
Member

Marc-Andre Lemburg wrote:

Marc-Andre Lemburg <mal@egenix.com> added the comment:

Antoine Pitrou wrote:
>
> New submission from Antoine Pitrou <pitrou@free.fr>:
>
> Now that the option has probably been extensively tested, it would be nice to enable computed gotos by default on systems that support them.
> Perhaps this needs a dedicated test in the configure script.

I doubt that it has been tested enough, but enabling it
for those platform/compiler where we know it works, should be
fine.

Here's a compiler/platform combination where it should not be used
per default:

gcc 4.4.2 on AMD processors ...
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42621

The fix will go into gcc 4.5, the report doesn't say whether it was
also backported to 4.4.x.

@pitrou
Copy link
Member Author

pitrou commented Jul 8, 2010

gcc 4.4.2 on AMD processors ...
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42621

So? Compilers have all kinds of optimization bugs, it's not our job to
have a compiler matrix with optimal flags per compiler.

(well, you can of course maintain such a matrix if you want)

For the record, I had an Athlon 64 machine until very recently, used it
with gcc 4.4.x and computed gotos were better than without.

@malemburg
Copy link
Member

Antoine Pitrou wrote:

Antoine Pitrou <pitrou@free.fr> added the comment:

> gcc 4.4.2 on AMD processors ...
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42621

So? Compilers have all kinds of optimization bugs, it's not our job to
have a compiler matrix with optimal flags per compiler.

No, but we do need to make sure that the casual user does not
run into such issues by using the default compiler on a typical
Unix system with Python default settings.

An expert user can always use the right compiler... but then an
expert can also use the configure option :-)

gcc 4.4.2 is not exactly an old version of gcc. The perspective
of (apparently) not having the patch in in Ubuntu 10.4 LTS's
default compiler suggests that we do need be more careful about
enabling the switch per default.

(well, you can of course maintain such a matrix if you want)

For the record, I had an Athlon 64 machine until very recently, used it
with gcc 4.4.x and computed gotos were better than without.

Perhaps someone can report whether this affects Ubuntu 10.4 LTS ?!

@pitrou
Copy link
Member Author

pitrou commented Jul 8, 2010

No, but we do need to make sure that the casual user does not
run into such issues by using the default compiler on a typical
Unix system with Python default settings.

The point is that we cannot make it sure. There is nothing specific
about computed gotos here: any kind of compiler bug could threaten
Python performance without us being able to do anything about it. Even
if we restrict ourselves to standard C features.

(also, I'll point out that it's not demonstrated that the problem above
affects Python, rather than simply the particular piece of code shown in
the bugzilla report)

@loewis
Copy link
Mannequin

loewis mannequin commented Jul 8, 2010

No, but we do need to make sure that the casual user does not
run into such issues by using the default compiler on a typical
Unix system with Python default settings.

I think there is no risk here. The casual Python user on an AMD64 system
where gcc is the default compiler will use the Python installation that
comes with the operating system.

@malemburg
Copy link
Member

Martin v. Löwis wrote:

Martin v. Löwis <martin@v.loewis.de> added the comment:

> No, but we do need to make sure that the casual user does not
> run into such issues by using the default compiler on a typical
> Unix system with Python default settings.

I think there is no risk here. The casual Python user on an AMD64 system
where gcc is the default compiler will use the Python installation that
comes with the operating system.

I'd be fine with enabling the switch on those platforms as well,
if we could get feedback from someone testing the behavior of gcc
on an AMD platform running Ubuntu 10.4.

If the problem is really only with the code snippet listed in the
bug report, there shouldn't be a problem and we can go ahead with
enabling computed gotos on that platform/compiler combination as well.

@pitrou
Copy link
Member Author

pitrou commented Aug 13, 2010

I've committed a patch in r83986. I will watch the buildbots and close if everything's fine.

@pitrou pitrou self-assigned this Aug 13, 2010
@pitrou pitrou closed this as completed Aug 13, 2010
@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

@pitrou pitrou

Labels
build The build process and cross-build interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@pitrou @malemburg