-
Notifications
You must be signed in to change notification settings - Fork 768
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
pread/pwrite #951
Comments
Hi @yamt, thanks for opening an issue. I'm glad you proposed this, because I only learned pread/pwrite are in standard POSIX recently. It makes sense as an API and I think it would be good to add. pread/pwrite also shouldn't add much code cost as most of the logic should be shared behind the scenes.
The reason littlefs's seek moves the cache is because we usually need it for reading/writing, so I don't think pread/pwrite will escape this cost. Maybe when pread/pwrite bypasses the cache. That being said, non-disk-format related API changes are low priority at the moment. So it may take some time for this to land. |
i meant that, a straightforward emulation with lseek like the following would be inefficient as it moves
|
A native implementation could save a CTZ skip-list lookup, that's true. And there may be other opportunities for minor optimizations. But caching is a bit of a harder problem, since |
are you concerning cache-hit rate here? |
Ah yeah. Cases like these: // write blob
pwrite(f, 0, <data_blob>)
// scan for start of payload
lfs_off_t payload_start = 0;
while (true) {
pread(f, payload_start, buf, strlen("<header>"));
if (strcmp(buf, "<header>") == 0) {
break;
}
payload_start += 1;
} You could argue this should be done with Though, as an "optimization problem", there may not be a single correct impl here. |
Though I suppose you could also argue the FUSE impl should still call Maybe the best option is unclear. Not moving the cache in |
it would be nice to support pread/pwrite natively.
while an emulation with seek is possible, i guess it can have a few drawbacks:
The text was updated successfully, but these errors were encountered: