-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Faster default coder for unknown windows. #33382
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1421,6 +1421,37 @@ def estimate_size(self, value, nested=False): | |
return size | ||
|
||
|
||
class _OrderedUnionCoderImpl(StreamCoderImpl): | ||
def __init__(self, coder_impl_types, fallback_coder_impl): | ||
assert len(coder_impl_types) < 128 | ||
self._types, self._coder_impls = zip(*coder_impl_types) | ||
self._fallback_coder_impl = fallback_coder_impl | ||
|
||
def encode_to_stream(self, value, out, nested): | ||
value_t = type(value) | ||
for (ix, t) in enumerate(self._types): | ||
if value_t is t: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we want to enforce the class strictly or allow matching for subclasses too? if issubclass(value_t, t):
... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm comparing exact types here to make the round trip faithful. E.g. a Coder[T] is unlikely to faithfully encode and decode all subclasses of T. This is how fast primitives coder works as well. |
||
out.write_byte(ix) | ||
c = self._coder_impls[ix] # for typing | ||
c.encode_to_stream(value, out, nested) | ||
break | ||
else: | ||
if self._fallback_coder_impl is None: | ||
raise ValueError("No fallback.") | ||
out.write_byte(0xFF) | ||
self._fallback_coder_impl.encode_to_stream(value, out, nested) | ||
|
||
def decode_from_stream(self, in_stream, nested): | ||
ix = in_stream.read_byte() | ||
if ix == 0xFF: | ||
if self._fallback_coder_impl is None: | ||
raise ValueError("No fallback.") | ||
return self._fallback_coder_impl.decode_from_stream(in_stream, nested) | ||
else: | ||
c = self._coder_impls[ix] # for typing | ||
return c.decode_from_stream(in_stream, nested) | ||
|
||
|
||
class WindowedValueCoderImpl(StreamCoderImpl): | ||
"""For internal use only; no backwards-compatibility guarantees. | ||
|
||
|
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.
Just out of curiosity, why do we set the upper bound to 128 here? Shouldn't it be 255 given ix==0xFF is reserved for fallback coder in the following code?
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.
At first I was thinking about avoiding any sign issues, but also this leaves headroom for expanding the protocol in the future.