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

add AsLargeBytes support to unixfs files #24

Merged
merged 8 commits into from
Mar 3, 2022
Merged

add AsLargeBytes support to unixfs files #24

merged 8 commits into from
Mar 3, 2022

Conversation

willscott
Copy link
Collaborator

Supports seeking to relevant subset of a unixfs file.

@willscott willscott requested a review from mvdan March 2, 2022 13:37
file/file.go Outdated Show resolved Hide resolved
file/shard.go Outdated Show resolved Hide resolved
file/shard.go Outdated Show resolved Hide resolved
}

func (s *shardNodeFile) length() int64 {
// see if we have size specified in the unixfs data. errors fall back to length from links
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm slightly worried that, if a node is neither a "data" node nor a "links" node, you'll happily fall back to return 0 - you won't return any error at all, and you might swallow genuinely useful errors such as those from DecodeUnixFSData.

case io.SeekCurrent:
s.offset += offset
case io.SeekEnd:
s.offset = s.length() + offset
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note that this will recompute the length for every Seek call; consider memoizing the computed length, or even computing it upfront in AsLargeBytes

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should be a fairly cheap single field access in the map, and i don't know if lots of end seeks is a usage pattern that it makes sense to optimize for?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, if you're only using it for end seeks, you may be right that it's likely premature optimization

@willscott willscott requested a review from mvdan March 2, 2022 17:31
file/shard.go Outdated Show resolved Hide resolved
file/shard.go Outdated Show resolved Hide resolved
file/deferred.go Outdated Show resolved Hide resolved
@willscott willscott merged commit 94986a7 into main Mar 3, 2022
@willscott willscott deleted the seekbytes branch March 3, 2022 11:11
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