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

Avoid using pb internals for datastore on App Engine #298

Closed
dhermes opened this issue Oct 27, 2014 · 8 comments
Closed

Avoid using pb internals for datastore on App Engine #298

dhermes opened this issue Oct 27, 2014 · 8 comments
Assignees
Labels
api: datastore Issues related to the Datastore API. 🚨 This issue needs some love. triage me I really want to be triaged.

Comments

@dhermes
Copy link
Contributor

dhermes commented Oct 27, 2014

See #282 for context.

From @pcostell:

Just a random note about proto usage:
If you plan to let users run gcloud-python in the App Engine python runtime, proto access is
really slow (I believe because of the sandbox). I would limit proto access to serialization
and deserialization only.

and

Not 100% sure, but I think it uses a C proto library so it goes outside the process boundary.

@dhermes
Copy link
Contributor Author

dhermes commented Dec 30, 2014

Currently digging into https://www.simonmweber.com/2013/06/18/python-protobuf-on-app-engine.html to see what I can find out.

@dhermes
Copy link
Contributor Author

dhermes commented Dec 30, 2014

@pcostell I'm not sure your previous comments are relevant to gcloud. I just created a test application and the proto used is via the external protobuf library, not the proto libraries provided in App Engine.

I added a debug print to check Entity.save() times in a single request and got:

  • 0.197650 seconds
  • 0.113310 seconds
  • 0.169830 seconds

I was worried this was slow so did the same locally with the version on PyPI (0.3.0) and got similar times:

  • 1.152835 seconds (app was cold)
  • 0.233535 seconds
  • 0.359164 seconds
  • 0.217317 seconds

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 google.net.proto and google.net.proto2 as the libraries to avoid using. I checked to make sure we don't ever import from proto libraries included in GAE.

SECOND UPDATE: I broke down Entity.save() into lower and lower level component pieces until the network was the only thing left. The times of each show that the impact of using the protobuf library is negligible.

@dhermes
Copy link
Contributor Author

dhermes commented Dec 30, 2014

@silvolu @tseaver FYI gcloud works just fine deployed in App Engine.

@tseaver
Copy link
Contributor

tseaver commented Dec 31, 2014

Great analysis!

@tseaver tseaver closed this as completed Dec 31, 2014
@pcostell
Copy link
Contributor

pcostell commented Jan 4, 2015

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).

@dhermes
Copy link
Contributor Author

dhermes commented Jan 5, 2015

Luckily the protobuf library only uses C-code from the standard library. As @tseaver pointed out earlier today, struct is plenty fast.

I analyzed the imports, and the only imports from outside the library are:

StringIO.StringIO
cStringIO
cStringIO.StringIO
copy_reg
copyreg  # Py3, comment added by dhermes
io.BytesIO  # Py3, comment added by dhermes
operator
os
re
struct
sys
uuid
weakref

@pcostell
Copy link
Contributor

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.

@tseaver
Copy link
Contributor

tseaver commented Jan 12, 2015

@pcostell we just recently dropped the "long-lived protobuf" inside Query: glad that helps.

@jgeewax jgeewax modified the milestone: Datastore Stable Jan 30, 2015
@yoshi-automation yoshi-automation added triage me I really want to be triaged. 🚨 This issue needs some love. labels Apr 7, 2020
atulep pushed a commit that referenced this issue Apr 6, 2023
Co-authored-by: Anthonios Partheniou <partheniou@google.com>
atulep pushed a commit that referenced this issue Apr 6, 2023
Co-authored-by: Anthonios Partheniou <partheniou@google.com>
atulep pushed a commit that referenced this issue Apr 18, 2023
Co-authored-by: Anthonios Partheniou <partheniou@google.com>
parthea pushed a commit that referenced this issue Jun 4, 2023
Source-Link: https://togithub.com/googleapis/synthtool/commit/92006bb3cdc84677aa93c7f5235424ec2b157146
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:2e247c7bf5154df7f98cce087a20ca7605e236340c7d6d1a14447e5c06791bd6
parthea pushed a commit that referenced this issue Jun 4, 2023
)

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>
parthea pushed a commit that referenced this issue Jun 4, 2023
- [ ] Regenerate this pull request now.

PiperOrigin-RevId: 472561635

Source-Link: googleapis/googleapis@332ecf5

Source-Link: googleapis/googleapis-gen@4313d68
Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiNDMxM2Q2ODI4ODBmZDlkNzI0NzI5MTE2NGQ0ZTlkM2Q1YmQ5ZjE3NyJ9
parthea pushed a commit that referenced this issue Jun 4, 2023
* 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>
parthea pushed a commit that referenced this issue Jun 4, 2023
* 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>
parthea pushed a commit that referenced this issue Jun 4, 2023
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
parthea pushed a commit that referenced this issue Jun 4, 2023
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
parthea pushed a commit that referenced this issue Jun 4, 2023
…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>
parthea pushed a commit that referenced this issue Jul 6, 2023
- [ ] 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
parthea pushed a commit that referenced this issue Sep 20, 2023
Source-Link: googleapis/synthtool@1b9ad76
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:9db98b055a7f8bd82351238ccaacfd3cda58cdf73012ab58b8da146368330021
parthea pushed a commit that referenced this issue Sep 20, 2023
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
vchudnov-g pushed a commit that referenced this issue Sep 20, 2023
…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
parthea added a commit that referenced this issue Sep 22, 2023
….10.0 (#298)

Co-authored-by: Anthonios Partheniou <partheniou@google.com>
parthea pushed a commit that referenced this issue Sep 22, 2023
Source-Link: googleapis/synthtool@38e11ad
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:4e1991042fe54b991db9ca17c8fb386e61b22fe4d1472a568bf0fcac85dcf5d3
parthea pushed a commit that referenced this issue Sep 22, 2023
Source-Link: googleapis/synthtool@c4dd595
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:ce3c1686bc81145c81dd269bd12c4025c6b275b22d14641358827334fddb1d72
parthea pushed a commit that referenced this issue Sep 22, 2023
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
parthea added a commit that referenced this issue Sep 22, 2023
Co-authored-by: Anthonios Partheniou <partheniou@google.com>
parthea pushed a commit that referenced this issue Sep 22, 2023
Source-Link: https://togithub.com/googleapis/synthtool/commit/25083af347468dd5f90f69627420f7d452b6c50e
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:e6cbd61f1838d9ff6a31436dfc13717f372a7482a82fc1863ca954ec47bff8c8
parthea added a commit that referenced this issue Sep 22, 2023
* 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>
parthea pushed a commit that referenced this issue Sep 22, 2023
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>
parthea added a commit that referenced this issue Oct 21, 2023
* chore(tests): Request id test

* Fixing test formatting.

Co-authored-by: Anthonios Partheniou <partheniou@google.com>
parthea added a commit that referenced this issue Oct 21, 2023
Co-authored-by: Anthonios Partheniou <partheniou@google.com>
parthea added a commit that referenced this issue Oct 21, 2023
* 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>
parthea added a commit that referenced this issue Oct 21, 2023
…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>
parthea pushed a commit that referenced this issue Oct 21, 2023
* 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>
parthea added a commit that referenced this issue Oct 22, 2023
Co-authored-by: Anthonios Partheniou <partheniou@google.com>
parthea pushed a commit that referenced this issue Oct 22, 2023
Source-Link: https://togithub.com/googleapis/synthtool/commit/26c7505b2f76981ec1707b851e1595c8c06e90fc
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:f946c75373c2b0040e8e318c5e85d0cf46bc6e61d0a01f3ef94d8de974ac6790
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: datastore Issues related to the Datastore API. 🚨 This issue needs some love. triage me I really want to be triaged.
Projects
None yet
Development

No branches or pull requests

5 participants