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

handle_file_request - handling of large files with ranges with less resources #1559

Closed
wants to merge 2 commits into from
Closed

handle_file_request - handling of large files with ranges with less resources #1559

wants to merge 2 commits into from

Conversation

stanislavpetkov
Copy link

handle_file_request rewritten,
in case of RANGE header, only the part of ranges will be loaded and transmitted.
Before even if you need 10bytes of 2TB file it will try to load it in memory, and than will apply ranges.

file_request_handler_ call is moved before post_routing_handler_ (I don't know why file_request_handler_ is called before data delivery, but I kept it the same way)

find_content_type -

  1. changed return type to string, as it is converted to string anyway .
  2. default type is changed to application/octetstring
  3. added some extra types

in case of RANGE header, only the part of ranges will be loaded and transmitted.
Before even if you need 10bytes of 2TB file it will try to load it in memory

find_content_type changed return type to string, as it is converted to string anyway
default type is application/octetstring
added some extra types
@yhirose
Copy link
Owner

yhirose commented Apr 26, 2023

@stanislavpetkov thanks for the pull request. Before I start reviewing it, could you do the following things?

  1. Add at least one unit test to validate your change in test/test.cc
  2. Format your changes with clang-format with options in .clang-format in this repo.

Thanks!

@stanislavpetkov
Copy link
Author

stanislavpetkov commented Apr 27, 2023

@yhirose

  1. Add at least one unit test to validate your change in test/test.cc - it is covered by the range headers and static content download tests. I can't add anything more
  2. Format your changes with clang-format with options in .clang-format in this repo. - Done from MSVC - it detected the clang-format

@yhirose yhirose closed this in 6bb580c Jul 31, 2023
@yhirose
Copy link
Owner

yhirose commented Jul 31, 2023

@stanislavpetkov I reviewed the code, but I decided implementing it in a different way. It supports not only a single range, but multiple ranges properly. Also better performance and less memory usage with memory mapped file instead of using std::vector and std::ifstream. Thanks!

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