msg198487 - (view) |
Author: Stefan Behnel (scoder) *  |
Date: 2013-09-27 16:00 |
In changeset 88b6ef9aa9e9, a new function ported_lib() was added that crashes on error reporting in Py3 because it tries to do this:
raise RuntimeError("Benchmark died: " + err)
"err" is a bytes object that comes straight from the subprocess pipe.
The reason why I noticed this is because the new function assumes that it can run "python" with the "-c" option and pass an arbitrary Python command string. That may not be true if it is actually executing some wrapper script (as we do for Cython, for example). Currently, the only requirement for the external Python command is that it can run a Python script that it receives on the command line. We shouldn't unnecessarily extend that interface.
I'm not sure how to best fix this, but the only way I can currently see is to run an external Python script that prints the requested information.
|
msg198534 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2013-09-28 17:16 |
New changeset 921177a65386 by Antoine Pitrou in branch 'default':
Issue #19108: fix str/bytes mismatch when raising exception under Python 3
http://hg.python.org/benchmarks/rev/921177a65386
|
msg198535 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2013-09-28 17:18 |
Ok, I've fixed the str/bytes mismatch. I don't have any strong feelings about the rest, other than whoever wants perf.py to work with wrapper scripts should probably propose a patch. I'll let Brett decide.
|
msg198536 - (view) |
Author: Stefan Behnel (scoder) *  |
Date: 2013-09-28 17:20 |
Well, it worked before, so the current state is clearly a regression.
|
msg198755 - (view) |
Author: Stefan Behnel (scoder) *  |
Date: 2013-10-01 08:39 |
Antoine, I really don't like this attitude of adding code and then saying "well, it's there, I won't change it" when others complain about breakage. Please undo your change that broke the ability of using (non-trivial) wrapper scripts for the Python runtimes. If you think that that change provides an important feature, then please implement that feature in a way that does not break existing usages of the benchmark runner.
|
msg198760 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2013-10-01 09:12 |
> Antoine, I really don't like this attitude of adding code and then
> saying "well, it's there, I won't change it" when others complain
> about breakage.
Well, I don't care whether you "like this attitude". Your own
attitude has irked several developers enough that they're more or
less ignoring you. That includes me. If you're not willing to
be respectful and constructive, the situation won't improve.
As for the present issue, the benchmark suite is meant to work
with an actual Python interpreter. Not a wrapper script that
can pretend to be one for a single invocation mode.
If you think otherwise, you can of course provide evidence, and
a patch to fix things.
(and hope for someone to apply the patch)
|
msg198761 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2013-10-01 09:13 |
Brett, I'm leaving you with this, if you're wanting to do anything about it :-)
|
msg198762 - (view) |
Author: Stefan Behnel (scoder) *  |
Date: 2013-10-01 09:37 |
> I'm leaving you with this, if you're wanting to do anything about it
Sorry, but weren't you just asking *me* to be constructive?
I'm aware that getting this change right isn't trivial. But that doesn't mean we should happily break other people's code and then leave the cleanup to them. This isn't a green-field project, the code is actually being used by other projects than CPython. In fact, the whole purpose of this code is to be used by other projects as well. That means that there may be some additional restrictions on what we should expect from a benchmarked runtime. Previously, the requirements were "can run a Python script from the command line". Now, the additional requirements are "accepts the -c switch" and "can execute a line of Python code passed on the command line". That is substantially harder to handle in a script.
|
msg198764 - (view) |
Author: Marc-Andre Lemburg (lemburg) *  |
Date: 2013-10-01 10:40 |
Wouldn't it be possible to use the old version as fallback solution
in case the -c switch approach fails or have a command line option
to pass in the version in order to bypass all of this ?
Stefan: Why don't you propose a patch which implements this ?
|
msg198866 - (view) |
Author: Stefan Behnel (scoder) *  |
Date: 2013-10-02 19:59 |
What about this: by default, we assume all runtimes to have the same major version as the Python runtime that executes the benchmark runner. If that's not the case, users must override it explicitly with a command line option, say, "--pyversions 2:3" for a reference 2.x and a comparison 3.x version of Python (i.e. in the order the programs appear on the command line).
Does that sound ok?
I'd like to avoid a trial-and-fallback approach as that would mean that any wrapper scripts would still have to support being called twice and handling the -c option in one way or another.
|
msg198867 - (view) |
Author: Brett Cannon (brett.cannon) *  |
Date: 2013-10-02 20:06 |
If you want to propose a patch to add specifying the version on the command-line to avoid inferring the version I would be fine with that.
|
msg198894 - (view) |
Author: Brett Cannon (brett.cannon) *  |
Date: 2013-10-03 13:39 |
I should mention any solution for the command-line should take a N.N value *only* and not just 2/3 for instances where tests do not work with the latest version of Python yet.
|
msg198898 - (view) |
Author: Stefan Behnel (scoder) *  |
Date: 2013-10-03 14:17 |
I'm having trouble understanding your last comment. Are you saing that you want the exact value to be a two digits version and therefore use separate arguments for both Pythons (e.g. "--basever 2.7 --cmpver 3.3"), or that you want it to accept two digit versions instead of major-only regardless of the number of arguments (e.g. "--pyversion 2.7,3.3")?
And are you saying that you want to use the version also for, say, disabling a test because it doesn't run in Py3.4 yet, despite supporting Py3.3, for example?
A comma is what is used by "--args", BTW, that's why I'd reuse it for the versions argument, i.e. "--pyversion 2.7,3.3" and additionally "--pyversion 3.3" for the simple case.
I haven't come up with a patch yet because it requires a number of non-local changes throughout the file.
|
msg198899 - (view) |
Author: Brett Cannon (brett.cannon) *  |
Date: 2013-10-03 15:16 |
I want to only accept major.minor version specifications; how that is done on the command-line I don't care and leave up to you.
And yes, the version may be used in the future to disable tests that e.g. don't work on Python 3.4 like Chameleon.
|
msg199579 - (view) |
Author: Stefan Behnel (scoder) *  |
Date: 2013-10-12 15:52 |
Here's a patch that replaces the current simplistic Python executable command config with a dedicated PythonRuntime config class. That makes it easy to properly pass around the program specific configuration. Part of that is the Python executable path, the Python version, the specific command line arguments and the relative benchmark library path.
The patch also adds a "--pyversions" command line option to avoid calling the executable for figuring out the Python versions, as discussed.
|
msg207131 - (view) |
Author: Stefan Behnel (scoder) *  |
Date: 2014-01-01 09:23 |
*bump* - patch pending review.
|
msg207138 - (view) |
Author: R. David Murray (r.david.murray) *  |
Date: 2014-01-01 18:54 |
The patch looks good to me, but since I'm not familiar with perf.py I'm not a good person to do a final review and commit it.
One trivial question: why do you check for tupleness in PythonRuntime's init? Don't you control the input on both code paths to obtaining the version?
|
msg273929 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2016-08-30 16:26 |
The new https://github.com/python/performance benchmark suite has a very different design than the old https://hg.python.org/benchmarks benchmark suite:
* it now only runs benchmarks in a virtual environment
* dependencies are installed in the venv by pip
* there is a "run" command which now accepts a --python=PYTHON option to choose the Python binary (and so the tested Python version)
There is no more such thing as ported_lib().
Can I now close this issue as outdated, or do you consider that performance still has this issue?
|
msg274287 - (view) |
Author: Stefan Behnel (scoder) *  |
Date: 2016-09-03 05:35 |
Let's close this as outdated. New bugs for the new project should be reported in github anyway.
|
|
Date |
User |
Action |
Args |
2022-04-11 14:57:51 | admin | set | github: 63307 |
2016-09-03 05:35:34 | scoder | set | status: open -> closed resolution: out of date messages:
+ msg274287
|
2016-08-30 16:26:14 | vstinner | set | nosy:
+ vstinner messages:
+ msg273929
|
2014-01-01 18:54:16 | r.david.murray | set | nosy:
+ r.david.murray messages:
+ msg207138
|
2014-01-01 09:23:56 | scoder | set | messages:
+ msg207131 |
2013-10-12 15:52:54 | scoder | set | files:
+ add_pyversions_option_and_refactor_runtime_config.patch keywords:
+ patch messages:
+ msg199579
|
2013-10-03 15:16:44 | brett.cannon | set | messages:
+ msg198899 |
2013-10-03 14:17:16 | scoder | set | messages:
+ msg198898 |
2013-10-03 13:39:24 | brett.cannon | set | messages:
+ msg198894 |
2013-10-02 20:06:11 | brett.cannon | set | messages:
+ msg198867 stage: needs patch |
2013-10-02 19:59:25 | scoder | set | messages:
+ msg198866 |
2013-10-01 10:40:38 | lemburg | set | nosy:
+ lemburg messages:
+ msg198764
|
2013-10-01 09:37:48 | scoder | set | messages:
+ msg198762 |
2013-10-01 09:13:59 | pitrou | set | nosy:
- pitrou
|
2013-10-01 09:13:54 | pitrou | set | nosy:
brett.cannon, pitrou, scoder, python-dev messages:
+ msg198761 |
2013-10-01 09:12:56 | pitrou | set | messages:
+ msg198760 |
2013-10-01 08:39:30 | scoder | set | messages:
+ msg198755 |
2013-09-28 17:20:50 | scoder | set | messages:
+ msg198536 |
2013-09-28 17:18:21 | pitrou | set | messages:
+ msg198535 |
2013-09-28 17:16:48 | python-dev | set | nosy:
+ python-dev messages:
+ msg198534
|
2013-09-27 16:00:36 | scoder | create | |