msg249119 - (view) |
Author: Florin Papa (florin.papa) * |
Date: 2015-08-25 13:09 |
Hi All,
My name is Florin Papa and I work in the Server Languages Optimizations Team at Intel Corporation.
I would like to submit a patch that solves compatibility issues of the django_v2 benchmark in the Grand Unified Python Benchmark. The django_v2 benchmark uses inspect.getargspec(), which is deprecated and was removed in Python 3.6. Therefore, it crashes with the message "ImportError: cannot import name 'getargspec'" when using the latest version of Python on the default branch.
The patch modifies the benchmark to use inspect.signature() when Python version is 3.6 or above and keep using inspect.getargspec() otherwise.
Regards,
Florin
|
msg249135 - (view) |
Author: Brett Cannon (brett.cannon) *  |
Date: 2015-08-25 16:31 |
The proposed patch directly modifies the copy of Django in the benchmark suite which isn't acceptable; we purposefully don't modify the project code we pull in for benchmark consistency and ease of maintenance.
It would be better to make sure a newer version of Django is fixed upstream, pull in the newer version to the benchmark suite, and then create a django_v3 benchmark using that newer version while removing the django_v2 benchmark from the default set of benchmarks (if not fully remove it).
And can you sign the contributor agreement, Florin?
|
msg249142 - (view) |
Author: R. David Murray (r.david.murray) *  |
Date: 2015-08-25 17:30 |
Florin is covered by the Intel corporate agreement. I've updated his user record accordingly.
|
msg249153 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2015-08-25 18:55 |
According to the docs, getargspec() should be replaced with getfullargspec(), which returns a compatible named tuple. This would probably also minimize the performance impact of the patch (I'm not sure getargspec() is in the critical path, but let's be careful).
|
msg249158 - (view) |
Author: Florin Papa (florin.papa) * |
Date: 2015-08-25 20:08 |
Thank you all for your feedback on this matter.
According to the docs, starting from version 3.4, getfullargspec() is based on signature(), therefore the performance impact would not differ from the proposed implementation.
I will check if a newer version of django fixes this problem and modify the patch accordingly.
|
msg249184 - (view) |
Author: Florin Papa (florin.papa) * |
Date: 2015-08-26 07:20 |
I have modified the patch to introduce django_v3 to the benchmark suite and deprecate django_v2 for Python 3.6 and up. In order for django_v3 to work, the latest django release must be present in lib/Django-1.8.
Django-1.8 is attached as a zip file because the patch would be too large if it included all these files. In order for the modifications to work, extract the archive to lib/Django-1.8 .
Thank you,
Florin
|
msg249209 - (view) |
Author: Brett Cannon (brett.cannon) *  |
Date: 2015-08-26 16:55 |
LGTM on a cursory glance. Hopefully someone will have time to try the patch out and then commit it if everything works out.
|
msg249225 - (view) |
Author: Florin Papa (florin.papa) * |
Date: 2015-08-27 08:49 |
I have modified the Django-1.8.zip archive to extract to Django-1.8 directory.
To apply the patch please follow these steps:
hg clone https://hg.python.org/benchmarks
cd benchmarks/
copy django_v3.patch to the current directory and Django-1.8.zip to the lib/ directory
hg import --no-commit django_v3.patch
cd lib/
unzip Django-1.8.zip
|
msg249226 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2015-08-27 08:58 |
Both the fix and the v3 benchmark sound good to me. Note I haven't tried them.
|
msg249289 - (view) |
Author: Brett Cannon (brett.cannon) *  |
Date: 2015-08-28 19:00 |
Changeset 2f3b3c6c1190 adds the version range check to django_v2 to block it past 3.5.
I didn't add the django_v3 quite yet because I didn't realize that Django 1.8 won't work without being manually patched by us (I thought that issue was limited to our Django 1.5 copy). I did check and the issue is fixed in Django 1.9 which is due before the end of the year: https://code.djangoproject.com/wiki/Version1.9Roadmap .
Since this is for Python 3.6 which is in no way close to coming out I say we simply wait until Django 1.9.0 is released and then introduce django_v3 without having to keep our own patched version of Django around.
|
msg249386 - (view) |
Author: Florin Papa (florin.papa) * |
Date: 2015-08-31 07:27 |
The Django 1.8 archive that I attached is not patched, it is downloaded from the django github page without modifications. I have tested and Django 1.8 works with the latest Python version pulled from hg.python.org/cpython.
|
msg249412 - (view) |
Author: Brett Cannon (brett.cannon) *  |
Date: 2015-08-31 16:47 |
Hmm, strange. I downloaded Django 1.8.latest myself and placed it and I still got the getargspec() failure using the default branch. I'll try again and verify that it wasn't something odd on my end.
|
msg249463 - (view) |
Author: Berker Peksag (berker.peksag) *  |
Date: 2015-09-01 07:33 |
I just downloaded Django-1.8.zip (florin.papa, 2015-08-27 11:49) and opened Django-1.8/django/__init__.py to check the Django version:
VERSION = (1, 9, 0, 'alpha', 0)
I think GitHub lets you download the master branch by default. I'd suggest to download the source release of Django 1.8.4 at https://pypi.python.org/pypi/Django/1.8.4
|
msg249466 - (view) |
Author: Florin Papa (florin.papa) * |
Date: 2015-09-01 08:02 |
Please download Django from their github website: https://github.com/django/django
I checked and their latest stable release - Django 1.8.4 - from https://pypi.python.org/pypi/Django still has the getargspec issue.
Apparently the github version is 1.9.0 alpha. I have modified the patch and archive accordingly.
|
msg249494 - (view) |
Author: Brett Cannon (brett.cannon) *  |
Date: 2015-09-01 17:16 |
I would still rather wait until Django 1.9.0 is officially released else we are benchmarking alpha code which doesn't seem worth it. With Python 3.6.0 not due out until about 15 months after 1.9.0 comes out I think going 3 months without a Django benchmark for 3.6 is acceptable.
|
msg254073 - (view) |
Author: R. David Murray (r.david.murray) *  |
Date: 2015-11-04 22:23 |
An additional argument for waiting is issue 25486.
|
msg255746 - (view) |
Author: Brett Cannon (brett.cannon) *  |
Date: 2015-12-02 18:21 |
Django 1.9 is out, so when I have time I will create a django_v3 benchmark.
|
msg255811 - (view) |
Author: Florin Papa (florin.papa) * |
Date: 2015-12-03 10:17 |
I have adapted the existent patch to the new benchmarks codebase. Please see the new patch (django_v3_2.patch) in the files section. The benchmark expects to have django in lib/Django-1.9 folder. The Django-1.9 sources are attached separately because of their size.
hg clone https://hg.python.org/benchmarks
cd benchmarks/
copy django_v3_2.patch to the current directory and Django-1.9.tar.gz to the lib/ directory
hg import --no-commit django_v3_2.patch
cd lib/
tar -xvf Django-1.9.tar.gz
|
msg255812 - (view) |
Author: Florin Papa (florin.papa) * |
Date: 2015-12-03 10:27 |
I have attached django_v3_3.patch, as the previous one did not include the added file (performance/bm_django_v3.py).
|
msg255887 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2015-12-04 22:16 |
New changeset 8a9b86071c15 by Brett Cannon in branch 'default':
Issue #24934: Introduce the django_v2 benchmark using Django 1.9.0.
https://hg.python.org/benchmarks/rev/8a9b86071c15
|
msg255889 - (view) |
Author: Brett Cannon (brett.cannon) *  |
Date: 2015-12-04 22:16 |
Thanks for the patch, Florin! Only changes I made was to deprecate django_v2 and to set the minimum Python version to 2.7.
|
msg256053 - (view) |
Author: Florin Papa (florin.papa) * |
Date: 2015-12-07 07:37 |
No problem. Thank you for merging the patch!
|
|
Date |
User |
Action |
Args |
2022-04-11 14:58:20 | admin | set | github: 69122 |
2015-12-07 07:37:26 | florin.papa | set | messages:
+ msg256053 |
2015-12-04 22:16:52 | brett.cannon | set | status: open -> closed resolution: fixed messages:
+ msg255889
stage: commit review -> resolved |
2015-12-04 22:16:03 | python-dev | set | nosy:
+ python-dev messages:
+ msg255887
|
2015-12-03 10:27:46 | florin.papa | set | files:
+ django_v3_3.patch
messages:
+ msg255812 |
2015-12-03 10:17:49 | florin.papa | set | files:
+ Django-1.9.tar.gz |
2015-12-03 10:17:35 | florin.papa | set | files:
+ django_v3_2.patch
messages:
+ msg255811 |
2015-12-02 18:21:45 | brett.cannon | set | messages:
+ msg255746 |
2015-11-04 22:23:28 | r.david.murray | set | type: crash -> behavior messages:
+ msg254073 |
2015-09-01 17:16:44 | brett.cannon | set | assignee: brett.cannon |
2015-09-01 17:16:39 | brett.cannon | set | messages:
+ msg249494 |
2015-09-01 08:03:21 | florin.papa | set | files:
+ Django-1.9.zip |
2015-09-01 08:03:00 | florin.papa | set | files:
+ django_v3.patch
messages:
+ msg249466 |
2015-09-01 07:33:41 | berker.peksag | set | nosy:
+ berker.peksag messages:
+ msg249463
|
2015-08-31 16:47:54 | brett.cannon | set | messages:
+ msg249412 |
2015-08-31 07:27:31 | florin.papa | set | messages:
+ msg249386 |
2015-08-28 19:00:39 | brett.cannon | set | messages:
+ msg249289 |
2015-08-27 08:58:08 | pitrou | set | messages:
+ msg249226 |
2015-08-27 08:49:12 | florin.papa | set | files:
+ Django-1.8.zip
messages:
+ msg249225 |
2015-08-26 16:55:55 | brett.cannon | set | messages:
+ msg249209 stage: commit review |
2015-08-26 07:21:43 | florin.papa | set | files:
+ Django-1.8.zip |
2015-08-26 07:20:44 | florin.papa | set | files:
+ django_v3.patch
messages:
+ msg249184 |
2015-08-25 20:08:47 | florin.papa | set | messages:
+ msg249158 |
2015-08-25 18:55:46 | pitrou | set | messages:
+ msg249153 |
2015-08-25 17:30:46 | r.david.murray | set | nosy:
+ r.david.murray messages:
+ msg249142
type: crash stage: patch review -> (no value) |
2015-08-25 16:31:02 | brett.cannon | set | nosy:
+ brett.cannon, pitrou messages:
+ msg249135
type: crash -> (no value) stage: patch review |
2015-08-25 13:09:38 | florin.papa | create | |