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

[TIMOB-20483] Android: fix file upload progress by adding setChunkedStreamingMode #8003

Merged
merged 2 commits into from Jun 27, 2016
Merged

Conversation

m1ga
Copy link
Contributor

@m1ga m1ga commented May 17, 2016

JIRA: https://jira.appcelerator.org/browse/TIMOB-20483

Problem
When upload bigger files the progress is faster then the real upload (see JIRA for full description)

Solution:
Android dev guide (https://developer.android.com/reference/java/net/HttpURLConnection.html) recommends the following:

For best performance, you should call either setFixedLengthStreamingMode(int) when the body length is known in advance, or setChunkedStreamingMode(int) when it is not. Otherwise HttpURLConnection will be forced to buffer the complete request body in memory before it is transmitted, wasting (and possibly exhausting) heap and increasing latency. 

adding setChunkedStreamingMode(1024) fixed the problem. 1024 (1kb) is recommended in different stackoverflow postings as a good default value.

@hieupham007
Copy link
Contributor

Code tested and works fine! However, what about this change that is NOT in the PR:
setRequestHeader("Transfer-Encoding", "chunked")
I saw some problems with it not being there: http://stackoverflow.com/questions/15913844/setting-setchunkedstreamingmode-in-httpurlconnection-fails-to-deliver-data-to-se

@hansemannn
Copy link
Collaborator

@m1ga thoughts?

@m1ga
Copy link
Contributor Author

m1ga commented Jun 3, 2016

sure, I can add that. It seems that you don't need it everytime but it shouldn't break anything so it's better to have it in!

@sgtcoolguy sgtcoolguy modified the milestone: 6.0.0 Jun 17, 2016
@sgtcoolguy sgtcoolguy removed the 6.0.0 label Jun 17, 2016
@ashcoding
Copy link
Contributor

Functionally tested and reviewed. APPROVED

@build
Copy link
Contributor

build commented Jun 27, 2016

Can one of the admins verify this patch?

@ashcoding
Copy link
Contributor

@build Verified.

@ashcoding ashcoding merged commit 65b25f7 into tidev:master Jun 27, 2016
@m1ga m1ga deleted the http_post branch June 28, 2016 20:07
ashcoding added a commit to ashcoding/titanium_mobile that referenced this pull request Jun 30, 2016
This was originally merged from
tidev#8003 and added by
m1ga. Due to merge conflicts, this code was moved and this commit is to
put it back in place.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants