Skip to content

Commit

Permalink
fix: yet more perf tweaks (#90)
Browse files Browse the repository at this point in the history
Responsible for a 33% performance boost in certain synthetic benchmarks.
  • Loading branch information
software-dov committed Aug 4, 2020
1 parent 2634ba5 commit eb7891c
Showing 1 changed file with 28 additions and 28 deletions.
56 changes: 28 additions & 28 deletions proto/message.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,18 +107,17 @@ def __new__(mcls, name, bases, attrs):
# Iterate over all the attributes and separate the fields into
# their own sequence.
fields = []
new_attrs = {}
oneofs = collections.OrderedDict()
proto_imports = set()
index = 0
for key, field in copy.copy(attrs).items():
for key, field in attrs.items():
# Sanity check: If this is not a field, do nothing.
if not isinstance(field, Field):
# The field objects themselves should not be direct attributes.
new_attrs[key] = field
continue

# Remove the field from the attrs dictionary; the field objects
# themselves should not be direct attributes.
attrs.pop(key)

# Add data that the field requires that we do not take in the
# constructor because we can derive it from the metaclass.
# (The goal is to make the declaration syntax as nice as possible.)
Expand Down Expand Up @@ -184,7 +183,7 @@ def __new__(mcls, name, bases, attrs):
# We determine an appropriate proto filename based on the
# Python module.
filename = "{0}.proto".format(
attrs.get("__module__", name.lower()).replace(".", "/")
new_attrs.get("__module__", name.lower()).replace(".", "/")
)

# Get or create the information about the file, including the
Expand All @@ -209,7 +208,7 @@ def __new__(mcls, name, bases, attrs):
file_info.descriptor.dependency.append(proto_import)

# Retrieve any message options.
opts = descriptor_pb2.MessageOptions(**attrs.pop("_pb_options", {}))
opts = descriptor_pb2.MessageOptions(**new_attrs.pop("_pb_options", {}))

# Create the underlying proto descriptor.
desc = descriptor_pb2.DescriptorProto(
Expand All @@ -223,9 +222,9 @@ def __new__(mcls, name, bases, attrs):

# If any descriptors were nested under this one, they need to be
# attached as nested types here.
for child_path in copy.copy(file_info.nested).keys():
if local_path == child_path[:-1]:
desc.nested_type.add().MergeFrom(file_info.nested.pop(child_path),)
child_paths = [p for p in file_info.nested.keys() if local_path == p[:-1]]
for child_path in child_paths:
desc.nested_type.add().MergeFrom(file_info.nested.pop(child_path))

# Add the descriptor to the file if it is a top-level descriptor,
# or to a "holding area" for nested messages otherwise.
Expand All @@ -235,7 +234,7 @@ def __new__(mcls, name, bases, attrs):
file_info.nested[local_path] = desc

# Create the MessageInfo instance to be attached to this message.
attrs["_meta"] = _MessageInfo(
new_attrs["_meta"] = _MessageInfo(
fields=fields,
full_name=full_name,
marshal=marshal,
Expand All @@ -244,7 +243,7 @@ def __new__(mcls, name, bases, attrs):
)

# Run the superclass constructor.
cls = super().__new__(mcls, name, bases, attrs)
cls = super().__new__(mcls, name, bases, new_attrs)

# The info class and fields need a reference to the class just created.
cls._meta.parent = cls
Expand Down Expand Up @@ -371,9 +370,14 @@ def __init__(self, mapping=None, **kwargs):
# * A dict
# * Nothing (keyword arguments only).

# Handle the first two cases: they both involve keeping
# a copy of the underlying protobuf descriptor instance.
if isinstance(mapping, self._meta.pb):
if mapping is None:
if not kwargs:
# Special fast path for empty construction.
self._pb = self._meta.pb()
return

mapping = kwargs
elif isinstance(mapping, self._meta.pb):
# Make a copy of the mapping.
# This is a constructor for a new object, so users will assume
# that it will not have side effects on the arguments being
Expand All @@ -389,26 +393,22 @@ def __init__(self, mapping=None, **kwargs):
if kwargs:
self._pb.MergeFrom(self._meta.pb(**kwargs))
return
if isinstance(mapping, type(self)):
# Performance hack to streamline construction from vanilla protos.
elif isinstance(mapping, type(self)):
# Just use the above logic on mapping's underlying pb.
self.__init__(mapping=mapping._pb, **kwargs)
return

# Handle the remaining case by converging the mapping and kwargs
# dictionaries (with kwargs winning), and saving a descriptor
# based on that.

if mapping is None:
mapping = {}
elif not isinstance(mapping, collections.abc.Mapping):
# Sanity check: Did we get something not a map? Error if so.
raise TypeError(
"Invalid constructor input for %s: %r"
% (self.__class__.__name__, mapping,)
)
mapping.update(kwargs)
else:
# Can't have side effects on mapping.
mapping = copy.copy(mapping)
# kwargs entries take priority for duplicate keys.
mapping.update(kwargs)

# Avoid copying the mapping unnecessarily
params = {}
# Update the mapping to address any values that need to be
# coerced.
Expand Down Expand Up @@ -572,8 +572,8 @@ def __init__(
self.package = package
self.full_name = full_name
self.options = options
self.fields = collections.OrderedDict([(i.name, i) for i in fields])
self.fields_by_number = collections.OrderedDict([(i.number, i) for i in fields])
self.fields = collections.OrderedDict((i.name, i) for i in fields)
self.fields_by_number = collections.OrderedDict((i.number, i) for i in fields)
self.marshal = marshal
self._pb = None

Expand Down

0 comments on commit eb7891c

Please sign in to comment.