-
Notifications
You must be signed in to change notification settings - Fork 7
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
Tape storage re-use #79
Comments
That sounds reasonable, I like the idea:) definitely worth a try |
I think the reason this didn't happen is lifetimes. since the tape has a lifetime bound to it's elements the re-use would make the borrowchedker think the life times are related: let a: &'a[u8] = b"42";
let mut reusable_tape = make_new_tape();;
parse(a, &mut reusable_tape);
// reusable_tape: Tape<'a>
do_stuff_with(reusable_tape);
reusable_tape.clear()
let a: &'b[u8] = b"43";
parse(b, &mut reusable_tape);
// reusable_tape: Tape<'b> <= lifetime conflict! 'a now needs to be 'b or wider |
I was able to modify the library to support this by adding an additional lifetime on the tape, I'll open a PR with the changes shortly but it was pretty straightforward to implement. The tape type now looks like this: pub type Tape<'tape, 'input> = Peekable<Drain<'tape, Node<'input>>>;
pub trait Deserialize<'input> {
fn from_tape<'tape>(tape: &mut Tape<'tape, 'input>) -> simd_json::Result<Self>
where
Self: Sized + 'input;
/// etc...
} This also required modifying every deserializer implementation to match the trait lifetime changes. After the changes I can reuse a tape like so (roughly): simd_json::fill_tape(json_msg, buffers, tape)?;
let mut itr = tape.0.drain(..).peekable();
let item = MyStruct::from_tape(&mut itr);
println!("item: {:?}", item);
let tape = tape.reset();
simd_json::fill_tape(second_json_msg, buffers, tape)?;
let mut itr = tape.0.drain(..).peekable();
let item = MyStruct::from_tape(&mut itr);
println!("other item: {:?}", item); The only downside of this is that any existing deserializer implementations need to be updated with the new type signatures |
Is it possible to reuse the storage that was allocated for the Tape Vec? Based on my digging it doesn't look to be possible in the current state.
If I am right about this not being possible at the moment, It feels like it isn't supported due to the tape type in this package being
Peekable<IntoIter<>>
but this looks like it could be solved by using aPeekable<Drain<>>
?Let me know if I am on the right track with this...
The text was updated successfully, but these errors were encountered: