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

fix: Helpers.swift: iOS application getting killed for using too much memory when creating temp file with range #439

Merged
merged 1 commit into from
Jan 18, 2025

Conversation

G7r7
Copy link
Contributor

@G7r7 G7r7 commented Jan 16, 2025

On an iphone 12 Pro when trying to upload a file of 3GB:

If found with the help of ChatGPT this alternative method of creating a temp file that works for a big file and keeps the memory usage stable because we have a fixed chunk size for read and write operations.

Thanks, again for the awesome project.

@781flyingdutchman
Copy link
Owner

That makes sense - I don't think I expected a range (chunk size) that big! Why did you delete the lines with the size check? That's still relevant here, isn't it?

@G7r7
Copy link
Contributor Author

G7r7 commented Jan 17, 2025

You are right it is an overlook on my side of the generated code. I will rework the commit to make a more conservative approach. I should be able to send an update this morning after I tested it again.

… file

Uploading a large file (e.g. approx. 3GB), with the range header set,
consummes too much memory resulting in app being killed by the task manager.

This patch creates a temporary file using chunks of 1MB from the source file.
This limits the overall memory consumption of such operation.
@G7r7
Copy link
Contributor Author

G7r7 commented Jan 17, 2025

I edited the commit to keep the size check at the beginning, changed error message to be more informative, and edited the commit message to be more descriptive and fitting to the repository existing commit messages.

I tested it and it works.

@781flyingdutchman 781flyingdutchman merged commit 3ebbde7 into 781flyingdutchman:main Jan 18, 2025
10 checks passed
@781flyingdutchman
Copy link
Owner

Thanks much! Merged (with some additional simplification) in V8.9.3

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