Datastore: expose serializedSize for Entity. #6842
Conversation
| return properties; | ||
| } | ||
|
|
||
| /** Returns the entity size. */ |
There was a problem hiding this comment.
"Returns the number of bytes in the serialized protobuf."
Is this definitely a fixed and reproducible value for a given protobuf?
There was a problem hiding this comment.
Yes,Once build it would be fixed number.
| } | ||
|
|
||
| @Test | ||
| public void testEntitySize() throws Exception { |
There was a problem hiding this comment.
Does this really throw exception?
There was a problem hiding this comment.
No,This method does not throw exception.
| BaseEntity(Builder<K, ?> builder) { | ||
| this.key = builder.key; | ||
| this.properties = ImmutableSortedMap.copyOf(builder.properties); | ||
| this.serializedSize = this.toPb().getSerializedSize(); |
There was a problem hiding this comment.
what's the cost of calculating this for a large entity? Most of the time we don't need this value. Perhaps this could be done in demand and not stored or only lazily stored in the field?
There was a problem hiding this comment.
The cost could be varied it depends on user entered properties. Does this look fine?
/** Returns the number of bytes in the serialized protobuf. */
public int getSerializedSize() {
if (serializedSize == 0) {
serializedSize = this.toPb().getSerializedSize();
}
return serializedSize;
}Here we are calculating entity size when demand.
Codecov Report
@@ Coverage Diff @@
## master #6842 +/- ##
=============================================
- Coverage 45.72% 45.28% -0.45%
+ Complexity 26528 6251 -20277
=============================================
Files 2615 453 -2162
Lines 282766 56282 -226484
Branches 32745 7229 -25516
=============================================
- Hits 129299 25486 -103813
+ Misses 143350 29140 -114210
+ Partials 10117 1656 -8461
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
I've spent some time thinking about this and I feel like this is more of a utility method than a property of the entity. Entities are immutable, but in order to not incur a penalty of adding making size available serializedSize would need to be non-final which opens up other issues.
Instead of this being a field with a getter, can you add a new static method to com.google.cloud.datastore.Entity:
public static int calculateSerializedSize(BaseEntity<? extends IncompleteKey> e) {
return e.toPb().getSerializedSize();
}This way we don't impose a penalty to those users that don't need this operation.
| return new Builder().fill(entityPb).build(); | ||
| } | ||
|
|
||
| public static int calculateSerializedSize(BaseEntity<? extends IncompleteKey> e) { |
There was a problem hiding this comment.
Please add a javadoc comment to explain the usage of this method (and to help people find it when they google for this type of thing).
| /** | ||
| * Returns the serialized protobuf size of provided entity object. | ||
| * | ||
| * @param entity entity object to calculate serialize size. |
There was a problem hiding this comment.
"object whose size is measured"
| checkNotNull(e); | ||
| return e.toPb().getSerializedSize(); | ||
| /** | ||
| * Returns the serialized protobuf size of provided entity object. |
There was a problem hiding this comment.
"Returns the size in bytes of the protobuf form of the provided entity."
Fixes #6389