Repository: Xilinx/inference-server (language: python)

Issue:
Update benchmarks
# Summary of Changes

* Update the ResNet50 benchmark to work for all backends

# Motivation

Having benchmarks to measure the server performance is important to find bugs and improve the performance.

# Implementation

The updated benchmark has implementations for each backend to run a ResNet50 model. Each model is run with a variety of configurations and the results can be saved as JSON. I added a script to convert this JSON output into a format that can be placed in RST for the docs. I also found and added a way to add dynamic tables in the documentation which will be useful once we add more information.

The backend implementation can be generalized once another benchmark is added. It should be generic enough that we can reuse a lot of the logic but define unique preprocessing methods to support different models.

# Notes

The benchmarks are now split by protocol. The reason for this is because HTTP (drogon) cannot be easily restarted within a process between different test runs. So each protocol is now in a separate executable which will start the server and the particular protocol once before running the benchmarks. This way, one protocol does not interfere with the others for benchmarks.



Discussion / hints:
Build successful!


Fix patch:
diff --git a/include/amdinfer/core/tensor.hpp b/include/amdinfer/core/tensor.hpp
index 4c95c8427..8f36f5aff 100644
--- a/include/amdinfer/core/tensor.hpp
+++ b/include/amdinfer/core/tensor.hpp
@@ -76,6 +76,8 @@ class Tensor : public Serializable {
    */
   const std::byte *deserialize(const std::byte *data_in) override;
 
+  friend std::ostream &operator<<(std::ostream &os, const Tensor &self);
+
  private:
   std::string name_;
   std::vector<uint64_t> shape_;
diff --git a/src/amdinfer/core/memory_pool/cpu_allocator.cpp b/src/amdinfer/core/memory_pool/cpu_allocator.cpp
index aec5065a0..f3b5a81ca 100644
--- a/src/amdinfer/core/memory_pool/cpu_allocator.cpp
+++ b/src/amdinfer/core/memory_pool/cpu_allocator.cpp
@@ -66,8 +66,7 @@ BufferPtr CpuAllocator::get(const Tensor& tensor, size_t batch_size) {
   if (allocated_ + size_to_allocate > max_allocate_) {
     throw runtime_error("Too much requested");
   }
-  auto& new_block = data_.emplace_back();
-  new_block.resize(size_to_allocate);
+  auto& new_block = data_.emplace_back(size_to_allocate);
   allocated_ += size_to_allocate;
 
   auto* retval = new_block.data();
diff --git a/src/amdinfer/core/tensor.cpp b/src/amdinfer/core/tensor.cpp
index 65c947457..4a15b822c 100644
--- a/src/amdinfer/core/tensor.cpp
+++ b/src/amdinfer/core/tensor.cpp
@@ -95,4 +95,14 @@ const std::byte *Tensor::deserialize(const std::byte *data_in) {
   return data_in;
 }
 
+std::ostream &operator<<(std::ostream &os, const Tensor &self) {
+  os << "Tensor(" << self.name_ << ", [";
+  for (const auto &index : self.shape_) {
+    os << index << ",";
+  }
+  os << "], " << self.data_type_.str() << ")";
+
+  return os;
+}
+
 }  // namespace amdinfer
diff --git a/src/amdinfer/core/worker_info.cpp b/src/amdinfer/core/worker_info.cpp
index ab58c8b56..6282a9df4 100644
--- a/src/amdinfer/core/worker_info.cpp
+++ b/src/amdinfer/core/worker_info.cpp
@@ -40,17 +40,16 @@
 
 namespace amdinfer {
 
-/**
- * @brief Find the named function in a *.so file
- *
- * @param func name of the symbol  to find
- * @param so_path path to the *.so file to search
- * @return void* pointer to the function
- */
-void* findFunc(const std::string& func, const std::string& so_path) {
-  if (func.empty() || so_path.empty()) {
-    throw invalid_argument("Function or .so path empty");
+void* getHandle(const std::string& name) {
+  // multiple workers with different configurations may exist. Remove the config
+  // tag that starts with "-" in the name prior to loading the .so
+  auto lib_name = name;
+  lib_name[0] = static_cast<char>(std::toupper(lib_name[0]));
+  if (auto hyphen_pos = name.find('-'); hyphen_pos != std::string::npos) {
+    lib_name.erase(hyphen_pos);
   }
+  std::string library =
+    std::string("libworker") + lib_name + std::string(".so");
 
   // reset errors
   dlerror();
@@ -70,7 +69,21 @@ void* findFunc(const std::string& func, const std::string& so_path) {
   workers is dlmopen but that also should not be used here due to its own set of
   issues (https://sourceware.org/bugzilla/show_bug.cgi?id=24776).
   */
-  void* handle = dlopen(so_path.c_str(), RTLD_LOCAL | RTLD_LAZY);
+  void* handle = dlopen(library.c_str(), RTLD_LOCAL | RTLD_LAZY);
+  return handle;
+}
+
+/**
+ * @brief Find the named function in a *.so file
+ *
+ * @param func name of the symbol  to find
+ * @param so_path path to the *.so file to search
+ * @return void* pointer to the function
+ */
+void* findFunc(void* handle, const std::string& func) {
+  // reset errors
+  dlerror();
+
   if (handle == nullptr) {
     const char* error_str = dlerror();
     throw file_not_found_error(error_str);
@@ -85,18 +98,8 @@ void* findFunc(const std::string& func, const std::string& so_path) {
   return fptr;
 }
 
-workers::Worker* getWorker(const std::string& name) {
-  // multiple workers with different configurations may exist. Remove the config
-  // tag that starts with "-" in the name prior to loading the .so
-  auto lib_name = name;
-  lib_name[0] = static_cast<char>(std::toupper(lib_name[0]));
-  if (auto hyphen_pos = name.find('-'); hyphen_pos != std::string::npos) {
-    lib_name.erase(hyphen_pos);
-  }
-  std::string library =
-    std::string("libworker") + lib_name + std::string(".so");
-
-  void* func_ptr = findFunc("getWorker", library);
+workers::Worker* getWorker(void* handle) {
+  void* func_ptr = findFunc(handle, "getWorker");
 
   // cast the void pointer from dlsym to a function pointer. This assumes that
   // void* is same size as function pointer, which should be true on POSIX
@@ -108,6 +111,7 @@ WorkerInfo::WorkerInfo(const std::string& name, ParameterMap* parameters,
                        MemoryPool* pool, BatchPtrQueue* next,
                        const std::vector<MemoryAllocators>& next_allocators)
   : next_(next), next_allocators_(next_allocators) {
+  handle_ = getHandle(name);
   this->addAndStartWorker(name, parameters, pool);
 }
 
@@ -116,11 +120,13 @@ WorkerInfo::~WorkerInfo() {
   for (const auto& [thread_id, worker] : workers_) {
     delete worker;  // NOLINT(cppcoreguidelines-owning-memory)
   }
+  dlclose(handle_);
+  handle_ = nullptr;
 }
 
 void WorkerInfo::addAndStartWorker(const std::string& name,
                                    ParameterMap* parameters, MemoryPool* pool) {
-  auto* worker = getWorker(name);
+  auto* worker = getWorker(handle_);
   worker->init(parameters);
 
   std::vector<MemoryAllocators> allocators = worker->getAllocators();
diff --git a/src/amdinfer/core/worker_info.hpp b/src/amdinfer/core/worker_info.hpp
index daaaa1d94..8f32407e3 100644
--- a/src/amdinfer/core/worker_info.hpp
+++ b/src/amdinfer/core/worker_info.hpp
@@ -106,6 +106,7 @@ class WorkerInfo {
 
  private:
   std::map<std::thread::id, std::thread> worker_threads_;
+  void* handle_;
   std::map<std::thread::id, workers::Worker*> workers_;
   std::vector<std::unique_ptr<Batcher>> batchers_;
   size_t batch_size_ = 1;
diff --git a/src/amdinfer/observation/observer.hpp b/src/amdinfer/observation/observer.hpp
index 0f7c5bcfe..630a36e5e 100644
--- a/src/amdinfer/observation/observer.hpp
+++ b/src/amdinfer/observation/observer.hpp
@@ -44,7 +44,7 @@ inline void logTraceBuffer([[maybe_unused]] const Logger& logger, void* data,
     bytes += std::to_string(static_cast<int>(addr[i])) + ", ";
   }
   AMDINFER_LOG_TRACE(
-    logger, "Buffer(" + util::addressToString(data) + ") has bytes: " + bytes);
+    logger, "Buffer(" + util::toString(data) + ") has bytes: " + bytes);
 }
 
 #endif  // AMDINFER_ENABLE_LOGGING
diff --git a/src/amdinfer/servers/grpc_server.cpp b/src/amdinfer/servers/grpc_server.cpp
index 467031dab..3ed385318 100644
--- a/src/amdinfer/servers/grpc_server.cpp
+++ b/src/amdinfer/servers/grpc_server.cpp
@@ -330,7 +330,7 @@ InferenceRequestInput getInput(
   AMDINFER_LOG_TRACE(observer.logger, "Writing " + std::to_string(size) +
                                         " elements of type " +
                                         input.getDatatype().str() + " to " +
-                                        util::addressToString(buffer->data(0)));
+                                        util::toString(buffer->data(0)));
 
   switchOverTypes(WriteData(), input.getDatatype(), buffer.get(), &req, 0, size,
                   observer);
diff --git a/src/amdinfer/util/string.hpp b/src/amdinfer/util/string.hpp
index 9b1a72aa9..6d5bc8577 100644
--- a/src/amdinfer/util/string.hpp
+++ b/src/amdinfer/util/string.hpp
@@ -80,10 +80,11 @@ inline bool isLower(std::string_view str) {
                      [](char c) { return c == std::tolower(c); });
 }
 
-inline std::string addressToString(const void* ptr) {
-  std::ostringstream addr;
-  addr << ptr;
-  return addr.str();
+template <typename T>
+inline std::string toString(const T& object) {
+  std::ostringstream ss;
+  ss << object;
+  return ss.str();
 }
 
 }  // namespace amdinfer::util
diff --git a/src/amdinfer/workers/tf_zendnn.cpp b/src/amdinfer/workers/tf_zendnn.cpp
index 513704358..32978b8a2 100644
--- a/src/amdinfer/workers/tf_zendnn.cpp
+++ b/src/amdinfer/workers/tf_zendnn.cpp
@@ -66,6 +66,15 @@ const int kResNetImageSize = 224;
 const int kResNetImageChannels = 3;
 const int kResNetOutputClasses = 1000;
 
+void* openLibrary(const char* library, int dlopen_flags) {
+  void* handle = dlopen(library, dlopen_flags);
+  if (handle == nullptr) {
+    const char* error_str = dlerror();
+    throw amdinfer::file_not_found_error(error_str);
+  }
+  return handle;
+}
+
 /**
  * @brief The TfZendnn worker is a simple worker that accepts a single uint32_t
  * argument and adds 1 to it and returns. It accepts multiple input tensors and
@@ -74,8 +83,11 @@ const int kResNetOutputClasses = 1000;
  */
 class TfZendnn : public SingleThreadedWorker {
  public:
-  using SingleThreadedWorker::SingleThreadedWorker;
+  TfZendnn(const std::string& name, const std::string& platform,
+           bool allow_next);
+  // using SingleThreadedWorker::SingleThreadedWorker;
   [[nodiscard]] std::vector<MemoryAllocators> getAllocators() const override;
+  ~TfZendnn() override;
 
  private:
   void doInit(ParameterMap* parameters) override;
@@ -84,6 +96,10 @@ class TfZendnn : public SingleThreadedWorker {
   void doRelease() override;
   void doDestroy() override;
 
+  // handles must be the first member variable so the TF libraries can be loaded
+  // and saved before attempting to create the other members
+  std::vector<void*> handles_;
+
   // TF session and graphs
   tf::Session* session_ = nullptr;
   tf::GraphDef graph_def_;
@@ -102,6 +118,36 @@ class TfZendnn : public SingleThreadedWorker {
   DataType input_dt_ = DataType::Fp32;
 };
 
+std::vector<void*> saveHandles() {
+  std::vector<void*> handles;
+
+  // Due to the DEEPBIND change for tensorflow_cc.so below, OMP now gives a
+  // segfault unexpectedly if the server is run from Python. Preloading iomp
+  // without DEEPBIND addresses this problem.
+  handles.emplace_back(openLibrary("libiomp5.so", RTLD_LOCAL | RTLD_LAZY));
+  // Upcoming changes in Tensorflow move the Protobuf symbols defined in this
+  // library to another library called tensorflow_framework.so. At runtime,
+  // tensorflow_cc should resolve its missing protobuf symbols against this
+  // library but instead it finds the protobuf used in the inference server.
+  // Using DEEPBIND addresses this problem so the protobuf symbols get found
+  // correctly when this library uses its own dependencies to resolve missing
+  // symbols before the global scope.
+  handles.emplace_back(openLibrary("libtensorflow_cc.so",
+                                   RTLD_GLOBAL | RTLD_LAZY | RTLD_DEEPBIND));
+  return handles;
+}
+
+TfZendnn::TfZendnn(const std::string& name, const std::string& platform,
+                   bool allow_next)
+  : SingleThreadedWorker(name, platform, allow_next), handles_(saveHandles()) {}
+
+TfZendnn::~TfZendnn() {
+  for (auto* handle : handles_) {
+    dlclose(handle);
+  }
+  handles_.clear();
+}
+
 std::vector<MemoryAllocators> TfZendnn::getAllocators() const {
   return {MemoryAllocators::Cpu};
 }
@@ -303,35 +349,12 @@ void TfZendnn::doRelease() {
 }
 void TfZendnn::doDestroy() {}
 
-void* openLibrary(const char* library, int dlopen_flags) {
-  void* handle = dlopen(library, dlopen_flags);
-  if (handle == nullptr) {
-    const char* error_str = dlerror();
-    throw amdinfer::file_not_found_error(error_str);
-  }
-  return handle;
-}
-
 }  // namespace amdinfer::workers
 
 extern "C" {
 // using smart pointer here may cause problems inside shared object so managing
 // manually
 amdinfer::workers::Worker* getWorker() {
-  using amdinfer::workers::openLibrary;
-  // Due to the DEEPBIND change for tensorflow_cc.so below, OMP now gives a
-  // segfault unexpectedly if the server is run from Python. Preloading iomp
-  // without DEEPBIND addresses this problem.
-  openLibrary("libiomp5.so", RTLD_LOCAL | RTLD_LAZY);
-  // Upcoming changes in Tensorflow move the Protobuf symbols defined in this
-  // library to another library called tensorflow_framework.so. At runtime,
-  // tensorflow_cc should resolve its missing protobuf symbols against this
-  // library but instead it finds the protobuf used in the inference server.
-  // Using DEEPBIND addresses this problem so the protobuf symbols get found
-  // correctly when this library uses its own dependencies to resolve missing
-  // symbols before the global scope.
-  openLibrary("libtensorflow_cc.so", RTLD_GLOBAL | RTLD_LAZY | RTLD_DEEPBIND);
-
   return new amdinfer::workers::TfZendnn("TfZendnn", "CPU", true);
 }
 }  // extern C


Test patch:
diff --git a/tests/performance/models/benchmark_resnet50.cpp b/tests/performance/models/benchmark_resnet50.cpp
index bea421f09..740965a9f 100644
--- a/tests/performance/models/benchmark_resnet50.cpp
+++ b/tests/performance/models/benchmark_resnet50.cpp
@@ -385,7 +385,7 @@ void resnet50(benchmark::State& st, const amdinfer::Client* client,
 const std::initializer_list<std::vector<int64_t>> kRange{
   {1, 4, 64},                         // batch size
   benchmark::CreateRange(1, 256, 4),  // number of requests
-  benchmark::CreateRange(1, 8, 2)     // workers
+  benchmark::CreateRange(1, 4, 2)     // workers
 };
 
 int main(int argc, char* argv[]) {
@@ -394,15 +394,14 @@ int main(int argc, char* argv[]) {
 #if defined(PROTOCOL_HTTP)
   const auto default_http_port = 8998;
   server.startHttp(default_http_port);
-  auto client = std::make_unique<amdinfer::HttpClient>(
-    "http://127.0.0.1:" + std::to_string(default_http_port));
+  amdinfer::HttpClient client{"http://127.0.0.1:" +
+                              std::to_string(default_http_port)};
 #elif defined(PROTOCOL_GRPC)
   const auto default_grpc_port = 50051;
   server.startGrpc(default_grpc_port);
-  auto client = std::make_unique<amdinfer::GrpcClient>(
-    "127.0.0.1:" + std::to_string(default_grpc_port));
+  amdinfer::GrpcClient client{"127.0.0.1:" + std::to_string(default_grpc_port)};
 #else
-  auto client = std::make_unique<amdinfer::NativeClient>(&server);
+  amdinfer::NativeClient client{&server};
 #endif
 
   std::vector<std::unique_ptr<Backend>> backends;
@@ -414,8 +413,8 @@ int main(int argc, char* argv[]) {
     backends.push_back(getBackend(static_cast<Backends>(i)));
     auto* backend = backends.back().get();
     auto name = "ResNet50/" + backend->name();
-    auto* benchmark = benchmark::RegisterBenchmark(name.c_str(), resnet50,
-                                                   client.get(), backend);
+    auto* benchmark =
+      benchmark::RegisterBenchmark(name.c_str(), resnet50, &client, backend);
     benchmark->ArgsProduct(kRange);
     benchmark->Unit(benchmark::kMillisecond);
 
@@ -424,7 +423,7 @@ int main(int argc, char* argv[]) {
     backend->preprocess(image_location);
   }
 
-  amdinfer::waitUntilServerReady(client.get());
+  amdinfer::waitUntilServerReady(&client);
 
   benchmark::Initialize(&argc, argv);
   benchmark::RunSpecifiedBenchmarks();