Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

chore(Spanner): Make createTransactionSelector faster#7342

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
saranshdhingra wants to merge4 commits intomain
base:main
Choose a base branch
Loading
fromspanner-replace-decodemsg

Conversation

@saranshdhingra
Copy link
Contributor

This PR simply replaces the use ofdecodeMessage from the Serializer and uses themergeFromJsonString and the protobuf class's setter method.

The metrics on average are:

Using decodeMessage

WhencreateTransactionSelector is called the first time: 900 us(micros)
WhencreateTransactionSelector is called the second time: 300 us (This is low because the Serializer caches a lot of things like proto descriptors etc).

Using mergeFromJsonString

WhencreateTransactionSelector is called the first time: 200 us(micros)
WhencreateTransactionSelector is called the second time: 150 us

@saranshdhingrasaranshdhingra requested review froma team ascode ownersMay 24, 2024 11:43
@product-auto-labelproduct-auto-labelbot added the api: spannerIssues related to the Spanner API. labelMay 24, 2024
@saranshdhingrasaranshdhingra added the do not mergeIndicates a pull request not ready for merge, due to either quality or timing. labelMay 24, 2024
@saranshdhingra
Copy link
ContributorAuthor

Seems like protobuf'smergeFromString doesn't work with the following:
{"begin": {"readWrite": []}} whereas it works with:{"begin": {"readWrite": {}}}.

For now, closing this PR.

@bshaffer
Copy link
Contributor

Seems like protobuf's mergeFromString doesn't work with the following:
{"begin": {"readWrite": []}} whereas it works with: {"begin": {"readWrite": {}}}.

I have made sure that the JSON string uses{} for empty arrays by providing theJSON_FORCE_OBJECT flag, but the emulator tests are still failing. This will require further investigation.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@bshafferbshafferbshaffer left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

api: spannerIssues related to the Spanner API.do not mergeIndicates a pull request not ready for merge, due to either quality or timing.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@saranshdhingra@bshaffer
[8]ページ先頭
©2009-2025 Movatter.jp