Skip to content

Add skip() method to Source interface to allow efficient seeks#14291

Merged
edolstra merged 3 commits intomasterfrom
skip-source
Oct 20, 2025
Merged

Add skip() method to Source interface to allow efficient seeks#14291
edolstra merged 3 commits intomasterfrom
skip-source

Conversation

@edolstra
Copy link
Member

Motivation

Taken from DeterminateSystems#232.

When listing the contents of a NAR or nario, if the input is a seekable file descriptor, we can save a lot of time by seeking over the bytes that we're not interested in. For instance, this allows speeding up nix nario list --no-contents from 7.42s to 0.42s on a cold page cache for a 15 GB system closure.

In a followup PR this will allow a cleaner alternative to #14273.

Context


Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

This allows FdSource to efficiently skip data we don't care about.
@xokdvium
Copy link
Contributor

@edolstra tests seem to fail with sanitizers

@xokdvium xokdvium disabled auto-merge October 17, 2025 20:39
@fzakaria
Copy link
Contributor

I was just thinking about this when I did #14273
I thought a "SeekableSource" or "RandomAccessSource" would have made it a little easier but then I couldn't figure out if NARAccessor should only accept that interface or also the source.

For now, it ended up passing GetNarBytes which special cases it but something like this would make it unecessary.

@edolstra
Copy link
Member Author

@xokdvium Fixed.

@edolstra edolstra enabled auto-merge October 20, 2025 12:25
sink.preallocateContents(size);

if (sink.skipContents) {
source.skip(size + (size % 8 ? 8 - (size % 8) : 0));
Copy link
Contributor

Choose a reason for hiding this comment

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

This just does an alignUp with alignment = 8? Seems like a useful utility that we could move to libutil in a follow-up?

Copy link
Contributor

Choose a reason for hiding this comment

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

(for my own edification) why does it align on 8 byte boundaries ?
Is too specific to x86_64 ?

What about other ISAs or 32bit.

Copy link
Contributor

@xokdvium xokdvium Oct 20, 2025

Choose a reason for hiding this comment

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

NARs pad to 8 bytes

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay you will hate this idea; but let's write a spec for https://kaitai.io/#what-is-it for fun as a side-project.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, that would be useful as a utility.

Copy link
Contributor

Choose a reason for hiding this comment

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

Done in #14305

Copy link
Member

@Ericson2314 Ericson2314 Oct 20, 2025

Choose a reason for hiding this comment

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

Okay you will hate this idea; but let's write a spec for https://kaitai.io/#what-is-it for fun as a side-project.

https://chatgpt.com/share/68f6a47c-add8-800c-9bcc-bf17799ea0f3 hope it can be improved! from a quickest of glances that has some cruft that ought not to be there

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll work on this tonight if you want to hack on it.

Copy link
Member

Choose a reason for hiding this comment

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

I can't join tonight, but yes I do think that would be a nice complement to the JSON Schema stuff @roberth and I have been doing.

@edolstra edolstra added this pull request to the merge queue Oct 20, 2025
Merged via the queue into master with commit ddf7de0 Oct 20, 2025
20 checks passed
@edolstra edolstra deleted the skip-source branch October 20, 2025 15:50
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.

4 participants