Skip to content

Commit b4de9f2

Browse files
committed
BUG#27463149 - SCALABILITY ISSUE WITH XPLUGINS MESSAG_BUILDER
Description =========== Reception of a resultset in X Protocol consists of multiple protocol messages. X Plugin allocates CodedOutputStream on heap for each protocol message. The allocation degrades performance: * PFS OFF - degradation around 3% for point_select test * PFS ON - degradation around 100%-300% for point_select test Fix === The memory region for CodedOutputStream is allocated once, and for each message the object is created by placement new operator. RB:18975 Reviewed-by: Tomasz Stepniak <[email protected]> Reviewed-by: Andrzej Religa <[email protected]>
1 parent 1033c68 commit b4de9f2

File tree

4 files changed

+44
-22
lines changed

4 files changed

+44
-22
lines changed

plugin/x/ngs/include/ngs/protocol/message_builder.h

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2016, 2017, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights reserved.
33
*
44
* This program is free software; you can redistribute it and/or modify
55
* it under the terms of the GNU General Public License, version 2.0,
@@ -25,8 +25,10 @@
2525
#ifndef _NGS_MESSAGE_BUILDER_H_
2626
#define _NGS_MESSAGE_BUILDER_H_
2727

28+
#include <memory>
2829
#include "m_ctype.h"
2930
#include "my_inttypes.h"
31+
3032
#include "plugin/x/ngs/include/ngs/memory.h"
3133
#include "plugin/x/ngs/include/ngs_common/protocol_protobuf.h"
3234

@@ -43,15 +45,19 @@ class Message_builder {
4345

4446
protected:
4547
using CodedOutputStream = ::google::protobuf::io::CodedOutputStream;
46-
using CodedOutputStream_ptr =
47-
ngs::Memory_instrumented<CodedOutputStream>::Unique_ptr;
48+
using Stream_allocator = std::allocator<CodedOutputStream>;
4849

4950
protected:
5051
void start_message(Output_buffer *out_buffer, const uint8 type);
5152
void end_message();
5253

54+
void construct_stream();
55+
void reset_stream();
56+
5357
Output_buffer *m_out_buffer;
54-
CodedOutputStream_ptr m_out_stream;
58+
CodedOutputStream *m_out_stream;
59+
bool m_valid_out_stream{false};
60+
5561
int m_size_addr2_size;
5662
int m_field_number;
5763

plugin/x/ngs/src/protocol/message_builder.cc

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2015, 2017, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2015, 2018, Oracle and/or its affiliates. All rights reserved.
33
*
44
* This program is free software; you can redistribute it and/or modify
55
* it under the terms of the GNU General Public License, version 2.0,
@@ -30,9 +30,12 @@
3030

3131
namespace ngs {
3232

33-
Message_builder::Message_builder() : m_out_buffer(NULL) {}
33+
Message_builder::Message_builder()
34+
: m_out_buffer(nullptr), m_out_stream(Stream_allocator().allocate(1)) {}
3435

35-
Message_builder::~Message_builder() {}
36+
Message_builder::~Message_builder() {
37+
Stream_allocator().deallocate(m_out_stream, 1);
38+
}
3639

3740
void Message_builder::encode_uint32(const uint32 value, const bool write) {
3841
++m_field_number;
@@ -41,7 +44,7 @@ void Message_builder::encode_uint32(const uint32 value, const bool write) {
4144
google::protobuf::internal::WireFormatLite::WriteTag(
4245
m_field_number,
4346
google::protobuf::internal::WireFormatLite::WIRETYPE_VARINT,
44-
m_out_stream.get());
47+
m_out_stream);
4548
m_out_stream->WriteVarint32(value);
4649
}
4750
}
@@ -53,7 +56,7 @@ void Message_builder::encode_uint64(const uint64 value, const bool write) {
5356
google::protobuf::internal::WireFormatLite::WriteTag(
5457
m_field_number,
5558
google::protobuf::internal::WireFormatLite::WIRETYPE_VARINT,
56-
m_out_stream.get());
59+
m_out_stream);
5760
m_out_stream->WriteVarint64(value);
5861
}
5962
}
@@ -64,7 +67,7 @@ void Message_builder::encode_int32(const int32 value, const bool write) {
6467
google::protobuf::internal::WireFormatLite::WriteTag(
6568
m_field_number,
6669
google::protobuf::internal::WireFormatLite::WIRETYPE_VARINT,
67-
m_out_stream.get());
70+
m_out_stream);
6871
m_out_stream->WriteVarint32SignExtended(value);
6972
}
7073
}
@@ -77,7 +80,7 @@ void Message_builder::encode_string(const char *value, const size_t len,
7780
google::protobuf::internal::WireFormatLite::WriteTag(
7881
m_field_number,
7982
google::protobuf::internal::WireFormatLite::WIRETYPE_LENGTH_DELIMITED,
80-
m_out_stream.get());
83+
m_out_stream);
8184
m_out_stream->WriteVarint32(static_cast<google::protobuf::uint32>(len));
8285
m_out_stream->WriteRaw(value, static_cast<int>(len));
8386
}
@@ -87,6 +90,18 @@ void Message_builder::encode_string(const char *value, const bool write) {
8790
encode_string(value, write ? strlen(value) : 0, write);
8891
}
8992

93+
void Message_builder::construct_stream() {
94+
if (m_valid_out_stream) reset_stream();
95+
96+
Stream_allocator().construct(m_out_stream, m_out_buffer);
97+
m_valid_out_stream = true;
98+
}
99+
100+
void Message_builder::reset_stream() {
101+
Stream_allocator().destroy(m_out_stream);
102+
m_valid_out_stream = false;
103+
}
104+
90105
void Message_builder::start_message(Output_buffer *out_buffer,
91106
const uint8 type) {
92107
m_field_number = 0;
@@ -95,7 +110,8 @@ void Message_builder::start_message(Output_buffer *out_buffer,
95110
m_out_buffer->save_state();
96111
m_out_buffer->reserve(5);
97112
m_start_from = static_cast<uint32>(m_out_buffer->ByteCount());
98-
m_out_stream.reset(ngs::allocate_object<CodedOutputStream>(m_out_buffer));
113+
114+
construct_stream();
99115

100116
// at this point we don't know the size but we need to reserve the space for
101117
// it it is possible that the size which is stored on 4-bytes will be split
@@ -128,7 +144,7 @@ void Message_builder::end_message() {
128144
// here we already know the buffer size, so write it at the beginning of the
129145
// buffer the order is important here as the stream's destructor calls
130146
// buffer's BackUp() validating ByteCount
131-
m_out_stream.reset();
147+
reset_stream();
132148

133149
uint32 msg_size = static_cast<uint32>(m_out_buffer->ByteCount()) -
134150
m_start_from - sizeof(google::protobuf::uint32);

plugin/x/ngs/src/protocol/notice_builder.cc

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2015, 2017, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2015, 2018, Oracle and/or its affiliates. All rights reserved.
33
*
44
* This program is free software; you can redistribute it and/or modify
55
* it under the terms of the GNU General Public License, version 2.0,
@@ -58,7 +58,7 @@ void Notice_builder::encode_rows_affected(Output_buffer *out_buffer,
5858
// 3) Payload
5959
google::protobuf::internal::WireFormatLite::WriteTag(
6060
3, google::protobuf::internal::WireFormatLite::WIRETYPE_LENGTH_DELIMITED,
61-
m_out_stream.get());
61+
m_out_stream);
6262
uint32 size_scalar = CodedOutputStream::VarintSize32SignExtended(type) +
6363
CodedOutputStream::VarintSize64(value) + 2 /*tags*/;
6464
uint32 size_payload =
@@ -74,7 +74,7 @@ void Notice_builder::encode_rows_affected(Output_buffer *out_buffer,
7474
google::protobuf::internal::WireFormatLite::WriteTag(
7575
2,
7676
google::protobuf::internal::WireFormatLite::WIRETYPE_LENGTH_DELIMITED,
77-
m_out_stream.get());
77+
m_out_stream);
7878
m_out_stream->WriteVarint32(size_scalar);
7979
{
8080
m_field_number = 0;

plugin/x/ngs/src/protocol/row_builder.cc

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2015, 2017, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2015, 2018, Oracle and/or its affiliates. All rights reserved.
33
*
44
* This program is free software; you can redistribute it and/or modify
55
* it under the terms of the GNU General Public License, version 2.0,
@@ -46,7 +46,7 @@ using namespace ngs;
4646
google::protobuf::internal::WireFormatLite::WriteTag( \
4747
1, \
4848
google::protobuf::internal::WireFormatLite::WIRETYPE_LENGTH_DELIMITED, \
49-
m_out_stream.get()); \
49+
m_out_stream); \
5050
++m_num_fields;
5151

5252
Row_builder::Row_builder() : m_row_processing(false) {}
@@ -55,7 +55,7 @@ Row_builder::~Row_builder() { abort_row(); }
5555

5656
void Row_builder::abort_row() {
5757
if (m_row_processing) {
58-
m_out_stream.reset();
58+
reset_stream();
5959
m_out_buffer->rollback();
6060
m_row_processing = false;
6161
}
@@ -368,7 +368,7 @@ void Row_builder::add_time_field(const MYSQL_TIME *value, uint) {
368368
google::protobuf::uint8 neg = (value->neg) ? 0x01 : 0x00;
369369
m_out_stream->WriteRaw(&neg, 1);
370370

371-
append_time_values(value, m_out_stream.get());
371+
append_time_values(value, m_out_stream);
372372
}
373373

374374
void Row_builder::add_datetime_field(const MYSQL_TIME *value, uint) {
@@ -386,15 +386,15 @@ void Row_builder::add_datetime_field(const MYSQL_TIME *value, uint) {
386386
m_out_stream->WriteVarint64(value->month);
387387
m_out_stream->WriteVarint64(value->day);
388388

389-
append_time_values(value, m_out_stream.get());
389+
append_time_values(value, m_out_stream);
390390
}
391391

392392
void Row_builder::add_string_field(const char *const value, size_t length,
393393
const CHARSET_INFO *const) {
394394
ADD_FIELD_HEADER();
395395

396396
m_out_stream->WriteVarint32(static_cast<google::protobuf::uint32>(
397-
length + 1)); // 1 byte for thre trailing '\0'
397+
length + 1)); // 1 byte for the trailing '\0'
398398

399399
m_out_stream->WriteRaw(value, static_cast<int>(length));
400400
char zero = '\0';

0 commit comments

Comments
 (0)