Skip to content

Commit 87ddacb

Browse files
dorjesinpodorjesinpo
authored
Perf[BMQP]: minimizing copies in MessageProperties (#1014)
Signed-off-by: dorjesinpo <129227380+dorjesinpo@users.noreply.github.com>
1 parent 7ab4c13 commit 87ddacb

4 files changed

Lines changed: 165 additions & 58 deletions

File tree

‎src/groups/bmq/bmqp/bmqp_messageproperties.cpp‎

Lines changed: 95 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -104,10 +104,10 @@ class PropertyValueStreamOutVisitor {
104104
sizeof(nboValue));
105105
}
106106

107-
void operator()(const bsl::string& value)
107+
void operator()(const bsl::string_view& value)
108108
{
109109
bdlbb::BlobUtil::append(d_blob_p,
110-
value.c_str(),
110+
value.data(),
111111
static_cast<int>(value.length()));
112112
}
113113

@@ -234,6 +234,24 @@ MessageProperties::getPropertyValue(const Property& property) const
234234
return property.d_value;
235235
}
236236

237+
const MessageProperties::PropertyVariant&
238+
MessageProperties::getPropertyValueAsString(const Property& property) const
239+
{
240+
if (property.d_value.isUnset()) {
241+
BSLA_MAYBE_UNUSED bool result = streamInPropertyValue(property);
242+
BSLS_ASSERT_SAFE(result);
243+
// We assert 'true' result because the length and offset have already
244+
// been checked.
245+
}
246+
PropertyVariant& v = property.d_value;
247+
248+
if (v.is<bsl::string_view>()) {
249+
v = bsl::string(v.the<bsl::string_view>(), d_allocator_p);
250+
}
251+
252+
return v;
253+
}
254+
237255
bool MessageProperties::streamInPropertyValue(const Property& p) const
238256
{
239257
// PRECONDITIONS
@@ -242,16 +260,14 @@ bool MessageProperties::streamInPropertyValue(const Property& p) const
242260
BSLS_ASSERT_SAFE(p.d_offset);
243261

244262
bmqu::BlobPosition position;
245-
int rc = bmqu::BlobUtil::findOffsetSafe(&position,
246-
d_blob.object(),
247-
p.d_offset);
263+
int rc = bmqu::BlobUtil::findOffsetSafe(&position, *d_blob_p, p.d_offset);
248264
BSLS_ASSERT_SAFE(rc == 0);
249265

250266
switch (p.d_type) {
251267
case bmqt::PropertyType::e_BOOL: {
252268
char value;
253269
rc = bmqu::BlobUtil::readNBytes(&value,
254-
d_blob.object(),
270+
*d_blob_p,
255271
position,
256272
sizeof(value));
257273

@@ -261,7 +277,7 @@ bool MessageProperties::streamInPropertyValue(const Property& p) const
261277
case bmqt::PropertyType::e_CHAR: {
262278
char value;
263279
rc = bmqu::BlobUtil::readNBytes(&value,
264-
d_blob.object(),
280+
*d_blob_p,
265281
position,
266282
sizeof(value));
267283

@@ -271,7 +287,7 @@ bool MessageProperties::streamInPropertyValue(const Property& p) const
271287
case bmqt::PropertyType::e_SHORT: {
272288
bdlb::BigEndianInt16 nboValue;
273289
rc = bmqu::BlobUtil::readNBytes(reinterpret_cast<char*>(&nboValue),
274-
d_blob.object(),
290+
*d_blob_p,
275291
position,
276292
sizeof(nboValue));
277293

@@ -281,7 +297,7 @@ bool MessageProperties::streamInPropertyValue(const Property& p) const
281297
case bmqt::PropertyType::e_INT32: {
282298
bdlb::BigEndianInt32 nboValue;
283299
rc = bmqu::BlobUtil::readNBytes(reinterpret_cast<char*>(&nboValue),
284-
d_blob.object(),
300+
*d_blob_p,
285301
position,
286302
sizeof(nboValue));
287303

@@ -291,7 +307,7 @@ bool MessageProperties::streamInPropertyValue(const Property& p) const
291307
case bmqt::PropertyType::e_INT64: {
292308
bdlb::BigEndianInt64 nboValue;
293309
rc = bmqu::BlobUtil::readNBytes(reinterpret_cast<char*>(&nboValue),
294-
d_blob.object(),
310+
*d_blob_p,
295311
position,
296312
sizeof(nboValue));
297313

@@ -300,19 +316,37 @@ bool MessageProperties::streamInPropertyValue(const Property& p) const
300316
}
301317

302318
case bmqt::PropertyType::e_STRING: {
303-
bsl::string value(p.d_length, ' ');
304-
rc = bmqu::BlobUtil::readNBytes(&value[0],
305-
d_blob.object(),
306-
position,
307-
p.d_length);
319+
// Try to avoid copying the string. 'd_blop' already keeps a copy.
320+
bmqu::BlobPosition end;
321+
const int ret =
322+
bmqu::BlobUtil::findOffset(&end, *d_blob_p, position, p.d_length);
323+
bool doCopy = true;
324+
if (ret == 0) {
325+
// Do not align
326+
if (bmqu::BlobUtil::isDataContinuous(position, end)) {
327+
// Section is good
328+
char* start = d_blob_p->buffer(position.buffer()).data() +
329+
position.byte();
330+
331+
p.d_value = bsl::string_view(start, p.d_length);
332+
doCopy = false;
333+
}
334+
}
335+
if (doCopy) {
336+
bsl::string value(p.d_length, ' ', d_allocator_p);
337+
rc = bmqu::BlobUtil::readNBytes(&value[0],
338+
*d_blob_p,
339+
position,
340+
p.d_length);
341+
p.d_value = value;
342+
}
308343

309-
p.d_value = value;
310344
break;
311345
}
312346
case bmqt::PropertyType::e_BINARY: {
313-
bsl::vector<char> value(p.d_length);
347+
bsl::vector<char> value(p.d_length, d_allocator_p);
314348
rc = bmqu::BlobUtil::readNBytes(&value[0],
315-
d_blob.object(),
349+
*d_blob_p,
316350
position,
317351
p.d_length);
318352

@@ -336,14 +370,16 @@ MessageProperties::MessageProperties(bslma::Allocator* basicAllocator)
336370
, d_totalSize(0)
337371
, d_originalSize(0)
338372
, d_blob()
339-
, d_isBlobConstructed(false)
340-
, d_isDirty(true) // by default, this should be true
373+
, d_blob_p(d_blob.address())
341374
, d_mphSize(0)
342375
, d_mphOffset(0)
343376
, d_numProps(0)
344377
, d_dataOffset(0)
345378
, d_schema()
346379
, d_originalNumProps(0)
380+
, d_isBlobConstructed(false)
381+
, d_isDirty(true) // by default, this should be true
382+
, d_doDeepCopy(true)
347383
{
348384
}
349385

@@ -354,14 +390,16 @@ MessageProperties::MessageProperties(const MessageProperties& other,
354390
, d_totalSize(other.d_totalSize)
355391
, d_originalSize(other.d_originalSize)
356392
, d_blob()
357-
, d_isBlobConstructed(false)
358-
, d_isDirty(other.d_isDirty)
393+
, d_blob_p(d_blob.address())
359394
, d_mphSize(other.d_mphSize)
360395
, d_mphOffset(other.d_mphOffset)
361396
, d_numProps(other.d_numProps)
362397
, d_dataOffset(other.d_dataOffset)
363398
, d_schema(other.d_schema)
364399
, d_originalNumProps(other.d_originalNumProps)
400+
, d_isBlobConstructed(false)
401+
, d_isDirty(other.d_isDirty)
402+
, d_doDeepCopy(other.d_doDeepCopy)
365403
{
366404
if (other.d_isBlobConstructed) {
367405
new (d_blob.buffer())
@@ -394,6 +432,7 @@ MessageProperties& MessageProperties::operator=(const MessageProperties& rhs)
394432

395433
if (rhs.d_isBlobConstructed) {
396434
new (d_blob.buffer()) bdlbb::Blob(rhs.d_blob.object(), d_allocator_p);
435+
d_blob_p = d_blob.address();
397436
d_isBlobConstructed = true;
398437
}
399438

@@ -547,9 +586,15 @@ int MessageProperties::streamInHeader(const bdlbb::Blob& blob)
547586
return rc_INCORRECT_LENGTH; // RETURN
548587
}
549588

550-
new (d_blob.buffer()) bdlbb::Blob(d_allocator_p);
551-
bdlbb::BlobUtil::append(d_blob.address(), blob, 0, d_totalSize);
552-
d_isBlobConstructed = true;
589+
if (d_doDeepCopy) {
590+
new (d_blob.buffer()) bdlbb::Blob(d_allocator_p);
591+
bdlbb::BlobUtil::append(d_blob.address(), blob, 0, d_totalSize);
592+
d_blob_p = d_blob.address();
593+
d_isBlobConstructed = true;
594+
}
595+
else {
596+
d_blob_p = &blob;
597+
}
553598
d_originalSize = d_totalSize;
554599
d_originalNumProps = d_numProps;
555600

@@ -567,17 +612,18 @@ int MessageProperties::streamInPropertyHeader(Property* property,
567612
BSLS_ASSERT_SAFE(property);
568613
BSLS_ASSERT_SAFE(totalLength);
569614
BSLS_ASSERT_SAFE(d_dataOffset && start);
570-
BSLS_ASSERT_SAFE(d_isBlobConstructed);
615+
BSLS_ASSERT_SAFE(d_blob_p);
616+
BSLS_ASSERT_SAFE(d_isBlobConstructed || d_blob_p != d_blob.address());
571617

572618
bmqu::BlobPosition position;
573619

574-
if (bmqu::BlobUtil::findOffsetSafe(&position, d_blob.object(), start)) {
620+
if (bmqu::BlobUtil::findOffsetSafe(&position, *d_blob_p, start)) {
575621
// Failed to advance blob to next 'MessagePropertyHeader' location.
576622
return rc_NO_MSG_PROPERTY_HEADER; // RETURN
577623
}
578624

579625
bmqu::BlobObjectProxy<MessagePropertyHeader> mpHeader(
580-
&d_blob.object(),
626+
d_blob_p,
581627
position,
582628
d_mphSize,
583629
true, // read flag
@@ -693,14 +739,14 @@ int MessageProperties::streamInPropertyHeader(Property* property,
693739
name->assign(nameLen, ' ');
694740
bmqu::BlobPosition namePosition;
695741
int rc = bmqu::BlobUtil::findOffsetSafe(&namePosition,
696-
d_blob.object(),
742+
*d_blob_p,
697743
offset);
698744
if (rc) {
699745
return rc_MISSING_PROPERTY_AREA; // RETURN
700746
}
701747

702748
rc = bmqu::BlobUtil::readNBytes(name->begin(),
703-
d_blob.object(),
749+
*d_blob_p,
704750
namePosition,
705751
nameLen);
706752
if (rc) {
@@ -768,6 +814,9 @@ int MessageProperties::streamIn(const bdlbb::Blob& blob,
768814
return rc_MISSING_MSG_PROPERTY_HEADERS; // RETURN
769815
}
770816

817+
// TODO: This could always build schema on the fly if it is missing to
818+
// avoid extra iteration over properties in subsequent 'getSchema'.
819+
771820
rc = loadProperties(true, isNewStyleProperties);
772821
if (rc != rc_SUCCESS) {
773822
return rc; // RETURN
@@ -801,13 +850,14 @@ bdld::Datum
801850
MessageProperties::getPropertyRef(const bsl::string& name,
802851
bslma::Allocator* basicAllocator) const
803852
{
804-
PropertyMapConstIter cit = findProperty(name);
805-
if (cit == d_properties.end()) {
853+
PropertyMapIter it = findProperty(name);
854+
if (it == d_properties.end()) {
806855
return bdld::Datum::createError(-1); // RETURN
807856
}
857+
const Property& property = it->second;
808858

809-
const PropertyVariant& v = getPropertyValue(cit->second);
810-
switch (cit->second.d_type) {
859+
const PropertyVariant& v = getPropertyValue(property);
860+
switch (property.d_type) {
811861
case bmqt::PropertyType::e_BOOL:
812862
return bdld::Datum::createBoolean(v.the<bool>());
813863
case bmqt::PropertyType::e_CHAR:
@@ -820,8 +870,14 @@ MessageProperties::getPropertyRef(const bsl::string& name,
820870
return bdld::Datum::createInteger64(v.the<bsls::Types::Int64>(),
821871
basicAllocator);
822872
case bmqt::PropertyType::e_STRING:
823-
return bdld::Datum::createStringRef(v.the<bsl::string>(),
824-
basicAllocator);
873+
if (v.is<bsl::string>()) {
874+
return bdld::Datum::createStringRef(v.the<bsl::string>(),
875+
basicAllocator);
876+
}
877+
else {
878+
return bdld::Datum::createStringRef(v.the<bsl::string_view>(),
879+
basicAllocator);
880+
}
825881
case bmqt::PropertyType::e_BINARY:
826882
// do not want to use binary
827883
return bdld::Datum::createError(-2);
@@ -885,6 +941,8 @@ MessageProperties::streamOut(bdlbb::BlobBufferFactory* bufferFactory,
885941
}
886942

887943
new (d_blob.buffer()) bdlbb::Blob(bufferFactory, d_allocator_p);
944+
945+
d_blob_p = d_blob.address();
888946
d_isBlobConstructed = true;
889947

890948
if (0 == numProperties()) {
@@ -1006,7 +1064,7 @@ MessageProperties::streamOut(bdlbb::BlobBufferFactory* bufferFactory,
10061064
msgPropsHdr.reset();
10071065
d_isDirty = false;
10081066

1009-
return d_blob.object();
1067+
return *d_blob_p;
10101068
}
10111069

10121070
bsl::ostream& MessageProperties::print(bsl::ostream& stream,

0 commit comments

Comments
 (0)