Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve memory usage by move ZMQ serialize buffer from ZmqProducerStateTable to ZmqClient #955

Merged
merged 2 commits into from
Nov 25, 2024
Merged

Improve memory usage by move ZMQ serialize buffer from ZmqProducerStateTable to ZmqClient #955

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a3233ca
Improve memory usage by move ZMQ serialize buffer from ZmqProducerSta…
liuh-80 Nov 23, 2024
102b3af
Update zmqproducerstatetable.cpp
liuh-80 Nov 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions common/c-api/zmqclient.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,7 @@ void SWSSZmqClient_sendMsg(SWSSZmqClient zmqc, const char *dbName, const char *t
SWSSKeyOpFieldValuesArray arr) {
SWSSTry({
vector<KeyOpFieldsValuesTuple> kcos = takeKeyOpFieldValuesArray(arr);
size_t bufSize = BinarySerializer::serializedSize(dbName, tableName, kcos);
vector<char> v(bufSize);
((ZmqClient *)zmqc)
->sendMsg(string(dbName), string(tableName), kcos, v);
->sendMsg(string(dbName), string(tableName), kcos);
});
}
10 changes: 5 additions & 5 deletions common/zmqclient.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ void ZmqClient::initialize(const std::string& endpoint, const std::string& vrf)
m_context = nullptr;
m_socket = nullptr;
m_vrf = vrf;
m_sendbuffer.resize(MQ_RESPONSE_MAX_COUNT);

connect();
}
Expand Down Expand Up @@ -116,12 +117,11 @@ void ZmqClient::connect()
void ZmqClient::sendMsg(
const std::string& dbName,
const std::string& tableName,
const std::vector<KeyOpFieldsValuesTuple>& kcos,
std::vector<char>& sendbuffer)
const std::vector<KeyOpFieldsValuesTuple>& kcos)
{
int serializedlen = (int)BinarySerializer::serializeBuffer(
sendbuffer.data(),
sendbuffer.size(),
m_sendbuffer.data(),
m_sendbuffer.size(),
dbName,
tableName,
kcos);
Expand All @@ -144,7 +144,7 @@ void ZmqClient::sendMsg(
std::lock_guard<std::mutex> lock(m_socketMutex);

// Use none block mode to use all bandwidth: http://api.zeromq.org/2-1%3Azmq-send
rc = zmq_send(m_socket, sendbuffer.data(), serializedlen, ZMQ_NOBLOCK);
rc = zmq_send(m_socket, m_sendbuffer.data(), serializedlen, ZMQ_NOBLOCK);
}

if (rc >= 0)
Expand Down
5 changes: 3 additions & 2 deletions common/zmqclient.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ class ZmqClient

void sendMsg(const std::string& dbName,
const std::string& tableName,
const std::vector<KeyOpFieldsValuesTuple>& kcos,
std::vector<char>& sendbuffer);
const std::vector<KeyOpFieldsValuesTuple>& kcos);
private:
void initialize(const std::string& endpoint, const std::string& vrf);

Expand All @@ -38,6 +37,8 @@ class ZmqClient
bool m_connected;

std::mutex m_socketMutex;

std::vector<char> m_sendbuffer;
};

}
17 changes: 5 additions & 12 deletions common/zmqproducerstatetable.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@ ZmqProducerStateTable::ZmqProducerStateTable(RedisPipeline *pipeline, const stri

void ZmqProducerStateTable::initialize(DBConnector *db, const std::string &tableName, bool dbPersistence)
{
m_sendbuffer.resize(MQ_RESPONSE_MAX_COUNT);
Copy link
Contributor Author

@liuh-80 liuh-80 Nov 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line cause GNMI memory issue, it will allocate 16MB memory for every ZmqProducerStateTable.

However, by add debug log, the dtor of invoked by GNMI. so this memory should be release.

However, the GNMI service memory will keep fast increase until 1.3GB.

Seems it's related with GO memory management or go SWIG wrapper: some memory allocates and released by C++ not free in GO side.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand, if dtor is invoked, why is this memory not released?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why, this can only reproduce on starlab


if (dbPersistence)
{
SWSS_LOG_DEBUG("Database persistence enabled, tableName: %s", tableName.c_str());
Expand All @@ -64,8 +62,7 @@ void ZmqProducerStateTable::set(
m_zmqClient.sendMsg(
m_dbName,
m_tableNameStr,
kcos,
m_sendbuffer);
kcos);

if (m_asyncDBUpdater != nullptr)
{
Expand Down Expand Up @@ -93,8 +90,7 @@ void ZmqProducerStateTable::del(
m_zmqClient.sendMsg(
m_dbName,
m_tableNameStr,
kcos,
m_sendbuffer);
kcos);

if (m_asyncDBUpdater != nullptr)
{
Expand All @@ -112,8 +108,7 @@ void ZmqProducerStateTable::set(const std::vector<KeyOpFieldsValuesTuple> &value
m_zmqClient.sendMsg(
m_dbName,
m_tableNameStr,
values,
m_sendbuffer);
values);

if (m_asyncDBUpdater != nullptr)
{
Expand All @@ -136,8 +131,7 @@ void ZmqProducerStateTable::del(const std::vector<std::string> &keys)
m_zmqClient.sendMsg(
m_dbName,
m_tableNameStr,
kcos,
m_sendbuffer);
kcos);

if (m_asyncDBUpdater != nullptr)
{
Expand All @@ -157,8 +151,7 @@ void ZmqProducerStateTable::send(const std::vector<KeyOpFieldsValuesTuple> &kcos
m_zmqClient.sendMsg(
m_dbName,
m_tableNameStr,
kcos,
m_sendbuffer);
kcos);

if (m_asyncDBUpdater != nullptr)
{
Expand Down
2 changes: 0 additions & 2 deletions common/zmqproducerstatetable.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,6 @@ class ZmqProducerStateTable : public ProducerStateTable
void initialize(DBConnector *db, const std::string &tableName, bool dbPersistence);

ZmqClient& m_zmqClient;

std::vector<char> m_sendbuffer;

const std::string m_dbName;
const std::string m_tableNameStr;
Expand Down
Loading