Try msgspec#212
Closed
goodboy wants to merge 8 commits into
Closed
Conversation
4 tasks
Owner
Author
|
Gah next hassle thing is not supporting native Opened another issue: msgspec/msgspec#30. |
goodboy
changed the base branch from
wip_fix_asyncio_gen_streaming
to
stdstream_clobber_fix
June 11, 2021 20:58
Owner
Author
|
Aight, this finally gets us working There's a small legnth-prefix framing as part of the new Also had to force cast a few more spots since there's no |
goodboy
force-pushed
the
stdstream_clobber_fix
branch
from
June 14, 2021 13:26
4c3c88d to
634dee1
Compare
Can only really use an encoder currently since there is no streaming api in `msgspec` as of currently. See msgspec/msgspec#27. Not sure if any encoding speedups are currently noticeable especially without any validation going on yet XD. First experiments toward #196
Add a `tractor._ipc.MsgspecStream` type which can be swapped in for `msgspec` serialization transparently. A small msg-length-prefix framing is implemented as part of the type and we use `tricycle.BufferedReceieveStream` to handle buffering logic for the underlying transport. Notes: - had to force cast a few more list -> tuple spots due to no native `tuple`decode-by-default in `msgspec`: msgspec/msgspec#30 - the framing can be understood by this protobuf walkthrough: https://eli.thegreenplace.net/2011/08/02/length-prefix-framing-for-protocol-buffers - `tricycle` becomes a new dependency
This change some super old (and bad) code from the project's very early days. For some redic reason i must have thought masking `trio`'s internal stream / transport errors and a TCP EOF as `StopAsyncIteration` somehow a good idea. The reality is you probably want to know the difference between an unexpected transport error and a simple EOF lol. This begins to resolve that by adding our own special `TransportClosed` error to signal the "graceful" termination of a channel's underlying transport. Oh, and this builds on the `msgspec` integration which helped shed light on the core issues here B)
goodboy
force-pushed
the
stdstream_clobber_fix
branch
from
July 1, 2021 00:21
634dee1 to
498cd7e
Compare
Owner
Author
|
Dropping this in favor of #214 which branches from master and includes a simpler git history. |
This file contains hidden or 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
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.
First draft trying out
msgspecovermsgpack-python.We currently can't easily decode streams since there's no api support, see:msgspec/msgspec#27, see the updates; we just need to implement our own framing with a bytes len prefix 🥳
The bulk of speed improvements are supposed to come from decoding + validation so we're not really seeing benefits from either of those yet 😂
TODO:
start_methodkwarg toopen_root_actor()), maybeserialization_method?defaultdicts for some reasonmypystuffmsgpack-numpyas the optional dep if we end up liking the performance here?msgspecrequires it) and we might as well sine we're onto a new alpha release anywaymsgspecuse in the readme