From d8b555a670fc4201152d550764ccb68985857950 Mon Sep 17 00:00:00 2001
From: Ahson Khan <ahson_ahmedk@yahoo.com>
Date: Thu, 22 Aug 2024 17:34:17 -0700
Subject: [PATCH] Optimize Uuid.ToString() by avoiding using sprintf to do the
 basic string transformation (#5925)

* Optimize Uuid.ToString() to use a cached value on multiple calls.

* Add tests which call ToString() multiple times.

* Optimize Uuid::ToString by avoiding using sprintf.

* Assert all byte values will fit into a hex character.

* Update comment in test.
---
 sdk/core/azure-core/src/uuid.cpp          | 56 +++++++++++++----------
 sdk/core/azure-core/test/ut/uuid_test.cpp | 20 ++++++++
 2 files changed, 52 insertions(+), 24 deletions(-)

diff --git a/sdk/core/azure-core/src/uuid.cpp b/sdk/core/azure-core/src/uuid.cpp
index 463a8bad4..9d4a443be 100644
--- a/sdk/core/azure-core/src/uuid.cpp
+++ b/sdk/core/azure-core/src/uuid.cpp
@@ -3,6 +3,8 @@
 
 #include "azure/core/uuid.hpp"
 
+#include "azure/core/azure_assert.hpp"
+
 #include <cstdio>
 #include <random>
 
@@ -16,35 +18,41 @@ static thread_local std::mt19937_64 randomGenerator(std::random_device{}());
 } // namespace
 #endif
 
+namespace {
+static char ByteToHexChar(uint8_t byte)
+{
+  if (byte <= 9)
+  {
+    return '0' + byte;
+  }
+
+  AZURE_ASSERT_MSG(
+      byte >= 10 && byte <= 15,
+      "It is expected, for a valid Uuid, to have byte values, where each of the two nibbles fit "
+      "into a hexadecimal character");
+
+  return 'a' + (byte - 10);
+}
+} // namespace
+
 namespace Azure { namespace Core {
   std::string Uuid::ToString() const
   {
-    // Guid is 36 characters
-    //  Add one byte for the \0
-    char s[37];
+    std::string s(36, '-');
 
-    std::snprintf(
-        s,
-        sizeof(s),
-        "%2.2x%2.2x%2.2x%2.2x-%2.2x%2.2x-%2.2x%2.2x-%2.2x%2.2x-%2.2x%2.2x%2.2x%2.2x%2.2x%2.2x",
-        m_uuid[0],
-        m_uuid[1],
-        m_uuid[2],
-        m_uuid[3],
-        m_uuid[4],
-        m_uuid[5],
-        m_uuid[6],
-        m_uuid[7],
-        m_uuid[8],
-        m_uuid[9],
-        m_uuid[10],
-        m_uuid[11],
-        m_uuid[12],
-        m_uuid[13],
-        m_uuid[14],
-        m_uuid[15]);
+    for (size_t i = 0, j = 0; j < s.size() && i < UuidSize; i++)
+    {
+      if (i == 4 || i == 6 || i == 8 || i == 10)
+      {
+        j++; // Add hyphens at the appropriate places
+      }
 
-    return std::string(s);
+      uint8_t highNibble = (m_uuid[i] >> 4) & 0x0F;
+      uint8_t lowNibble = m_uuid[i] & 0x0F;
+      s[j++] = ByteToHexChar(highNibble);
+      s[j++] = ByteToHexChar(lowNibble);
+    }
+    return s;
   }
 
   Uuid Uuid::CreateUuid()
diff --git a/sdk/core/azure-core/test/ut/uuid_test.cpp b/sdk/core/azure-core/test/ut/uuid_test.cpp
index e0d0fec1c..9aaaf89c8 100644
--- a/sdk/core/azure-core/test/ut/uuid_test.cpp
+++ b/sdk/core/azure-core/test/ut/uuid_test.cpp
@@ -16,12 +16,32 @@ TEST(Uuid, Basic)
   EXPECT_EQ(uuid.ToString().length(), 36);
 }
 
+TEST(Uuid, Roundtrip)
+{
+  std::array<uint8_t, 16U> uuidArray
+      = {97, 126, 195, 45, 41, 178, 70, 23, 142, 131, 221, 245, 20, 45, 215, 15};
+
+  auto uuid = Uuid::CreateFromArray(uuidArray);
+  std::string uuidString = uuid.ToString();
+  std::string expectedString = "617ec32d-29b2-4617-8e83-ddf5142dd70f";
+  EXPECT_EQ(expectedString, uuidString);
+
+  auto roundTrip = uuid.AsArray();
+  EXPECT_EQ(uuidArray, roundTrip);
+}
+
 TEST(Uuid, Transparent)
 {
   auto uuid1 = Uuid::CreateUuid();
   auto arrayUuid1(uuid1.AsArray());
   auto uuid2 = Azure::Core::Uuid::CreateFromArray(arrayUuid1);
   EXPECT_EQ(uuid1.ToString(), uuid2.ToString());
+
+  // Repeated calls of ToString() to validate the same values are returned, whether it is cached or
+  // not.
+  EXPECT_EQ(uuid1.ToString(), uuid2.ToString());
+  EXPECT_EQ(uuid1.ToString(), uuid1.ToString());
+  EXPECT_EQ(uuid2.ToString(), uuid2.ToString());
 }
 
 TEST(Uuid, Randomness)