Skip to content

Allow patching from a Read-er rather than a buffer.#8

Closed
fullermd wants to merge 2 commits intohucsmn:masterfrom
fullermd:reader
Closed

Allow patching from a Read-er rather than a buffer.#8
fullermd wants to merge 2 commits intohucsmn:masterfrom
fullermd:reader

Conversation

@fullermd
Copy link
Copy Markdown

This allows patching a file in a more streaming way, without pre-loading the whole thing into memory.

I think there's an argument to be made that this should be the only interface, and if somebody wants to buffer up in memory they should wrap a Cursor around a Vec or something themselves. After all, the output is just a Write-r, not a memory buffer; why shouldn't the input be similar? That would be a significant compat break though, and would presumably mean a major version. So this parallel API implementation was chosen.

than a byte buffer, allowing it to be used directly with files without
having to load the whole thing into memory up front.  Plumb up some
testing to be sure it matches the buffer variant.
@hucsmn
Copy link
Copy Markdown
Owner

hucsmn commented Mar 15, 2024

I personally have reservations on exposing Read + Seek source file support to the public API, because random accessing unbuffered source files could lead to unwanted higher syscall overheads (looking at the attachment that revealed roughly 20%-30% performance regression introduced by unlimited seek syscalls).

Therefore, I think providing seek APIs without any document guidance might be misleading to users sensitive to performance.

The following attachment provides a modified qbspatch version that supports choosing preload/seek/mmap read strategy of the source file, together with my own benchmark results of those strategies on a laptop (CPU: AMD 5800H, SSD: ZHITAI PC005 Active 1T).

read-strategies.zip

Test sample was taken from bunzipped firefox 110.0 and 123.0 tarball releases.

@hucsmn
Copy link
Copy Markdown
Owner

hucsmn commented Apr 6, 2024

Any more idea?

If there were no further improvements or documetations, I would close this PR and add explanations about the inconsistent buffer API to the documentation.

@hucsmn hucsmn closed this Apr 15, 2024
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.

2 participants