Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nfo output is slow with pypy #76

Closed
andythenorth opened this issue Dec 21, 2019 · 3 comments
Closed

nfo output is slow with pypy #76

andythenorth opened this issue Dec 21, 2019 · 3 comments

Comments

@andythenorth
Copy link
Contributor

@andythenorth andythenorth commented Dec 21, 2019

nmlc is generally faster with pypy than python 3.8

However output to nfo is specifically about 5x slower with pypy compared to python 3.8.

This appears to be due to poor performance of StringIO, which is a known issue with pypy: https://bitbucket.org/pypy/pypy/pull-requests/132/speed-up-pickledumps-fixes-issue-979

I created a test substituting StringIO with simple list append / join, which performs much faster
andythenorth@b2c9338

For Iron Horse compile the difference is ~10s with pypy vs ~2s with python 3.8. In the slower version the extra 8s represents ~25% of total compile time for the grf, so it is not insignificant.

My patch is not a viable candidate for merging, but this would be a helpful issue to fix.

@andythenorth
Copy link
Contributor Author

@andythenorth andythenorth commented Dec 21, 2019

FLHerne has a patch which substantially reduces output time with pypy in Iron Horse and FIRS compiles http://www.flherne.uk/files/nicer-quick-output.diff

@andythenorth
Copy link
Contributor Author

@andythenorth andythenorth commented Apr 21, 2020

I ran some tests against http://www.flherne.uk/files/nicer-quick-output.diff using Iron Horse and nml at rev 660722b

Timings are purely for 'Writing output' step, tested over a couple of runs, with primed caches.

Python 3.8

Without patch: ~1.5s
With patch: ~1.7s - but also note there's an ignored Exception "FileNotFoundError: [Errno 2] No such file or directory: './generated/iron-horse.nfo.e5kf2jss.nmltemp'"

13% slower.

**PyPy"

Without patch: ~6.0s
With patch: ~1.0s

85% faster.

The specific nml rev used is chosen because the patch conflicts with the next rev f71103b

The 5s improvement for PyPy with this patch reduces a total compile time from 30s to 25s.

I am +/-0 on whether we should include this; it's a massive boost for PyPy, but a performance regression for Python 3.8.

For context though:

  • when developing newgrf, I compile frequently, 10s of times per hour, and reduced cycle times are really beneficial for staying in flow
  • the ideal compile is around 10 seconds
  • I've previously achieved ~30% performance increases by aggregating reductions of a couple of seconds here and there
  • improvements of less than a couple of seconds aren't usually worth the candle, and tend to just be noise
  • it's rare that a single large intervention be made to improve newgrf compile time; notable exceptions are:
    • the caching strategy that frosch added several years ago
    • using nmlc with PyPy instead of Python 3 for (around 50-60% faster)
    • aggressively substituting grfcodec for nml where only graphics have changed
@FLHerne
Copy link
Contributor

@FLHerne FLHerne commented Apr 22, 2020

The slow performance of StringIO is fixed in PyPy 7.3.1.

@FLHerne FLHerne closed this Apr 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.