Skip to content
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

Expand LZ4Pickler API to enable better perf & composability. #47

Merged
merged 2 commits into from
Jan 9, 2021
Merged

Conversation

geeknoid
Copy link

@geeknoid geeknoid commented Dec 29, 2020

  • Introduce UnpickledSize which returns the expected size of a pickled
    item once unpickled.

  • Introduce an Unpickle overload which takes a Span<byte> and one that
    takes an IBufferWriter for output. Both of these enable the caller to
    reuse buffers across multiple unpicklings, which increases perf and
    reduces pressure on the GC.

  • Introduce a Pickle overload which takes an IBufferWriter as output.
    Using this overload enables the caller to reuse buffers, and eliminates
    a full data copy by unpickling directly into the final destination
    buffer.

  • The code was rewritten to mostly be safe by leveraging Span<T>. There
    is only a teeny bit of unsafe code left in the pickling path.

Martin Taillefer added 2 commits December 29, 2020 17:19
- Introduce UnpickledSize which returns the expected size of a pickled
item once unpickled.

- Introduce an Unpickle overload which takes a Span<byte> and one that
takes an IBufferWriter for output. Both of these enable the caller to
reuse buffers across multiple unpicklings, which increases perf and
reduces pressure on the GC.

- Introduce a Pickle overload which takes an IBufferWriter as output.
Using this overload enables the caller to reuse buffers, and eliminates
a full data copy by unpickling directly into the final destination
buffer.

- The code was rewritten to mostly be safe by leveraging Span<T>. There
is only a teeny bit of unsafe code left in the pickling path.
@geeknoid geeknoid changed the title Add support for IBufferWriter to LZ4Pickler Expand LZ4Pickler API to enable better perf & composability. Dec 31, 2020
@MiloszKrajewski
Copy link
Owner

Hi,

Sorry for the delay, I just want to dedicate it a little bit more time that I have this weekend. I'll merge it next week.

Thanks,

@geeknoid
Copy link
Author

geeknoid commented Jan 8, 2021

Hi, anything I can do to make this easier to merge?

Thanks.

@MiloszKrajewski
Copy link
Owner

MiloszKrajewski commented Jan 9, 2021

Hi,
I made mess in git, as usual, git is just designed to be easily messed.
I merged it into master (I probably shouldn't). Unfortunately it was not compiling, so I reverted your pull request to keep master clean. I have your changes 'on a side' so I will take a look and reapply, but I definitely expected it to be less troublesome.
Work in progress.

@geeknoid
Copy link
Author

I'm sorry this is causing problems. I was not able to get your original build system working on my end, so I hacked something together enough so that I could run your unit tests and show that the code itself was functional.

Please let me know if I can help get this working fully.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants