PEG Parser benchmarks fail if memory_profiler is not installed

[lysnikolaou]

BPO	40669
Nosy	@gvanrossum, @vstinner, @lysnikolaou, @pablogsal, @miss-islington
PRs	bpo-40669: Make memory_profiler optional for running PEG benchmarks #20172 bpo-40669: Install PEG benchmarking dependencies in a venv #20183 bpo-40669: Use requirements.pip when installing PEG dependencies #20194 [3.9] bpo-40669: Use requirements.pip when installing PEG dependencies (GH-20194) #20203

^{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/lysnikolaou'
closed_at = <Date 2020-05-18.18:28:37.045>
created_at = <Date 2020-05-18.10:52:40.424>
labels = []
title = 'PEG Parser benchmarks fail if memory_profiler is not installed'
updated_at = <Date 2020-05-19.02:31:36.281>
user = 'https://github.com/lysnikolaou'

bugs.python.org fields:

activity = <Date 2020-05-19.02:31:36.281>
actor = 'miss-islington'
assignee = 'lys.nikolaou'
closed = True
closed_date = <Date 2020-05-18.18:28:37.045>
closer = 'lys.nikolaou'
components = []
creation = <Date 2020-05-18.10:52:40.424>
creator = 'lys.nikolaou'
dependencies = []
files = []
hgrepos = []
issue_num = 40669
keywords = ['patch']
message_count = 10.0
messages = ['369204', '369231', '369234', '369236', '369237', '369239', '369284', '369298', '369306', '369317']
nosy_count = 5.0
nosy_names = ['gvanrossum', 'vstinner', 'lys.nikolaou', 'pablogsal', 'miss-islington']
pr_nums = ['20172', '20183', '20194', '20203']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue40669'
versions = []

[lysnikolaou]

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.

[gvanrossum]

Why not just install it?

[lysnikolaou]

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?

[gvanrossum]

Soon nobody will remember to look at the memory stats at all. But I will review.

[vstinner]

Doc/Makefile uses a virtual environment to install Sphinx and Sphinx extensions. I suggest to do something similar.

[lysnikolaou]

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.

[miss-islington]

New changeset dc31800 by Lysandros Nikolaou in branch 'master':
bpo-40669: Install PEG benchmarking dependencies in a venv (GH-20183)
dc31800

[vstinner]

bpo-40669: Install PEG benchmarking dependencies in a venv (GH-20183)

I like this approach :-)

[pablogsal]

New changeset 3764069 by Pablo Galindo in branch 'master':
bpo-40669: Use requirements.pip when installing PEG dependencies (GH-20194)
3764069

[miss-islington]

New changeset 3d06282 by Miss Islington (bot) in branch '3.9':
bpo-40669: Use requirements.pip when installing PEG dependencies (GH-20194)
3d06282