Skip to content

ENH: Benchmarking import times. - #487

Closed
wrwrwr wants to merge 1 commit into
airspeed-velocity:masterfrom
wrwrwr:imports
Closed

wrwrwr wants to merge 1 commit into
airspeed-velocity:masterfrom
wrwrwr:imports

Conversation

@wrwrwr

@wrwrwr wrwrwr commented Nov 8, 2016

Copy link
Copy Markdown
Contributor

Imports require a fairly specific treatment:

  • just the first import takes time, a fresh interpreter is needed for each iteration;
  • the user may want to judge the impact of importing a standard library module, so the benchmark environment should be pristine;
  • goal_time may be approximated by adjusting repeat, but number in most cases should be 1.

There are a couple of approaches:

  • a specialized benchmark (as presented here);
  • a new subprocess argument;
  • finding a way to reset the Python interpreter;
  • sub-interpreters (Py_NewInterpreter).

The subprocess flag would be slightly more general, but for what else would it be useful? On the other hand, a specialized benchmark could be extended to also evaluate memory usage of imports (with #179).
Properly resetting the interpreter (if at all possible, cleaning sys.modules is not sufficient) or benchmarking in sub-interpreters may be hard to keep compatible across multiple versions of Python.

Some discussion can be found in numpy/numpy#8159 and astropy/astropy-benchmarks#15.

@wrwrwr
wrwrwr force-pushed the imports branch 2 times, most recently from fd65ca9 to 0250b51 Compare November 8, 2016 13:02
@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.3%) to 57.988% when pulling 0250b51 on wrwrwr:imports into a14d8dc on spacetelescope:master.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.2%) to 58.019% when pulling 0250b51 on wrwrwr:imports into a14d8dc on spacetelescope:master.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.07%) to 72.4% when pulling 86b6715 on wrwrwr:imports into cfe3d3e on spacetelescope:master.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.1%) to 72.427% when pulling 86b6715 on wrwrwr:imports into cfe3d3e on spacetelescope:master.

@pv

pv commented Dec 11, 2016 •

Copy link
Copy Markdown
Collaborator

It might be simpler to e.g. changing each repeat run in a separate process, or adding a separate num_processes attribute for it. This way, you could measure import time with an ordinary timing benchmark, setting number = 1.

The multi-process benchmarking appears to be useful also for allowing a better sampling of the measurement noise.

@wrwrwr

wrwrwr commented Dec 12, 2016 •

Copy link
Copy Markdown
Contributor Author

The first thing that I tried was a subprocess flag for the standard timing benchmark that made repeats run in fresh interpreters, but I didn't like how that came out, with most of the code conditioned on the flag and having to document that you should set number and repeat (in addition to all the current quirks).

I'll see how things will look with the changes from the statistics pull and maybe just drop support for combining goal_time with subprocess (that ended in a compromise on using system time anyway).

@astrofrog

Copy link
Copy Markdown
Contributor

Just wanted to comment that having this would be great!

@bsipocz

bsipocz commented May 21, 2018

Copy link
Copy Markdown

@pv - It seems this would be super useful for a number of projects, and I'm happy to rebase to the rid of the conflicts if that would be helpful to get it merged.

@bsipocz

bsipocz commented Jul 10, 2018

Copy link
Copy Markdown

@wrwrwr @pv - any progress on this? This would be super useful for quite a number of projects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants