Remove C++17 features/syntax (#173)

rmccorm4 · web-flow · commit e5afc19823e0 · 2023-03-07T00:33:40.000-08:00
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -126,7 +126,6 @@ if(CMAKE_CXX_COMPILER_ID STREQUAL "MSVC")
   message("Using MSVC as compiler, default target on Windows 10. "
 		  "If the target system is not Windows 10, please update _WIN32_WINNT "
 		  "to corresponding value.")
-  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /std:c++17")
   target_compile_options(
     triton-core-serverstub
     PRIVATE
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
@@ -27,8 +27,6 @@
 cmake_minimum_required(VERSION 3.18)
 
 project(libtritonserver LANGUAGES C CXX)
-set(CMAKE_CXX_STANDARD 17)
-set(CMAKE_CXX_STANDARD_REQUIRED ON)
 
 if(NOT CMAKE_BUILD_TYPE)
   set(CMAKE_BUILD_TYPE Release)
@@ -233,13 +231,11 @@ add_library(
   TritonCore::triton-core ALIAS triton-core
 )
 
-target_compile_features(triton-core PRIVATE cxx_std_17)
+target_compile_features(triton-core PRIVATE cxx_std_11)
 if(CMAKE_CXX_COMPILER_ID STREQUAL "MSVC")
   message("Using MSVC as compiler, default target on Windows 10. "
 		  "If the target system is not Windows 10, please update _WIN32_WINNT "
 		  "to corresponding value.")
-  # WAR for target_compile_features not applying correctly to MSVC.
-  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /std:c++17")
   target_compile_options(
     triton-core
     PRIVATE
diff --git a/src/cache_entry.cc b/src/cache_entry.cc
@@ -34,26 +34,26 @@ namespace triton { namespace core {
 size_t
 CacheEntry::BufferCount()
 {
-  std::unique_lock lk(buffer_mu_);
+  std::unique_lock<std::mutex> lk(buffer_mu_);
   return buffers_.size();
 }
 
 const std::vector<Buffer>&
 CacheEntry::Buffers()
 {
-  std::unique_lock lk(buffer_mu_);
+  std::unique_lock<std::mutex> lk(buffer_mu_);
   return buffers_;
 }
 
 std::vector<Buffer>&
 CacheEntry::MutableBuffers()
 {
-  std::unique_lock lk(buffer_mu_);
+  std::unique_lock<std::mutex> lk(buffer_mu_);
   return buffers_;
 }
 
 void
-CacheEntry::AddBuffer(boost::span<std::byte> byte_span)
+CacheEntry::AddBuffer(boost::span<Byte> byte_span)
 {
   void* base = static_cast<void*>(byte_span.data());
   AddBuffer(base, byte_span.size());
@@ -62,18 +62,19 @@ CacheEntry::AddBuffer(boost::span<std::byte> byte_span)
 void
 CacheEntry::AddBuffer(void* base, size_t byte_size)
 {
-  std::unique_lock lk(buffer_mu_);
+  std::unique_lock<std::mutex> lk(buffer_mu_);
   buffers_.emplace_back(std::make_pair(base, byte_size));
 }
 
 CacheEntry::~CacheEntry()
 {
-  std::unique_lock lk(buffer_mu_);
+  std::unique_lock<std::mutex> lk(buffer_mu_);
   if (!free_buffers_) {
     return;
   }
 
-  for (auto& [base, _] : buffers_) {
+  for (auto& iter : buffers_) {
+    auto& base = iter.first;
     if (base) {
       free(base);
       base = nullptr;
@@ -90,7 +91,7 @@ CacheEntry::AddPlaceholderBuffer(size_t byte_size)
 // Set the size of each buffer in the CacheEntry object so the cache knows
 // how much to allocate for each entry before insertion.
 Status
-CacheEntry::SetBufferSizes(std::vector<boost::span<std::byte>> buffers)
+CacheEntry::SetBufferSizes(std::vector<boost::span<Byte>> buffers)
 {
   for (const auto buffer : buffers) {
     AddPlaceholderBuffer(buffer.size());
@@ -163,7 +164,7 @@ CacheEntry::SerializeResponse(InferenceResponse* response, Buffer& buffer)
   // The packed_response buffer will look like:
   //   [num_outputs, sizeof(output1), output1, ..., sizeof(outputN), outputN]
   size_t position = 0;
-  std::byte* base = static_cast<std::byte*>(buffer.first);
+  Byte* base = static_cast<Byte*>(buffer.first);
   // 1. First the packed buffer will hold the number of outputs as a uint32_t
   uint32_t num_outputs = response->Outputs().size();
   std::memcpy(base, &num_outputs, sizeof(uint32_t));
@@ -193,8 +194,7 @@ CacheEntry::SerializeResponse(InferenceResponse* response, Buffer& buffer)
 
 Status
 CacheEntry::SerializeResponseOutput(
-    const InferenceResponse::Output& output, std::byte* buffer,
-    size_t* output_size)
+    const InferenceResponse::Output& output, Byte* buffer, size_t* output_size)
 {
   if (!buffer) {
     return Status(Status::Code::INVALID_ARG, "buffer arg was nullptr");
@@ -312,7 +312,7 @@ CacheEntry::DeserializeBuffer(InferenceResponse* response, const Buffer& buffer)
   }
 
 
-  const std::byte* base = static_cast<std::byte*>(buffer.first);
+  const Byte* base = static_cast<Byte*>(buffer.first);
   if (!base) {
     return Status(Status::Code::INTERNAL, "buffer was nullptr");
   }
@@ -441,7 +441,7 @@ CacheEntry::GetByteSize(
 
 Status
 CacheEntry::DeserializeResponseOutput(
-    boost::span<const std::byte> packed_bytes, CacheOutput* output)
+    boost::span<const Byte> packed_bytes, CacheOutput* output)
 {
   if (!output) {
     return Status(Status::Code::INVALID_ARG, "output arg was nullptr");
@@ -453,17 +453,17 @@ CacheEntry::DeserializeResponseOutput(
   memcpy(&name_byte_size, packed_bytes.begin() + position, sizeof(uint32_t));
   position += sizeof(uint32_t);
 
-  std::string name(name_byte_size, 'x');
-  memcpy(name.data(), packed_bytes.begin() + position, name_byte_size);
+  auto name_start = packed_bytes.begin() + position;
+  std::string name(name_start, name_start + name_byte_size);
   position += name_byte_size;
 
   // Dtype
   uint32_t dtype_byte_size = 0;
   memcpy(&dtype_byte_size, packed_bytes.begin() + position, sizeof(uint32_t));
   position += sizeof(uint32_t);
 
-  std::string dtype(dtype_byte_size, 'x');
-  memcpy(dtype.data(), packed_bytes.begin() + position, dtype_byte_size);
+  auto dtype_start = packed_bytes.begin() + position;
+  std::string dtype(dtype_start, dtype_start + dtype_byte_size);
   position += dtype_byte_size;
 
   // Shape
diff --git a/src/cache_entry.h b/src/cache_entry.h
@@ -62,7 +62,7 @@ class CacheEntry {
   const std::vector<Buffer>& Buffers();
   std::vector<Buffer>& MutableBuffers();
   size_t BufferCount();
-  void AddBuffer(boost::span<std::byte> buffer);
+  void AddBuffer(boost::span<Byte> buffer);
   void AddBuffer(void* base, size_t byte_size);
 
   /* Insert helpers */
@@ -81,7 +81,7 @@ class CacheEntry {
   // and sets entry buffer sizes
   Status SetBufferSizes(boost::span<InferenceResponse*> responses);
   // Directly sets entry buffer sizes from provided buffers
-  Status SetBufferSizes(std::vector<boost::span<std::byte>> buffers);
+  Status SetBufferSizes(std::vector<boost::span<Byte>> buffers);
 
   /* Lookup helpers */
   Status DeserializeBuffers(boost::span<InferenceResponse*> responses);
@@ -95,7 +95,7 @@ class CacheEntry {
   // Insert helpers
   Status SerializeResponse(InferenceResponse* response, Buffer& buffer);
   Status SerializeResponseOutput(
-      const InferenceResponse::Output& output, std::byte* buffer,
+      const InferenceResponse::Output& output, Byte* buffer,
       size_t* output_size);
   Status SetBufferSize(InferenceResponse* response);
   // Calculates total byte size required to serialize response output and
@@ -107,7 +107,7 @@ class CacheEntry {
   // Lookup helpers
   Status DeserializeBuffer(InferenceResponse* response, const Buffer& buffer);
   Status DeserializeResponseOutput(
-      boost::span<const std::byte> packed_bytes, CacheOutput* output);
+      boost::span<const Byte> packed_bytes, CacheOutput* output);
 
   // NOTE: performance gain may be possible by removing this mutex and
   //   guaranteeing that no two threads will access/modify an entry
diff --git a/src/cache_manager.cc b/src/cache_manager.cc
@@ -86,7 +86,10 @@ CacheToBytesAllocator::Allocate(TRITONCACHE_CacheEntry* entry)
 
   // NOTE: If the same entry object is re-used for multiple lookups and the
   // entry buffers are not freed between uses, this will leak memory.
-  for (auto& [base, byte_size] : buffers) {
+  for (auto& iter : buffers) {
+    auto& base = iter.first;
+    const auto& byte_size = iter.second;
+
     void* new_base = malloc(byte_size);
     std::memcpy(new_base, base, byte_size);
     base = new_base;
@@ -98,7 +101,7 @@ CacheToBytesAllocator::Allocate(TRITONCACHE_CacheEntry* entry)
 }
 
 BytesToCacheAllocator::BytesToCacheAllocator(
-    std::vector<boost::span<std::byte>> buffers)
+    std::vector<boost::span<Byte>> buffers)
 {
   // Span is a read-only view of the underlying buffer, this should only
   // perform a shallow copy of base pointer and size of each span.
@@ -126,7 +129,8 @@ BytesToCacheAllocator::Allocate(TRITONCACHE_CacheEntry* entry)
   // Copy from allocator provided buffers into cache-allocated buffers
   // that were setup in 'entry' by cache implementation.
   for (size_t i = 0; i < buffers_.size(); i++) {
-    auto [cache_buffer, cache_buffer_size] = cache_buffers[i];
+    auto cache_buffer = cache_buffers[i].first;
+    auto cache_buffer_size = cache_buffers[i].second;
     if (buffers_[i].size() != cache_buffer_size) {
       return Status(
           Status::Code::INTERNAL,
@@ -363,7 +367,7 @@ TritonCache::Insert(
 
 Status
 TritonCache::Insert(
-    std::vector<boost::span<std::byte>> buffers, const std::string& key)
+    std::vector<boost::span<Byte>> buffers, const std::string& key)
 {
   std::unique_ptr<CacheEntry> entry = std::make_unique<CacheEntry>();
   RETURN_IF_ERROR(entry->SetBufferSizes(buffers));
diff --git a/src/cache_manager.h b/src/cache_manager.h
@@ -77,11 +77,11 @@ class CacheToBytesAllocator : TritonCacheAllocator {
 
 class BytesToCacheAllocator : TritonCacheAllocator {
  public:
-  BytesToCacheAllocator(std::vector<boost::span<std::byte>> buffers);
+  BytesToCacheAllocator(std::vector<boost::span<Byte>> buffers);
   Status Allocate(TRITONCACHE_CacheEntry* entry);
 
  private:
-  std::vector<boost::span<std::byte>> buffers_;
+  std::vector<boost::span<Byte>> buffers_;
 };
 
 
@@ -100,8 +100,7 @@ class TritonCache {
   Status Insert(InferenceResponse* response, const std::string& key);
   Status Insert(
       boost::span<InferenceResponse*> responses, const std::string& key);
-  Status Insert(
-      std::vector<boost::span<std::byte>> buffers, const std::string& key);
+  Status Insert(std::vector<boost::span<Byte>> buffers, const std::string& key);
   Status Insert(
       CacheEntry* entry, const std::string& key,
       TRITONCACHE_Allocator* allocator);
diff --git a/src/constants.h b/src/constants.h
@@ -1,4 +1,4 @@
-// Copyright 2018-2022, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+// Copyright 2018-2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
 //
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions
@@ -93,6 +93,9 @@ constexpr int METRIC_REPORTER_ID_CPU = -1;
 constexpr int METRIC_REPORTER_ID_RESPONSE_CACHE = -2;
 #endif
 
+// Note: This can be replaced with std::byte starting in c++17
+using Byte = unsigned char;
+
 #define TIMESPEC_TO_NANOS(TS) \
   ((TS).tv_sec * triton::core::NANOS_PER_SECOND + (TS).tv_nsec)
 #define TIMESPEC_TO_MILLIS(TS) \
diff --git a/src/server.cc b/src/server.cc
@@ -167,7 +167,9 @@ InferenceServer::Init()
   }
 
   // Initialize each cache with its respective config
-  for (const auto& [name, config] : cache_config_map_) {
+  for (const auto& iter : cache_config_map_) {
+    const auto& name = iter.first;
+    const auto& config = iter.second;
     std::shared_ptr<TritonCache> cache;
     status = cache_manager_->CreateCache(name, config, &cache);
     if (!status.IsOk()) {
diff --git a/src/test/response_cache_test.cc b/src/test/response_cache_test.cc
@@ -438,8 +438,7 @@ GenerateRequest(
 tc::Status
 InsertLookupCompare(
     std::shared_ptr<tc::TritonCache> cache,
-    std::vector<boost::span<std::byte>> expected_buffers,
-    const std::string& key)
+    std::vector<boost::span<tc::Byte>> expected_buffers, const std::string& key)
 {
   if (!cache) {
     return tc::Status(tc::Status::Code::INTERNAL, "cache was nullptr");
@@ -448,14 +447,14 @@ InsertLookupCompare(
   }
 
   helpers::CheckStatus(cache->Insert(expected_buffers, key));
-  auto lookup_entry = tc::CacheEntry();
-  auto status = cache->Lookup(key, &lookup_entry);
+  auto lookup_entry = std::make_unique<tc::CacheEntry>();
+  auto status = cache->Lookup(key, lookup_entry.get());
   if (!status.IsOk()) {
     return tc::Status(
         tc::Status::Code::INTERNAL, "Lookup failed: " + status.Message());
   }
 
-  auto lookup_buffers = lookup_entry.Buffers();
+  auto lookup_buffers = lookup_entry->Buffers();
   if (lookup_buffers.size() != expected_buffers.size()) {
     return tc::Status(
         tc::Status::Code::INTERNAL,
@@ -464,10 +463,10 @@ InsertLookupCompare(
   }
 
   for (size_t b = 0; b < expected_buffers.size(); b++) {
-    boost::span<std::byte> lookup = {
-        static_cast<std::byte*>(lookup_buffers[b].first),
+    boost::span<tc::Byte> lookup = {
+        static_cast<tc::Byte*>(lookup_buffers[b].first),
         lookup_buffers[b].second};
-    boost::span<std::byte> expected = expected_buffers[b];
+    boost::span<tc::Byte> expected = expected_buffers[b];
     if (!std::equal(
             lookup.begin(), lookup.end(), expected.begin(), expected.end())) {
       return tc::Status(
@@ -656,9 +655,9 @@ TEST_F(RequestResponseCacheTest, TestCacheSizeSmallerThanEntryBytes)
   ASSERT_NE(cache, nullptr);
 
   // Setup byte buffer larger than cache size
-  std::vector<std::byte> large_data(cache_size + 1);
+  std::vector<tc::Byte> large_data(cache_size + 1);
   // Setup entry
-  std::vector<boost::span<std::byte>> entry;
+  std::vector<boost::span<tc::Byte>> entry;
   entry.push_back(large_data);
 
   auto status = cache->Insert(entry, "large_bytes");
@@ -724,13 +723,13 @@ TEST_F(RequestResponseCacheTest, TestCacheInsertLookupCompareBytes)
   auto cache = helpers::CreateCache(1024);
   ASSERT_NE(cache, nullptr);
   // Setup byte buffers
-  std::vector<std::byte> buffer1{1, std::byte{0x01}};
-  std::vector<std::byte> buffer2{2, std::byte{0x02}};
-  std::vector<std::byte> buffer3{4, std::byte{0x04}};
-  std::vector<std::byte> buffer4{8, std::byte{0x08}};
-  std::vector<std::byte> buffer5{16, std::byte{0xFF}};
+  std::vector<tc::Byte> buffer1{1, tc::Byte{1}};
+  std::vector<tc::Byte> buffer2{2, tc::Byte{2}};
+  std::vector<tc::Byte> buffer3{4, tc::Byte{4}};
+  std::vector<tc::Byte> buffer4{8, tc::Byte{8}};
+  std::vector<tc::Byte> buffer5{16, tc::Byte{16}};
   // Setup entry
-  std::vector<boost::span<std::byte>> entry;
+  std::vector<boost::span<tc::Byte>> entry;
   // Add buffers to entry
   entry.push_back(buffer1);
   entry.push_back(buffer2);
@@ -788,8 +787,8 @@ TEST_F(RequestResponseCacheTest, TestParallelInsert)
   size_t cache_misses = 0;
   for (size_t idx = 0; idx < thread_count; idx++) {
     auto key = std::to_string(idx);
-    auto entry = tc::CacheEntry();
-    auto status = cache->Lookup(key, &entry);
+    auto entry = std::make_unique<tc::CacheEntry>();
+    auto status = cache->Lookup(key, entry.get());
     if (status.IsOk()) {
       cache_hits++;
     } else {
@@ -828,9 +827,10 @@ TEST_F(RequestResponseCacheTest, TestParallelLookup)
   size_t cache_hits = 0;
   size_t cache_misses = 0;
   for (size_t idx = 0; idx < thread_count; idx++) {
-    auto entry = tc::CacheEntry();
     auto key = std::to_string(idx);
-    auto status = cache->Lookup(key, &entry);
+    auto entry = std::make_unique<tc::CacheEntry>();
+    auto status = cache->Lookup(key, entry.get());
+
     if (status.IsOk()) {
       cache_hits++;
     } else {
diff --git a/src/tritoncache.cc b/src/tritoncache.cc
