C API: improve performance by not memcpy-ing string DB values across ffi boundary

common/c-api/consumerstatetable.cpp

@@ -1,3 +1,4 @@
+#include <boost/numeric/conversion/cast.hpp>
 #include <cstdlib>
 #include <cstring>
 #include <deque>
@@ -10,6 +11,7 @@
 
 using namespace swss;
 using namespace std;
+using boost::numeric_cast;
 
 SWSSConsumerStateTable SWSSConsumerStateTable_new(SWSSDBConnector db, const char *tableName,
                                                   const int32_t *p_popBatchSize,

common/c-api/dbconnector.cpp

@@ -1,5 +1,6 @@
 #include <cstring>
 #include <string>
+#include <utility>
 
 #include "../dbconnector.h"
 #include "dbconnector.h"
@@ -37,14 +38,14 @@ int8_t SWSSDBConnector_del(SWSSDBConnector db, const char *key) {
     SWSSTry(return ((DBConnector *)db)->del(string(key)) ? 1 : 0);
 }
 
-void SWSSDBConnector_set(SWSSDBConnector db, const char *key, const char *value) {
-    SWSSTry(((DBConnector *)db)->set(string(key), string(value)));
+void SWSSDBConnector_set(SWSSDBConnector db, const char *key, SWSSStrRef value) {
+    SWSSTry(((DBConnector *)db)->set(string(key), takeStrRef(value)));
 }
 
-char *SWSSDBConnector_get(SWSSDBConnector db, const char *key) {
+SWSSString SWSSDBConnector_get(SWSSDBConnector db, const char *key) {
     SWSSTry({
         shared_ptr<string> s = ((DBConnector *)db)->get(string(key));
-        return s ? strdup(s->c_str()) : nullptr;
+        return s ? makeString(move(*s)) : nullptr;
     });
 }
 
@@ -57,21 +58,29 @@ int8_t SWSSDBConnector_hdel(SWSSDBConnector db, const char *key, const char *fie
 }
 
 void SWSSDBConnector_hset(SWSSDBConnector db, const char *key, const char *field,
-                          const char *value) {
-    SWSSTry(((DBConnector *)db)->hset(string(key), string(field), string(value)));
+                          SWSSStrRef value) {
+    SWSSTry(((DBConnector *)db)->hset(string(key), string(field), takeStrRef(value)));
 }
 
-char *SWSSDBConnector_hget(SWSSDBConnector db, const char *key, const char *field) {
+SWSSString SWSSDBConnector_hget(SWSSDBConnector db, const char *key, const char *field) {
     SWSSTry({
         shared_ptr<string> s = ((DBConnector *)db)->hget(string(key), string(field));
-        return s ? strdup(s->c_str()) : nullptr;
+        return s ? makeString(move(*s)) : nullptr;
     });
 }
 
 SWSSFieldValueArray SWSSDBConnector_hgetall(SWSSDBConnector db, const char *key) {
     SWSSTry({
-        auto map = ((DBConnector *)db)->hgetall(key);
-        return makeFieldValueArray(map);
+        auto map = ((DBConnector *)db)->hgetall(string(key));
+
+        // We can't move keys out of the map, we have to copy them, until C++17 map::extract so we
+        // copy them here into a vector to avoid needing an overload on makeFieldValueArray
+        vector<pair<string, string>> pairs;
+        pairs.reserve(map.size());
+        for (auto &pair : map)
+            pairs.push_back(make_pair(pair.first, move(pair.second)));
+
+        return makeFieldValueArray(std::move(pairs));
     });
 }
 

common/c-api/dbconnector.h

@@ -29,71 +29,34 @@ void SWSSDBConnector_free(SWSSDBConnector db);
 // Returns 0 when key doesn't exist, 1 when key was deleted
 int8_t SWSSDBConnector_del(SWSSDBConnector db, const char *key);
 
-void SWSSDBConnector_set(SWSSDBConnector db, const char *key, const char *value);
+void SWSSDBConnector_set(SWSSDBConnector db, const char *key, SWSSStrRef value);
 
-// Returns NULL if key doesn't exist.
-// Result must be freed using free()
-char *SWSSDBConnector_get(SWSSDBConnector db, const char *key);
+// Returns NULL if key doesn't exist
+// Result must be freed using SWSSString_free()
+SWSSString SWSSDBConnector_get(SWSSDBConnector db, const char *key);
 
 // Returns 0 for false, 1 for true
 int8_t SWSSDBConnector_exists(SWSSDBConnector db, const char *key);
 
 // Returns 0 when key or field doesn't exist, 1 when field was deleted
 int8_t SWSSDBConnector_hdel(SWSSDBConnector db, const char *key, const char *field);
 
-void SWSSDBConnector_hset(SWSSDBConnector db, const char *key, const char *field,
-                          const char *value);
+void SWSSDBConnector_hset(SWSSDBConnector db, const char *key, const char *field, SWSSStrRef value);
 
-// Returns NULL if key or field doesn't exist.
-// Result must be freed using free()
-char *SWSSDBConnector_hget(SWSSDBConnector db, const char *key, const char *field);
+// Returns NULL if key or field doesn't exist
+// Result must be freed using SWSSString_free()
+SWSSString SWSSDBConnector_hget(SWSSDBConnector db, const char *key, const char *field);
 
-// Returns an empty map when the key doesn't exist.
-// Result array and all of its elements must be freed using free()
+// Returns an empty map when the key doesn't exist
+// Result array and all of its elements must be freed using appropriate free functions
 SWSSFieldValueArray SWSSDBConnector_hgetall(SWSSDBConnector db, const char *key);
 
 // Returns 0 when key or field doesn't exist, 1 when field exists
 int8_t SWSSDBConnector_hexists(SWSSDBConnector db, const char *key, const char *field);
 
-// std::vector<std::string> keys(const std::string &key);
-
-// std::pair<int, std::vector<std::string>> scan(int cursor = 0, const char
-// *match = "", uint32_t count = 10);
-
-// template<typename InputIterator>
-// void hmset(const std::string &key, InputIterator start, InputIterator stop);
-
-// void hmset(const std::unordered_map<std::string,
-// std::vector<std::pair<std::string, std::string>>>& multiHash);
-
-// std::shared_ptr<std::string> get(const std::string &key);
-
-// std::shared_ptr<std::string> hget(const std::string &key, const std::string
-// &field);
-
-// int64_t incr(const std::string &key);
-
-// int64_t decr(const std::string &key);
-
-// int64_t rpush(const std::string &list, const std::string &item);
-
-// std::shared_ptr<std::string> blpop(const std::string &list, int timeout);
-
-// void subscribe(const std::string &pattern);
-
-// void psubscribe(const std::string &pattern);
-
-// void punsubscribe(const std::string &pattern);
-
-// int64_t publish(const std::string &channel, const std::string &message);
-
-// void config_set(const std::string &key, const std::string &value);
-
 // Returns 1 on success, 0 on failure
 int8_t SWSSDBConnector_flushdb(SWSSDBConnector db);
 
-// std::map<std::string, std::map<std::string, std::map<std::string,
-// std::string>>> getall();
 #ifdef __cplusplus
 }
 #endif

common/c-api/producerstatetable.cpp

@@ -25,7 +25,7 @@ void SWSSProducerStateTable_setBuffered(SWSSProducerStateTable tbl, uint8_t buff
 
 void SWSSProducerStateTable_set(SWSSProducerStateTable tbl, const char *key,
                                 SWSSFieldValueArray values) {
-    SWSSTry(((ProducerStateTable *)tbl)->set(string(key), takeFieldValueArray(values)));
+    SWSSTry(((ProducerStateTable *)tbl)->set(string(key), takeFieldValueArray(std::move(values))));
 }
 
 void SWSSProducerStateTable_del(SWSSProducerStateTable tbl, const char *key) {

common/c-api/producerstatetable.h

@@ -22,11 +22,6 @@ void SWSSProducerStateTable_set(SWSSProducerStateTable tbl, const char *key, SWS
 
 void SWSSProducerStateTable_del(SWSSProducerStateTable tbl, const char *key);
 
-// Batched version of set() and del().
-// virtual void set(const std::vector<KeyOpFieldsValuesTuple>& values);
-
-// virtual void del(const std::vector<std::string>& keys);
-
 void SWSSProducerStateTable_flush(SWSSProducerStateTable tbl);
 
 int64_t SWSSProducerStateTable_count(SWSSProducerStateTable tbl);

common/c-api/subscriberstatetable.cpp

@@ -1,3 +1,4 @@
+#include <boost/numeric/conversion/cast.hpp>
 #include <cstdlib>
 #include <cstring>
 #include <deque>
@@ -11,6 +12,7 @@
 
 using namespace swss;
 using namespace std;
+using boost::numeric_cast;
 
 SWSSSubscriberStateTable SWSSSubscriberStateTable_new(SWSSDBConnector db, const char *tableName,
                                                       const int32_t *p_popBatchSize,

common/c-api/util.cpp

@@ -1,3 +1,33 @@
 #include "util.h"
 
+using namespace swss;
+
 bool swss::cApiTestingDisableAbort = false;
+
+SWSSString SWSSString_new(const char *data, uint64_t length) {
+    SWSSTry(return makeString(std::string(data, numeric_cast<std::string::size_type>(length))));
+}
+
+SWSSString SWSSString_new_c_str(const char *c_str) {
+    SWSSTry(return makeString(std::string(c_str)));
+}
+
+const char *SWSSStrRef_c_str(SWSSStrRef s) {
+    SWSSTry(return ((std::string *)s)->c_str());
+}
+
+uint64_t SWSSStrRef_length(SWSSStrRef s) {
+    SWSSTry(return ((std::string *)s)->length());
+}
+
+void SWSSString_free(SWSSString s) {
+    SWSSTry(delete (std::string *)s);
+}
+
+void SWSSFieldValueArray_free(SWSSFieldValueArray arr) {
+    SWSSTry(delete[] arr.data);
+}
+
+void SWSSKeyOpFieldValuesArray_free(SWSSKeyOpFieldValuesArray kfvs) {
+    SWSSTry(delete[] kfvs.data);
+}