This repository was archived by the owner on Sep 26, 2023. It is now read-only.
BREAKING: bundling: optimize partion key#241
Merged
garrettjonesgoogle merged 7 commits intogoogleapis:masterfrom Mar 31, 2017
pongad:partition-key
Merged
BREAKING: bundling: optimize partion key#241garrettjonesgoogle merged 7 commits intogoogleapis:masterfrom pongad:partition-key
garrettjonesgoogle merged 7 commits intogoogleapis:masterfrom
pongad:partition-key
Conversation
Partition key is used in bundling methods to determine whether two method calls can be merged into one RPC request. Previously this key is a string. This is inefficient, as a new string needs to be created every time a bundling method is called. This commit replaces the string with the PartitionKey type. Since protobuf objects have value-equality, PartitionKey instances simply compare equal if their constituents are equal and in the same order. In a simple load-test, this decreases the time required to write 100,000 log messages to Stackdriver from 9.6s to 5.4s and reduces CPU consumption from 136% to 117%. I have not compared this with Pubsub's implementation, so it is possible that we might need to change bundling at a more fundamental level. Posting this PR for early feedback and to safeguard against my machine dying.
Codecov Report
@@ Coverage Diff @@
## master #241 +/- ##
============================================
- Coverage 70.89% 70.87% -0.02%
- Complexity 541 547 +6
============================================
Files 78 79 +1
Lines 2680 2699 +19
Branches 268 273 +5
============================================
+ Hits 1900 1913 +13
- Misses 676 679 +3
- Partials 104 107 +3
Continue to review full report at Codecov.
|
Contributor
|
This looks good to me. Very interesting that we get such a large performance increase just from the partition key!
😰 |
Contributor
|
LGTM also, after toolkit is updated and client code is regenerated. |
pongad
changed the title
michaelbausor
approved these changes
Mar 21, 2017
pongad
added a commit
to googleapis/gapic-generator
that referenced
this pull request
Mar 21, 2017
This change is still a breaking change, but it is just compatible enough that GAPIC-generated clients will compile and work properly. Toolkit will be updated to take advantage of the new PartitionKey endpoint. Once all clients are migrated, we can get rid of the old String endpoint.
| * Returns the value of the partition key for the given request. | ||
| */ | ||
| String getBatchPartitionKey(RequestT request); | ||
| protected String getBatchPartitionKey(RequestT request) { |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
pongad
changed the title
This reverts commit 9d1f090.
Contributor
Author
|
As discussed, we'll keep this a breaking change and "atomically" update toolkit and clients |
Contributor
|
LGTM |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Partition key is used in bundling methods to determine
whether two method calls can be merged into one RPC request.
Previously this key is a string.
This is inefficient, as a new string needs to be created
every time a bundling method is called.
This commit replaces the string with the PartitionKey type.
Since protobuf objects have value-equality,
PartitionKey instances simply compare equal if their
constituents are equal and in the same order.
In a simple load-test, this decreases the time required to
write 100,000 log messages to Stackdriver from
9.6s to 5.4s and reduces CPU consumption from 136% to 117%.
I have not compared this with Pubsub's implementation,
so it is possible that we might need to change bundling
at a more fundamental level.
Posting this PR for early feedback and to safeguard against
my machine dying.