Make all classes new-style. #160
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jmuchemb
reviewed
Apr 13, 2017
src/ZODB/serialize.py
Outdated
| @@ -680,7 +680,7 @@ def get_refs(a_pickle): | |||
| u.noload() | |||
|
|
|||
| # Now we have a list of references. Need to convert to list of | |||
| # oids and class info: | |||
| # oids and class info(object): | |||
There was a problem hiding this comment.
can you recheck with s/^(\s*)class ([[:alnum:]_]*):/\1class \2(object):/g ?
There was a problem hiding this comment.
Thanks for finding that!
Unfortunately the other regex misses not just the things that are in docstrings/doctests (which aren't important for performance, granted, but are important for consistency) but it also missed a handful of classes (E.g this one) for some reason.
jamadden
added a commit
to zopefoundation/ZEO
that referenced
this pull request
Apr 13, 2017
See zopefoundation/ZODB#160 for rationale. Also restore PyPy to the build matrix.
On PyPy, it's documented (http://pypy.org/performance.html#id3) that old-style classes are slow, and classes that derive from both old and new are especially slow. I checked with the PyPy devs on IRC today and they confirmed that's still true with the latest PyPy2 releases. Unfortunately, FileStorage was one such mixed class, as was Connection. Moving to new-style classes seems to have a positive impact in the benchmarks. Here's zodbshootout on PyPy2 5.7.1 against a FileStorage (running in --threads and with 5 reps to be sure we get warmed up). First, current ZODB: "Transaction", fs "Add 1000 Objects", 31,738 "Update 1000 Objects", 42,444 "Read 1000 Cold Objects", 51,894 "Read 1000 Hot Objects", 53,187 "Read 1000 Steamin' Objects", 835,877 And with this PR: "Transaction", fs "Add 1000 Objects", 35,651 "Update 1000 Objects", 54,906 "Read 1000 Cold Objects", 103,484 "Read 1000 Hot Objects", 84,721 "Read 1000 Steamin' Objects", 2,112,095 The tests that hit the storage extensively are notably faster, as are steamin and hot, Connection having been a mixed class. I ran all tests multiple times. The data files were removed between runs. There's some variation, but the new-style classes always seem better. For comparison, here's CPython 2.7.13: "Transaction", fs "Add 1000 Objects", 19,531 "Update 1000 Objects", 16,201 "Read 1000 Cold Objects", 22,111 "Read 1000 Hot Objects", 21,851 "Read 1000 Steamin' Objects", 880,582 Locally I've run the tests under 2.7 and they all passed.
jamadden
force-pushed
the
only-new-style
branch
from
e37eba9
to
69b580a
Compare
|
Rebased on master to resolve CHANGES.rst conflict with #161. |
|
Thanks. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
On PyPy, it's documented (http://pypy.org/performance.html#id3) that old-style classes are slow, and classes that derive from both old and new are especially slow. I checked with the PyPy devs on IRC today and they confirmed that's still true with the latest PyPy2 releases.
Unfortunately, FileStorage was one such mixed class (due to UndoLogCompatible), as was Connection (due to ExportImport).
Moving to new-style classes seems to have a positive impact in the benchmarks. Here's zodbshootout on PyPy2 5.7.1 against a FileStorage (running in --threads and with 5 reps to be sure we get warmed up). First, current ZODB:
And with this PR:
The tests that hit the storage extensively are notably faster, as are steamin and hot (Connection having been a mixed class).
I ran all tests multiple times. The data files were removed between runs. There's some variation, but the new-style classes always seem better.
For comparison, here's CPython 2.7.13:
Locally I've run the tests under 2.7 and they all passed.
Besides the benchmark improvements, this makes Python 2 closer to Python 3 for those classes.
This was a mechanical transform (with only one mis-change reverted) produced by this script: