Skip to content

Commit 1ada41f

Browse files
committed
minimizing copies
Signed-off-by: dorjesinpo <[email protected]>
1 parent 894fa9f commit 1ada41f

File tree

4 files changed

+143
-45
lines changed

4 files changed

+143
-45
lines changed

src/groups/bmq/bmqa/bmqa_messageproperties.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ class MessageProperties {
101101
/// Constant representing the maximum size of a
102102
/// `bmqp::MessageProperties` object, so that the below AlignedBuffer
103103
/// is big enough.
104-
static const int k_MAX_SIZEOF_BMQP_MESSAGEPROPERTIES = 184;
104+
static const int k_MAX_SIZEOF_BMQP_MESSAGEPROPERTIES = 280;
105105

106106
// PRIVATE TYPES
107107
typedef bsls::AlignedBuffer<k_MAX_SIZEOF_BMQP_MESSAGEPROPERTIES>

src/groups/bmq/bmqp/bmqp_messageproperties.cpp

Lines changed: 92 additions & 33 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,38 @@ 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+
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 (position.buffer() == end.buffer() ||
327+
(position.buffer() + 1 == end.buffer() && end.byte() == 0)) {
328+
// Section is good
329+
char* start = d_blob_p->buffer(position.buffer()).data() +
330+
position.byte();
331+
332+
p.d_value = bsl::string_view(start, p.d_length);
333+
doCopy = false;
334+
}
335+
}
336+
if (doCopy) {
337+
bsl::string value(p.d_length, ' ', d_allocator_p);
338+
rc = bmqu::BlobUtil::readNBytes(&value[0],
339+
*d_blob_p,
340+
position,
341+
p.d_length);
342+
p.d_value = value;
343+
}
308344

309-
p.d_value = value;
310345
break;
311346
}
312347
case bmqt::PropertyType::e_BINARY: {
313-
bsl::vector<char> value(p.d_length);
348+
bsl::vector<char> value(p.d_length, d_allocator_p);
314349
rc = bmqu::BlobUtil::readNBytes(&value[0],
315-
d_blob.object(),
350+
*d_blob_p,
316351
position,
317352
p.d_length);
318353

@@ -336,6 +371,7 @@ MessageProperties::MessageProperties(bslma::Allocator* basicAllocator)
336371
, d_totalSize(0)
337372
, d_originalSize(0)
338373
, d_blob()
374+
, d_blob_p(d_blob.address())
339375
, d_isBlobConstructed(false)
340376
, d_isDirty(true) // by default, this should be true
341377
, d_mphSize(0)
@@ -344,6 +380,7 @@ MessageProperties::MessageProperties(bslma::Allocator* basicAllocator)
344380
, d_dataOffset(0)
345381
, d_schema()
346382
, d_originalNumProps(0)
383+
, d_doDeepCopy(true)
347384
{
348385
}
349386

@@ -354,6 +391,7 @@ MessageProperties::MessageProperties(const MessageProperties& other,
354391
, d_totalSize(other.d_totalSize)
355392
, d_originalSize(other.d_originalSize)
356393
, d_blob()
394+
, d_blob_p(d_blob.address())
357395
, d_isBlobConstructed(false)
358396
, d_isDirty(other.d_isDirty)
359397
, d_mphSize(other.d_mphSize)
@@ -362,6 +400,7 @@ MessageProperties::MessageProperties(const MessageProperties& other,
362400
, d_dataOffset(other.d_dataOffset)
363401
, d_schema(other.d_schema)
364402
, d_originalNumProps(other.d_originalNumProps)
403+
, d_doDeepCopy(other.d_doDeepCopy)
365404
{
366405
if (other.d_isBlobConstructed) {
367406
new (d_blob.buffer())
@@ -394,6 +433,7 @@ MessageProperties& MessageProperties::operator=(const MessageProperties& rhs)
394433

395434
if (rhs.d_isBlobConstructed) {
396435
new (d_blob.buffer()) bdlbb::Blob(rhs.d_blob.object(), d_allocator_p);
436+
d_blob_p = d_blob.address();
397437
d_isBlobConstructed = true;
398438
}
399439

@@ -547,9 +587,15 @@ int MessageProperties::streamInHeader(const bdlbb::Blob& blob)
547587
return rc_INCORRECT_LENGTH; // RETURN
548588
}
549589

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;
590+
if (d_doDeepCopy) {
591+
new (d_blob.buffer()) bdlbb::Blob(d_allocator_p);
592+
bdlbb::BlobUtil::append(d_blob.address(), blob, 0, d_totalSize);
593+
d_blob_p = d_blob.address();
594+
d_isBlobConstructed = true;
595+
}
596+
else {
597+
d_blob_p = &blob;
598+
}
553599
d_originalSize = d_totalSize;
554600
d_originalNumProps = d_numProps;
555601

@@ -567,17 +613,18 @@ int MessageProperties::streamInPropertyHeader(Property* property,
567613
BSLS_ASSERT_SAFE(property);
568614
BSLS_ASSERT_SAFE(totalLength);
569615
BSLS_ASSERT_SAFE(d_dataOffset && start);
570-
BSLS_ASSERT_SAFE(d_isBlobConstructed);
616+
BSLS_ASSERT_SAFE(d_blob_p);
617+
BSLS_ASSERT_SAFE(d_isBlobConstructed || d_blob_p != d_blob.address());
571618

572619
bmqu::BlobPosition position;
573620

574-
if (bmqu::BlobUtil::findOffsetSafe(&position, d_blob.object(), start)) {
621+
if (bmqu::BlobUtil::findOffsetSafe(&position, *d_blob_p, start)) {
575622
// Failed to advance blob to next 'MessagePropertyHeader' location.
576623
return rc_NO_MSG_PROPERTY_HEADER; // RETURN
577624
}
578625

579626
bmqu::BlobObjectProxy<MessagePropertyHeader> mpHeader(
580-
&d_blob.object(),
627+
d_blob_p,
581628
position,
582629
d_mphSize,
583630
true, // read flag
@@ -693,14 +740,14 @@ int MessageProperties::streamInPropertyHeader(Property* property,
693740
name->assign(nameLen, ' ');
694741
bmqu::BlobPosition namePosition;
695742
int rc = bmqu::BlobUtil::findOffsetSafe(&namePosition,
696-
d_blob.object(),
743+
*d_blob_p,
697744
offset);
698745
if (rc) {
699746
return rc_MISSING_PROPERTY_AREA; // RETURN
700747
}
701748

702749
rc = bmqu::BlobUtil::readNBytes(name->begin(),
703-
d_blob.object(),
750+
*d_blob_p,
704751
namePosition,
705752
nameLen);
706753
if (rc) {
@@ -768,6 +815,9 @@ int MessageProperties::streamIn(const bdlbb::Blob& blob,
768815
return rc_MISSING_MSG_PROPERTY_HEADERS; // RETURN
769816
}
770817

818+
// TODO: This could always build schema on the fly if it is missing to
819+
// avoid extra iteration over properties in subsequent 'getSchema'.
820+
771821
rc = loadProperties(true, isNewStyleProperties);
772822
if (rc != rc_SUCCESS) {
773823
return rc; // RETURN
@@ -801,13 +851,14 @@ bdld::Datum
801851
MessageProperties::getPropertyRef(const bsl::string& name,
802852
bslma::Allocator* basicAllocator) const
803853
{
804-
PropertyMapConstIter cit = findProperty(name);
805-
if (cit == d_properties.end()) {
854+
PropertyMapIter it = findProperty(name);
855+
if (it == d_properties.end()) {
806856
return bdld::Datum::createError(-1); // RETURN
807857
}
858+
const Property& property = it->second;
808859

809-
const PropertyVariant& v = getPropertyValue(cit->second);
810-
switch (cit->second.d_type) {
860+
const PropertyVariant& v = getPropertyValue(property);
861+
switch (property.d_type) {
811862
case bmqt::PropertyType::e_BOOL:
812863
return bdld::Datum::createBoolean(v.the<bool>());
813864
case bmqt::PropertyType::e_CHAR:
@@ -820,8 +871,14 @@ MessageProperties::getPropertyRef(const bsl::string& name,
820871
return bdld::Datum::createInteger64(v.the<bsls::Types::Int64>(),
821872
basicAllocator);
822873
case bmqt::PropertyType::e_STRING:
823-
return bdld::Datum::createStringRef(v.the<bsl::string>(),
824-
basicAllocator);
874+
if (v.is<bsl::string>()) {
875+
return bdld::Datum::createStringRef(v.the<bsl::string>(),
876+
basicAllocator);
877+
}
878+
else {
879+
return bdld::Datum::createStringRef(v.the<bsl::string_view>(),
880+
basicAllocator);
881+
}
825882
case bmqt::PropertyType::e_BINARY:
826883
// do not want to use binary
827884
return bdld::Datum::createError(-2);
@@ -885,6 +942,8 @@ MessageProperties::streamOut(bdlbb::BlobBufferFactory* bufferFactory,
885942
}
886943

887944
new (d_blob.buffer()) bdlbb::Blob(bufferFactory, d_allocator_p);
945+
946+
d_blob_p = d_blob.address();
888947
d_isBlobConstructed = true;
889948

890949
if (0 == numProperties()) {
@@ -1006,7 +1065,7 @@ MessageProperties::streamOut(bdlbb::BlobBufferFactory* bufferFactory,
10061065
msgPropsHdr.reset();
10071066
d_isDirty = false;
10081067

1009-
return d_blob.object();
1068+
return *d_blob_p;
10101069
}
10111070

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

0 commit comments

Comments
 (0)