classification
Title: PEG Parser benchmarks fail if memory_profiler is not installed
Type: Stage: resolved
Components: Versions:
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: lys.nikolaou Nosy List: gvanrossum, lys.nikolaou, miss-islington, pablogsal, vstinner
Priority: normal Keywords: patch

Created on 2020-05-18 10:52 by lys.nikolaou, last changed 2020-05-19 02:31 by miss-islington. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 20172 closed lys.nikolaou, 2020-05-18 11:00
PR 20183 closed lys.nikolaou, 2020-05-18 15:51
PR 20194 merged pablogsal, 2020-05-18 21:15
PR 20203 merged miss-islington, 2020-05-19 01:54
Messages (10)
msg369204 - (view) Author: Lysandros Nikolaou (lys.nikolaou) * (Python committer) Date: 2020-05-18 10:52
Running the PEG Parser benchmarks requires having memory_profiler installed.

In Tools/peg_generator:

➜  peg_generator git:(master) make time_stdlib
../../python -m zipfile -e data/xxl.zip data
../../python scripts/benchmark.py --parser=cpython --target=xxl compile
Traceback (most recent call last):
  File "/home/lysnikolaou/repos/cpython/Tools/peg_generator/scripts/benchmark.py", line 9, in <module>
    import memory_profiler
ModuleNotFoundError: No module named 'memory_profiler'

I propose to make that optional and only compute the timing benchmarks, in case memory_profiler is not available, since it's these benchmarks that are most important and most frequently run.
msg369231 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2020-05-18 14:19
Why not just install it?
msg369234 - (view) Author: Lysandros Nikolaou (lys.nikolaou) * (Python committer) Date: 2020-05-18 14:36
Because `make time` and all the related Makefile targets use the local built Python in the cpython parent directory. In order to install it, one would have to go through various steps, like installing openssl, configuring python with ssl support, building and installing python, creating a venv etc. Since it's more time-consuming that just running `make time` and since it's only needed for something that in most cases is not the most important thing, I think we can just go ahead and make it optional.

Is there a downside to this change I'm missing?
msg369236 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2020-05-18 14:44
Soon nobody will remember to look at the memory stats at all. But I will review.
msg369237 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-05-18 14:49
Doc/Makefile uses a virtual environment to install Sphinx and Sphinx extensions. I suggest to do something similar.
msg369239 - (view) Author: Lysandros Nikolaou (lys.nikolaou) * (Python committer) Date: 2020-05-18 15:12
> Doc/Makefile uses a virtual environment to install Sphinx and Sphinx extensions. I suggest to do something similar.

I like this approach. Let me try it.
msg369284 - (view) Author: miss-islington (miss-islington) Date: 2020-05-18 18:27
New changeset dc31800f86fbcd40ee616984820b885d8adaa6a7 by Lysandros Nikolaou in branch 'master':
bpo-40669: Install PEG benchmarking dependencies in a venv (GH-20183)
https://github.com/python/cpython/commit/dc31800f86fbcd40ee616984820b885d8adaa6a7
msg369298 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-05-18 22:11
> bpo-40669: Install PEG benchmarking dependencies in a venv (GH-20183)

I like this approach :-)
msg369306 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2020-05-18 22:37
New changeset 3764069f3ba2a7e932837ae19265059339dc86e3 by Pablo Galindo in branch 'master':
bpo-40669: Use requirements.pip when installing PEG dependencies (GH-20194)
https://github.com/python/cpython/commit/3764069f3ba2a7e932837ae19265059339dc86e3
msg369317 - (view) Author: miss-islington (miss-islington) Date: 2020-05-19 02:31
New changeset 3d062829deadcb8355e97090aba47138eb9bc649 by Miss Islington (bot) in branch '3.9':
bpo-40669: Use requirements.pip when installing PEG dependencies (GH-20194)
https://github.com/python/cpython/commit/3d062829deadcb8355e97090aba47138eb9bc649
History
Date User Action Args
2020-05-19 02:31:36miss-islingtonsetmessages: + msg369317
2020-05-19 01:54:07miss-islingtonsetpull_requests: + pull_request19501
2020-05-18 22:37:13pablogsalsetmessages: + msg369306
2020-05-18 22:11:40vstinnersetmessages: + msg369298
2020-05-18 21:15:54pablogsalsetpull_requests: + pull_request19495
2020-05-18 18:28:37lys.nikolaousetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2020-05-18 18:27:47miss-islingtonsetnosy: + miss-islington
messages: + msg369284
2020-05-18 15:51:04lys.nikolaousetpull_requests: + pull_request19484
2020-05-18 15:12:17lys.nikolaousetmessages: + msg369239
2020-05-18 14:49:01vstinnersetnosy: + vstinner
messages: + msg369237
2020-05-18 14:44:26gvanrossumsetmessages: + msg369236
2020-05-18 14:36:28lys.nikolaousetmessages: + msg369234
2020-05-18 14:19:55gvanrossumsetmessages: + msg369231
2020-05-18 11:00:05lys.nikolaousetkeywords: + patch
stage: patch review
pull_requests: + pull_request19471
2020-05-18 10:52:40lys.nikolaoucreate