Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upRead file URLs in chunks #21466
Read file URLs in chunks #21466
Comments
|
I would like to work on this issue, if possible. Just to be clear, the goal is to read the file in chunks, rather than in one go? |
|
@JacksonCoder This may be a little bit more involved. We should be using the FetchTaskTarget as the destination for the result of the file read, calling |
|
Since we're returning the contents of the entire file in the |
|
The goal is not necessarily about reducing overall memory usage, but making the behaviour match for consumers of the network response - right now, users of the file protocol will observe a single process_response_chunk message that contains the entire file. This means that any code that can deal with incremental responses (like the HTML parser) don't get any chance to make use of the incomplete body. |
|
I see. So I should incrementally read the file in chunks and call |
|
What should the ideal size be for each chunk? |
|
I would try 32kb. |
|
Alright, I pushed my code to my fork of the repository, under the |
|
Should I modify tests, or just make a PR? |
|
I don't think there are any tests that could observe this. |
Read file URL in chunks <!-- Please describe your changes on the following line: --> This is a very straightforward PR: essentially, I replaced a `read_to_end` call that occurs when processing a file URL fetch with a loop that reads the file in chunks (with a `FILE_CHUNK_SIZE` constant that specifies the chunk size, which I've put at 32KB), and then calls `target.process_response_chunk` to process the chunk. The chunk is then appended to the fetch result, and once the end of the file is reached, the result is returned. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #21466. <!-- Either: --> - [ ] There are tests for these changes OR - [X] These changes do not require tests because there isn't really a way to observe this. <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/21560) <!-- Reviewable:end -->
The code to read from file URLs uses read_to_end to consume the entire file buffer and send it back all at once. This means the behaviour of HTTP URLs and file URLs for large files is very different.