google-api-go-client
google-api-go-client copied to clipboard
googleapis
gensupport: Allow user to provide his own buffer used for uploading
When uploading lots of files using ObjectStorage.Insert(), each file allocates his own buffer. This results in increased CPU usage and high RSS memory.
WithBuffer allows user to provide his own buffer which will be used for streaming.
Graph below shows how many allocations were done during upload of ~30k of 10MB files:
With this change, there're no allocations at all on library side (pool.New is code on caller side)
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).
:memo: Please visit https://cla.developers.google.com/ to sign.
Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.
What to do if you already signed the CLA
Individual signers
- It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.
Corporate signers
- Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
- The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
- The email used to register you as an authorized contributor must also be attached to your GitHub account.
ℹ️ Googlers: Go here for more info.
![google-cla[bot] avatar](https://avatars.githubusercontent.com/in/42202?v=4 "Published by a pub.dev verified publisher"){kind=small}
@zimnx Can you please create an issue for this PR. We like to discuss features on issues before implementing them. Thanks!
any chance to merge it soon? Other MediaOptions are not directly used in generated clients, there are only mentions about them in comments around Media function where they can be used.
any chance to merge it soon? Other MediaOptions are not directly used in generated clients, there are only mentions about them in comments around Media function where they can be used.
Ahh I see I was mistaken about that-- so it will just be the manual client writer that will need to be updated for storage (and maybe bigquery).
I'm going to do a bit more discussion with my colleagues and let you know. You'd also definitely have to add a test for the new media option to https://github.com/googleapis/google-api-go-client/blob/326e17a21103f4ccf44ac1b40587ce7bcdd58b14/internal/gensupport/media_test.go#L150 (but no need to do that now, let me make sure I have approval on this direction first).
👋 Regularly seeing high heap/allocations which this PR could help. Anything else needed to get this merged?
If it helps anyone: we "fixed" this by setting ChunkSize=0 on all our Writers, which disables use of the buffer entirely. Since we do our own retries elsewhere that seems fine.