-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Avoid using pb
internals for datastore on App Engine
#298
Comments
Currently digging into https://www.simonmweber.com/2013/06/18/python-protobuf-on-app-engine.html to see what I can find out. |
@pcostell I'm not sure your previous comments are relevant to I added a debug print to check
I was worried this was slow so did the same locally with the version on PyPI (0.3.0) and got similar times:
ISTM the App Engine speed-up is due to the fast-path between the App Engine app and the Cloud Datastore machines. UPDATE: I'm fairly certain Patrick was referring to SECOND UPDATE: I broke down |
Great analysis! |
Awesome, good to know. Something that may be important: The reason GAE protos have slow access is that they use the C++ library, which requires a process boundary cross. This is used instead of a pure python library because serialization and deserialization is much faster in C++ than in python. In may be worthwhile to still minimize proto access in order to minimize SWIG-code entry points when users do have the C++ libraries (running on VMs or if GAE adds google.protobuf to the runtime). |
Luckily the I analyzed the imports, and the only imports from outside the library are:
|
Currently, when using the externally available protobuf library on App Engine, it is using the python-only version of the protobuf library (it's impossible to upload a C library to the python runtime). However, once we are able to get the external C library into the App Engine runtime it will start using the C version instead of the python version. The C version has faster encode/decode times however you do pay a penalty when accessing protocol buffer fields because it has to cross a process boundary. We saw this in NDB's Key implementation which tried to store everything internally in protobuf rather than storing the fields as native python types. I went through and it looks like you only access the protobuf on serialization and deserialization, which is perfect, so I'm leaving this as closed. |
@pcostell we just recently dropped the "long-lived protobuf" inside Query: glad that helps. |
Co-authored-by: Anthonios Partheniou <partheniou@google.com>
Co-authored-by: Anthonios Partheniou <partheniou@google.com>
Co-authored-by: Anthonios Partheniou <partheniou@google.com>
Source-Link: https://togithub.com/googleapis/synthtool/commit/92006bb3cdc84677aa93c7f5235424ec2b157146 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:2e247c7bf5154df7f98cce087a20ca7605e236340c7d6d1a14447e5c06791bd6
) Source-Link: googleapis/synthtool@c1dd87e Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:2d13c2172a5d6129c861edaa48b60ead15aeaf58aa75e02d870c4cbdfa63aaba Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
- [ ] Regenerate this pull request now. PiperOrigin-RevId: 472561635 Source-Link: googleapis/googleapis@332ecf5 Source-Link: googleapis/googleapis-gen@4313d68 Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiNDMxM2Q2ODI4ODBmZDlkNzI0NzI5MTE2NGQ0ZTlkM2Q1YmQ5ZjE3NyJ9
* chore: use gapic-generator-python 0.63.2 docs: add generated snippets PiperOrigin-RevId: 427792504 Source-Link: googleapis/googleapis@55b9e1e Source-Link: googleapis/googleapis-gen@bf4e86b Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiYmY0ZTg2Yjc1M2Y0MmNiMGVkYjFmZDUxZmJlODQwZDdkYTBhMWNkZSJ9 * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
* fix: integrate gapic-generator-python-1.4.1 and enable more py_test targets PiperOrigin-RevId: 473833416 Source-Link: googleapis/googleapis@565a550 Source-Link: googleapis/googleapis-gen@1ee1a06 Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiMWVlMWEwNmM2ZGUzY2E4Yjg0MzU3MmMxZmRlMDU0OGY4NDIzNjk4OSJ9 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
…298) Source-Link: googleapis/synthtool@6b4d5a6 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:f792ee1320e03eda2d13a5281a2989f7ed8a9e50b73ef6da97fac7e1e850b149 Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
- [ ] Regenerate this pull request now. PiperOrigin-RevId: 440970084 Source-Link: googleapis/googleapis@5e0a3d5 Source-Link: googleapis/googleapis-gen@b0c628a Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiYjBjNjI4YTNmYWRlNzY4ZjIyNWQ3Njk5Mjc5MWVhMWJhMmE4ODFiZSJ9 docs: fix type in docstring for map fields
Source-Link: googleapis/synthtool@1b9ad76 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:9db98b055a7f8bd82351238ccaacfd3cda58cdf73012ab58b8da146368330021
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
…posed match confidence and parameter in AnalyzeContentResponse feat: added DTMF and PARTIAL DTMF type in recognition result (#298) PiperOrigin-RevId: 374460003 Source-Link: googleapis/googleapis@1309578 Source-Link: googleapis/googleapis-gen@be22498
….10.0 (#298) Co-authored-by: Anthonios Partheniou <partheniou@google.com>
Source-Link: googleapis/synthtool@38e11ad Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:4e1991042fe54b991db9ca17c8fb386e61b22fe4d1472a568bf0fcac85dcf5d3
Source-Link: googleapis/synthtool@c4dd595 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:ce3c1686bc81145c81dd269bd12c4025c6b275b22d14641358827334fddb1d72
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Co-authored-by: Anthonios Partheniou <partheniou@google.com>
Source-Link: https://togithub.com/googleapis/synthtool/commit/25083af347468dd5f90f69627420f7d452b6c50e Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:e6cbd61f1838d9ff6a31436dfc13717f372a7482a82fc1863ca954ec47bff8c8
* chore(deps): update all dependencies * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * revert Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com> Co-authored-by: Anthonios Partheniou <partheniou@google.com>
Source-Link: googleapis/synthtool@06e8279 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:b3500c053313dc34e07b1632ba9e4e589f4f77036a7cf39e1fe8906811ae0fce Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
* chore(tests): Request id test * Fixing test formatting. Co-authored-by: Anthonios Partheniou <partheniou@google.com>
Co-authored-by: Anthonios Partheniou <partheniou@google.com>
* ci(python): run lint / unit tests / docs as GH actions Source-Link: googleapis/synthtool@57be0cd Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:ed1f9983d5a935a89fe8085e8bb97d94e41015252c5b6c9771257cf8624367e6 * add commit to trigger gh action * work around template bug; set coverage level to 99% * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com> Co-authored-by: Anthonios Partheniou <partheniou@google.com>
…regated data instead of individual data access records (#298) * update test configuration * remove custom noxfile configs for now * remove MaximumUserAccess rom sample * add comment in noxfile_config.py * run blacken session * lint * add pytest * Update noxfile_config.py * Update noxfile_config.py * delete enhanced measurement settings samples as this functionality is no longer supported in v1alpha * fix the samples tests * do not use the `maximum_user_access` field and `update` operation in properties.firebase_links tests as this is no longer supported in v1alpha * use `creator_email_address` instead of `email_address` field in properties.google_ads_links_list() test as the field has been renamed in v1alpha * fix the samples test * docs: updates the `properties_run_access_report` sample to return aggregated data instead of individual data access records * docs: updates the `properties_run_access_report` sample to return aggregated data instead of individual data access records Co-authored-by: Anthonios Partheniou <partheniou@google.com>
* chore: put Locations mixin back PiperOrigin-RevId: 477369320 Source-Link: googleapis/googleapis@6954c4d Source-Link: googleapis/googleapis-gen@81e5272 Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiODFlNTI3MjY3MWJkMWM1YzhhYzE1YTQ0YmQyNTkyMmYwYjkzZWFlNiJ9 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Co-authored-by: Anthonios Partheniou <partheniou@google.com>
Source-Link: https://togithub.com/googleapis/synthtool/commit/26c7505b2f76981ec1707b851e1595c8c06e90fc Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:f946c75373c2b0040e8e318c5e85d0cf46bc6e61d0a01f3ef94d8de974ac6790
Source-Link: googleapis/synthtool@a37f74c Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:d3de8a02819f65001effcbd3ea76ce97e9bcff035c7a89457f40f892c87c5b32 Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
See #282 for context.
From @pcostell:
and
The text was updated successfully, but these errors were encountered: