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.

Sign up for GitHub

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

Jump to bottom

3929: Changed to Guava ByteStreams.copy() that has a bit better performance #1239

Merged
merged 1 commit into from
Feb 5, 2019
Merged

3929: Changed to Guava ByteStreams.copy() that has a bit better performance #1239

merged 1 commit into from
Feb 5, 2019

Conversation

andrey-qlogic
Copy link
Contributor

@andrey-qlogic andrey-qlogic commented Jan 29, 2019 •
edited
Loading

@andrey-qlogic andrey-qlogic requested a review from a team as a code owner January 29, 2019 20:00
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jan 29, 2019
@andrey-qlogic
Copy link
Contributor Author

@frankyn , would you please take a look.

frankyn
frankyn approved these changes Feb 4, 2019
View reviewed changes
Copy link
Member

@frankyn frankyn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

I'm curious what the performance benefit of this change is though. Could you provide more insights?

@andrey-qlogic
Copy link
Contributor Author

IOUtils.copy() uses BUF_SIZE = 4096, but Guava ByteStreams.copy 8192. That allows to copy slightly faster.

@frankyn
Copy link
Member

frankyn commented Feb 5, 2019

Thanks Andrey, that makes sense. Given it's the default value, did you try using the overload for copy(java.io.InputStream, java.io.OutputStream, int) that adjusts the size of the copy?

If the issue is the buffer size then the code could use the same library and use the overload instead.

@andrey-qlogic
Copy link
Contributor Author

Another commit improves the performance more significant way.

@andrey-qlogic andrey-qlogic merged commit e41f69b into googleapis:master Feb 5, 2019
@andrey-qlogic andrey-qlogic deleted the 3929 branch February 5, 2019 19:44
sduskis pushed a commit that referenced this pull request Mar 12, 2019
@Bhanditz @sduskis
Google api java client (#1244)
c7afd0c 
* Fix Copyright notices (#1174)

* add OSGI metadata

* more default behavior

* add OSGI info

* whitespace

* Remove (c) from copyright notices

* Remove obsolete mercurial and google code detritus (#1175)

* add OSGI metadata

* more default behavior

* add OSGI info

* whitespace

* Remove out of date docs we don't use anymore now that we're on Github and git

* update appengine SDK (#1178)

* update minimum maven (#1179)

* Fix AppEngine tests (#1184)

* Add continuous integration Kokoro CI config and badges (#1171)

* Add continuous integration Kokoro CI config and badges

* Fix indentation and https

* Add CODEOWNERS and issue/pr templates (#1185)

*  App Engine standard uses Java 7 or Java 8, not 6 and not 9 or later (#1186)

* App Engine uses Java 7 or later

* App Engine uses Java 8 or earlier

* Add the ending Java 6 support notice to README (#1190)

* Add Kokoro release scripts (#1188)

* Add Kokoro release scripts

* Remove unnecessary extra maven-jar compile after upgrading plugins

* Add the appengine api

* skip tests

* Install play services

* Install play services only if not installed

* Install other dependencies

* Fix the BUGFIX_VERSION constant

* Fix CODEOWNERS format

* Annotate versions for replacement (#1192)

* Release v1.26.0 (#1193)

* Release v1.26.0

* Actually use the nexus-staging-maven-plugin

* Fix leftover snapshot versions in assembly

* Fix snapshot version in assembly

* Bump next snapshot (#1194)

* Bump next snapshot

* Fix version replace region tag.

* Release improvements (#1195)

* Can run mvn assembly:single outside of build

* Filter dependency html files

* Use assembly to rename versioned properties files

* Remove datanucleus dependency (#1183)

* Fix possible NPE when missing os.version system property (#1210)

* Add tests for missing os.version system property

* Only append the OS name/version if we can get them from the system properties

* Fix test allow testing different system properties

* Fix tests

* Fix the reset of system properties

* Remove unused import

* undo change to AsesrtHeaderTransport

* Fix codestyle

* Delay request initialization for resumable upload (#1211)

* Delay request initialization for resumable upload until the content chunk is ready

* Fix javadoc link

* Adding test for a slow writer

* Make ContentChunk class static for internal clarity lint.

* Add final to ContentChunk fields set only in constructor

* Add google-api-client-bom artifact (#1213)

* Add google-api-client-bom artifact

* Apply README.md suggestions

* Release google-api-java-client v1.27.0 (#1214)

* Release v1.27.0

* bom needs developers section and release config/profile

* Bump next snapshot (#1215)

* Update guava to 26.0-android (#1218)

* Compile to Java 1.7 and fix javadoc links (#1222)

* Add Java 11 test configs (#1223)

* Add Java 11 test configs

* Update README badges

* Update maven-javadoc-plugin for Java 11

* Fix X-Goog-Api-Client java version parsing for Java 11

* Update README: edited and removed some redundancy (#1226)

* Fix old links to code.google.com  (#1225)

* Update google-api-client-dependencies.html

* updated links

fixed links

* Update package-info.java

* corrected links

* edited links

* Update google-api-client-appengine-dependencies.html

* Update google-api-client-appengine-dependencies.html

* Update google-http-client-appengine-dependencies.html

* Update google-oauth-client-appengine-dependencies.html

* Update GoogleAuthorizationCodeTokenRequest.java

* Update package-info.java

* Update GoogleOAuthConstants.java

* Update package-info.java

* Update package-info.java

* Update GoogleJsonError.java

* Remove usage of deprecated Backoff from google-http-java-client (#1221)

* Remove ClientLogin (#1224)

* Remove ClientLogin

* Bump clirr comparison version and note that we are removing beta classes

* Set expires_in to an hour instead of 1000 hours (#1229)

* Use maven enforcer plugin for maven version requirements (#1228)

* Handle the legacy endpoint in the MockTokenServerTransport (#1232)

* Handle the legacy endpoint in the MockTokenServerTransport with a warning

* Bump maven-surefire-plugin version for test environment fixes

* Release google-api-java-client v1.28.0 (#1233)

* Release v1.28.0

* Fix dependency for google-http-client-apache and Apache transport

* Fix promote/drop kokoro configs

* Bump next snapshot (#1234)

* 3929: Changed to Guava ByteStreams.copy() that has a bit better performance (#1239)

* Add automatic module name (#1235)

* 956: Chech for null to prevent autoboxing NPE. (#1241)

* 956: Chech for null to prevent autoboxing NPE.

* 956: Add an unit test.

* Infinite cycle with MediaHttpDownloader setContentRange download (#1242)

* 1101: Fixed infinite cycle with MediaHttpDownloader setContentRange download.

* 1101: Added unit tests.

* 1101: Fixed comments.

* Fix Replaced invalidateToken method to clearToken (#1243)

* Replaced invalidateToken method to clearToken

* Fix throw exception
This was referenced May 20, 2019
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sduskis sduskis sduskis approved these changes

@frankyn frankyn frankyn approved these changes

Assignees
No one assigned
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@andrey-qlogic @frankyn @sduskis @googlebot