classification
Title: django_v2 benchmark not working in Python 3.6
Type: behavior Stage: resolved
Components: Benchmarks Versions: Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: brett.cannon Nosy List: berker.peksag, brett.cannon, florin.papa, pitrou, python-dev, r.david.murray
Priority: normal Keywords: patch

Created on 2015-08-25 13:09 by florin.papa, last changed 2015-12-07 07:37 by florin.papa. This issue is now closed.

Files
File name Uploaded Description Edit
django_v2_compat_3_6.patch florin.papa, 2015-08-25 13:09 django_v2 compatibility patch for Python 3.6
django_v3.patch florin.papa, 2015-08-26 07:20
Django-1.8.zip florin.papa, 2015-08-26 07:21
Django-1.8.zip florin.papa, 2015-08-27 08:49
django_v3.patch florin.papa, 2015-09-01 08:02
Django-1.9.zip florin.papa, 2015-09-01 08:03
django_v3_2.patch florin.papa, 2015-12-03 10:17
Django-1.9.tar.gz florin.papa, 2015-12-03 10:17
django_v3_3.patch florin.papa, 2015-12-03 10:27
Messages (22)
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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) Date: 2015-11-04 22:23
An additional argument for waiting is issue 25486.
msg255746 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) 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) (Python triager) 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) * (Python committer) 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!
History
Date User Action Args
2015-12-07 07:37:26florin.papasetmessages: + msg256053
2015-12-04 22:16:52brett.cannonsetstatus: open -> closed
resolution: fixed
messages: + msg255889

stage: commit review -> resolved
2015-12-04 22:16:03python-devsetnosy: + python-dev
messages: + msg255887
2015-12-03 10:27:46florin.papasetfiles: + django_v3_3.patch

messages: + msg255812
2015-12-03 10:17:49florin.papasetfiles: + Django-1.9.tar.gz
2015-12-03 10:17:35florin.papasetfiles: + django_v3_2.patch

messages: + msg255811
2015-12-02 18:21:45brett.cannonsetmessages: + msg255746
2015-11-04 22:23:28r.david.murraysettype: crash -> behavior
messages: + msg254073
2015-09-01 17:16:44brett.cannonsetassignee: brett.cannon
2015-09-01 17:16:39brett.cannonsetmessages: + msg249494
2015-09-01 08:03:21florin.papasetfiles: + Django-1.9.zip
2015-09-01 08:03:00florin.papasetfiles: + django_v3.patch

messages: + msg249466
2015-09-01 07:33:41berker.peksagsetnosy: + berker.peksag
messages: + msg249463
2015-08-31 16:47:54brett.cannonsetmessages: + msg249412
2015-08-31 07:27:31florin.papasetmessages: + msg249386
2015-08-28 19:00:39brett.cannonsetmessages: + msg249289
2015-08-27 08:58:08pitrousetmessages: + msg249226
2015-08-27 08:49:12florin.papasetfiles: + Django-1.8.zip

messages: + msg249225
2015-08-26 16:55:55brett.cannonsetmessages: + msg249209
stage: commit review
2015-08-26 07:21:43florin.papasetfiles: + Django-1.8.zip
2015-08-26 07:20:44florin.papasetfiles: + django_v3.patch

messages: + msg249184
2015-08-25 20:08:47florin.papasetmessages: + msg249158
2015-08-25 18:55:46pitrousetmessages: + msg249153
2015-08-25 17:30:46r.david.murraysetnosy: + r.david.murray
messages: + msg249142

type: crash
stage: patch review -> (no value)
2015-08-25 16:31:02brett.cannonsetnosy: + brett.cannon, pitrou
messages: + msg249135

type: crash -> (no value)
stage: patch review
2015-08-25 13:09:38florin.papacreate