Merge async APIs and generic reader/writer support #11
Conversation
|
@Stanzilla @Nnoggie FYI - want to take a look before I submit? |
|
Looked over it and tested it against our use case again. |
|
Thanks @Nnoggie! Before merging it, I'm waiting for @Meorawr to prep his "generic reader/writer" support (#8, #10) so I can evaluate using that model for where to add the coroutine suspension points. I'll create a separate PR for that one when it's ready, and if you can perform your local validation against it as well, I'd appreciate it - it'd be good to know if there are any performance differences. |
rossnichols
force-pushed
the
async-staging
branch
from
185abfa to
f8ec8b2
Compare
* Add chunks mode for coroutine processing * Requested updates * Split sync/async functions, others * Further updates
* Implement generic support for custom readers
This permits a new options table to be supplied to the Deserialize
and DeserializeValue functions on the library which may contain two
keys that control how the library reads bytes from the stream for
deserialization, and how it detects the end-of-stream state.
As-is this implementation is fully backwards compatible and allows
users of the library to throw processing into a coroutine for large
inputs if required, meeting some use cases that have prompted similar
PRs whilst also being more flexible and much simpler.
The new signatures for Deserialize and DeserializeValue are as follows:
function LibSerialize:DeserializeValue(input[, options])
function LibSerialize:Deserialize(input[, options])
The options parameter, if supplied, is expected to be a table that
may contain the following optional keys:
- readBytes
- atEnd
The "readBytes" key if present is expected to point to a function
of the signature "readBytes(input, i, j)" and must return a string
whose length is equal to (j - i) + 1). The "input" parameter is the
same as-supplied to Deserialize/DeserializeValue verbatim. The "i"
and "j" parameter represent an inclusive range of indices ([i, j])
typically as byte offsets within the input value. This convention is
identical to that provided by many Lua functions, such as string.sub,
table.concat, and unpack.
The "atEnd" key if present is expected to point to a function of the
signature "atEnd(input, i)". As with "readBytes", the "input" parameter
is whatever value was supplied to Deserialize/DeserializeValue, and the
"i" parameter represents the offset of the stream.
By default if neither of these keys are supplied, or no options table
is given, then the implementation will just default to using string.sub
for "readBytes" and for "atEnd" will test if the offset exceeds the
length of the input as-provided by the length operator.
Examples are provided in the test cases, where we test the following
scenarios:
- A case where "input" may have additional data after the payload that
we don't want deserializing.
- A case where the "input" is a table of character bytes.
- A case where we yield from within "readBytes" and feed data from the
main thread, simulating receiving data with a potentially unknown
bound.
* Implement support for custom writers
This mirrors recent changes submitted for the deserialization API and
provides a generic system for custom writer support to control how and
where the byte stream generated during serialization is written. The
change as-is is fully backwards compatible and passes the existing and
expanded test cases.
Primary use cases would align with existing async serialization attempts
but don't put the complexity of thread management or throttling logic
into the library, instead leaving the minutia of how those aspects are
handled to callers. It also enables some other advanced use cases such
as streaming the data to a destination without buffering it fully
beforehand, which could enable efficiencies elsewhere.
rossnichols
force-pushed
the
async-staging
branch
from
c3625fb to
906b036
Compare
|
Update - I've moved the built-in support for async into the reader/writer layer, rather than the serialize/deserialize layer. It's still written so that someone using a custom reader/writer can take advantage of it. I still need to do a pass over the documentation and tests. |
|
@Meorawr some of the updates I made here were inspired by your comment in your implementation: -- To minimize changes elsewhere with this initial implementation, this
-- function must return new closures that bind the 'object' to the first
-- parameter of the above functions. This could be optimized to remove the
-- indirection, but requires modifying all call sites of these functions.Do my changes align with how you envisioned the optimization? I still have a layer of indirection because the reader/writer calls are flowing through a specific function ( |
rossnichols
changed the title
|
Alright, the PR is ready for review! I've completed my merging of the two PRs and updated the tests and readme. @Nnoggie @Stanzilla mind validating one more time locally? @Meorawr any comments on my merging of your PR? |
|
I think the PR's in a good enough shape to submit. Will keep an eye on any issues being opened. |
|
Thank you for landing this, we'll report back if there are any issues! |
This PR continues the work from @oratory in #4. The API surface is unchanged, but the following updates were made:
asyncan option and renamedyieldCheckFntoyieldChecknils for serialization inputThis PR also merges in the generic reader/writer support from @Meorawr in #12. The API surface is again unchanged, but I revised the implementation a bit as I was combining it with the above work.