go-car
go-car copied to clipboard
ipld
Add a 'skip' parameter to writev1 so that the beginning of a car can …
Remaining:
- [x] cleaner/more efficient teeing implementation (remove un-needed buffer copies)
- [x] Add test demonstrating skipping arbitrary prefixes of the car
The remaining potential work here is allow skip to be an arbitrary byte offset in the car file. currently it's limited to block boundaries, and because of the API it's difficult to know which block boundary nearest to the requested skip offset has been used. If it's acceptable for the caller to skip only to block boundaries, this could be sufficient as-is, otherwise the wrapper for the individual reader needs to be updated to also partially skip.
I cleaned up the tee-ing writer and added a test that seems to verify that we can use this to advance to arbitrary byte offsets in a car.
@rvagg maybe can provide a first round of review
This is some super-impressive code @willscott, once I got my head around it. The scope is large and it's doing some really complex stuff and I can't really fault it from a review. Most of my suggested changes are just comments I think might be helpful for anyone else following the breadcrumbs in future.
I'm now toying with integrating it into Boost and will report back.
(FYI I messed this branch up a tiny bit so ended up rebasing it on master, removing the merge commits in the process, so the commits have changed but the resulting code is the same)
@rvagg are we able to merge this - was there something else we were waiting for on it?
Yeah, it needs #327 to get closer to working as advertised, but even then it's still not quite right. I stepped back from working on this as the complexity increased and I started questioning the utility since we mainly want this (for now) for the explore-all case and it's the selectors that make this difficult to properly do.
As it is, it can't replace Boost's impl because of caching enabled by explore-all shortcuts that we're not allowing ourselves to touch here.
I'd be fine merging in #327 to here, but I'm not sure we should merge this in since it's not going to give people what it says it does.
I forgot to add that I'm not intending to suggest this is a lost cause. My thinking, the last time I touched this, was that we should try to implement an efficient explore-all case here, and then iterate on that for the more complex cases.