Skip to content

Commit

Permalink
perf: collisions don't contain reserved names by default (#684)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
software-dov authored Oct 27, 2020
1 parent 18425e2 commit 2ec6ea6
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 9 deletions.
7 changes: 2 additions & 5 deletions gapic/schema/metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,6 @@ class Address:
)
collisions: FrozenSet[str] = dataclasses.field(default_factory=frozenset)

def __post_init__(self):
super().__setattr__("collisions", self.collisions | RESERVED_NAMES)

def __eq__(self, other) -> bool:
return all([getattr(self, i) == getattr(other, i) for i
in ('name', 'module', 'module_path', 'package', 'parent')])
Expand Down Expand Up @@ -114,7 +111,7 @@ def module_alias(self) -> str:
while still providing names that are fundamentally readable
to users (albeit looking auto-generated).
"""
if self.module in self.collisions:
if self.module in self.collisions | RESERVED_NAMES:
return '_'.join(
(
''.join(
Expand Down Expand Up @@ -283,7 +280,7 @@ def with_context(self, *, collisions: FrozenSet[str]) -> 'Address':
``Address`` object aliases module names to avoid naming collisions in
the file being written.
"""
return dataclasses.replace(self, collisions=frozenset(collisions))
return dataclasses.replace(self, collisions=collisions)


@dataclasses.dataclass(frozen=True)
Expand Down
14 changes: 10 additions & 4 deletions gapic/schema/wrappers.py
Original file line number Diff line number Diff line change
Expand Up @@ -314,13 +314,19 @@ def recursive_field_types(self) -> Sequence[
return tuple(types)

@utils.cached_property
def recursive_fields(self) -> FrozenSet[Field]:
return frozenset(chain(
def recursive_resource_fields(self) -> FrozenSet[Field]:
all_fields = chain(
self.fields.values(),
(field
for t in self.recursive_field_types if isinstance(t, MessageType)
for field in t.fields.values()),
))
)
return frozenset(
f
for f in all_fields
if (f.options.Extensions[resource_pb2.resource_reference].type or
f.options.Extensions[resource_pb2.resource_reference].child_type)
)

@property
def map(self) -> bool:
Expand Down Expand Up @@ -1060,7 +1066,7 @@ def gen_resources(message):
yield type_

def gen_indirect_resources_used(message):
for field in message.recursive_fields:
for field in message.recursive_resource_fields:
resource = field.options.Extensions[
resource_pb2.resource_reference]
resource_type = resource.type or resource.child_type
Expand Down

0 comments on commit 2ec6ea6

Please sign in to comment.