-
Notifications
You must be signed in to change notification settings - Fork 69
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
perf: collisions don't contain reserved names by default #684
perf: collisions don't contain reserved names by default #684
Conversation
The 'collisions' set does NOT contain RESERVED_NAMES; they are combined at runtime when needed. For a large real-world, this results in an order of magnitude reduction in memory usage. I'm not joking: Google Ads v5 uses 2.45 GB before this change and 223 MB after.
Codecov Report
@@ Coverage Diff @@
## master #684 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 26 26
Lines 1561 1560 -1
Branches 315 316 +1
=========================================
- Hits 1561 1560 -1
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also contains changes to add slots attributes to Metadata and Address. These are ancillary, optional, and open to negotiation.
In the above scenario, they reduce memory usage from 223 MB to 177 MB. If other people feel that the reduction in readability does not warrant the reduction in memory usage I am absolutely open to dropping that particular commit.
I would personally prefer preserving readability.
gapic/schema/metadata.py
Outdated
# Don't keep RESERVED_NAMES in self.collisions, | ||
# just combine the two when necessary. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps add a note that this is for memory usage?
9e25710
to
50c4dcd
Compare
🤖 I have created a release \*beep\* \*boop\* --- ### [0.35.9](https://www.github.com/googleapis/gapic-generator-python/compare/v0.35.8...v0.35.9) (2020-10-27) ### Performance Improvements * collisions don't contain reserved names by default ([#684](https://www.github.com/googleapis/gapic-generator-python/issues/684)) ([2ec6ea6](https://www.github.com/googleapis/gapic-generator-python/commit/2ec6ea6835256c0d7b252e035cf4eac1ff442647)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please).
The 'collisions' set does NOT contain RESERVED_NAMES; they are
combined at runtime when needed.
For a large real-world, this results in an order of magnitude
reduction in memory usage.
I'm not joking: Google Ads v5 uses 2.45 GB peak before this change
and 223 MB after.
Also contains changes to add
__slots__
attributes to Metadata and Address. These are ancillary, optional, and open to negotiation.In the above scenario, they reduce memory usage from 223 MB to 177 MB. If other people feel that the reduction in readability does not warrant the reduction in memory usage I am absolutely open to dropping that particular commit.
Includes other minor memory usage optimizations that collectively shave about 5 MB.