Speed up SuperPMI mcs -removeDup

src/coreclr/src/ToolBox/superpmi/mcs/CMakeLists.txt

@@ -31,6 +31,7 @@ set(MCS_SOURCES
     ../superpmi-shared/callutils.cpp
     ../superpmi-shared/compileresult.cpp
     ../superpmi-shared/errorhandling.cpp
+    ../superpmi-shared/hash.cpp
     ../superpmi-shared/logging.cpp
     ../superpmi-shared/mclist.cpp
     ../superpmi-shared/methodcontext.cpp

src/coreclr/src/ToolBox/superpmi/mcs/verbremovedup.cpp

@@ -25,8 +25,10 @@ bool unique(MethodContext* mc)
     unsigned            newFlags = 0;
     mc->repCompileMethod(&newInfo, &newFlags);
 
-    char* md5Buff = new char[MD5_HASH_BUFFER_SIZE];
-    mc->dumpMethodMD5HashToBuffer(md5Buff, MD5_HASH_BUFFER_SIZE, /* ignoreMethodName */ true);
+    // Assume that there are lots of duplicates, so don't allocate a new buffer for the MD5 hash data
+    // until we know we're going to add it to the map.
+    char md5Buff[MD5_HASH_BUFFER_SIZE];
+    mc->dumpMethodMD5HashToBuffer(md5Buff, MD5_HASH_BUFFER_SIZE, /* ignoreMethodName */ true, &newInfo, newFlags);
 
     if (inFile->GetIndex(newInfo.ILCodeSize) == -1)
         inFile->Add(newInfo.ILCodeSize, new DenseLightWeightMap<char*>());
@@ -36,15 +38,15 @@ bool unique(MethodContext* mc)
     for (int i = 0; i < (int)ourRank->GetCount(); i++)
     {
         char* md5Buff2 = ourRank->Get(i);
-
         if (strncmp(md5Buff, md5Buff2, MD5_HASH_BUFFER_SIZE) == 0)
         {
-            delete[] md5Buff;
             return false;
         }
     }
 
-    ourRank->Append(md5Buff);
+    char* newmd5Buff = new char[MD5_HASH_BUFFER_SIZE];
+    memcpy(newmd5Buff, md5Buff, MD5_HASH_BUFFER_SIZE);
+    ourRank->Append(newmd5Buff);
     return true;
 }
 
@@ -130,6 +132,8 @@ int verbRemoveDup::DoWork(const char* nameOfInput, const char* nameOfOutput, boo
         }
     }
 
+    LogInfo("Loaded %d, Saved %d", mci.MethodContextNumber(), savedCount);
+
     // We're leaking 'inFile' or 'inFileLegacy', but the process is going away, so it shouldn't matter.
 
     if (CloseHandle(hFileOut) == 0)
@@ -138,8 +142,6 @@ int verbRemoveDup::DoWork(const char* nameOfInput, const char* nameOfOutput, boo
         return -1;
     }
 
-    LogInfo("Loaded %d, Saved %d", mci.MethodContextNumber(), savedCount);
-
     if (!mci.Destroy())
         return -1;
 

src/coreclr/src/ToolBox/superpmi/superpmi-shared/hash.cpp

@@ -0,0 +1,197 @@
+//
+// Copyright (c) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+//
+
+//----------------------------------------------------------
+// hash.cpp - Class for hashing a text stream using MD5 hashing
+//
+// Note that on Windows, acquiring the Crypto hash provider is expensive, so
+// only do that once and cache it.
+//----------------------------------------------------------
+
+#include "standardpch.h"
+#include "runtimedetails.h"
+#include "errorhandling.h"
+#include "md5.h"
+#include "hash.h"
+
+Hash::Hash()
+#ifndef TARGET_UNIX
+    : m_Initialized(false)
+    , m_hCryptProv(NULL)
+#endif // !TARGET_UNIX
+{
+}
+
+Hash::~Hash()
+{
+    Destroy(); // Ignoring return code.
+}
+
+// static
+bool Hash::Initialize()
+{
+#ifdef TARGET_UNIX
+
+    // No initialization necessary.
+
+#else // !TARGET_UNIX
+
+    if (m_Initialized)
+    {
+        LogError("Hash class has already been initialized");
+        return false;
+    }
+
+    // Get handle to the crypto provider
+    if (!CryptAcquireContextA(&m_hCryptProv, NULL, NULL, PROV_RSA_FULL, CRYPT_VERIFYCONTEXT))
+        goto OnError;
+
+    m_Initialized = true;
+    return true;
+
+OnError:
+    LogError("Failed to create a hash using the Crypto API (Error 0x%X)", GetLastError());
+
+    if (m_hCryptProv != NULL)
+        CryptReleaseContext(m_hCryptProv, 0);
+
+    m_Initialized = false;
+    return false;
+
+#endif // !TARGET_UNIX
+}
+
+// static
+bool Hash::Destroy()
+{
+#ifdef TARGET_UNIX
+
+    // No destruction necessary.
+
+#else // !TARGET_UNIX
+
+    // Should probably check Crypt() function return codes.
+    if (m_hCryptProv != NULL)
+    {
+        CryptReleaseContext(m_hCryptProv, 0);
+        m_hCryptProv = NULL;
+    }
+
+    m_Initialized = false;
+    return true;
+
+#endif // !TARGET_UNIX
+}
+
+// Hash::WriteHashValueAsText - Take a binary hash value in the array of bytes pointed to by
+// 'pHash' (size in bytes 'cbHash'), and write an ASCII hexadecimal representation of it in the buffer
+// 'hashTextBuffer' (size in bytes 'hashTextBufferLen').
+//
+// Returns true on success, false on failure (only if the arguments are bad).
+bool Hash::WriteHashValueAsText(const BYTE* pHash, size_t cbHash, char* hashTextBuffer, size_t hashTextBufferLen)
+{
+    // This could be:
+    //
+    // for (DWORD i = 0; i < MD5_HASH_BYTE_SIZE; i++)
+    // {
+    //    sprintf_s(hash + i * 2, hashLen - i * 2, "%02X", bHash[i]);
+    // }
+    //
+    // But this function is hot, and sprintf_s is too slow. This is a specialized function to speed it up.
+
+    if (hashTextBufferLen < 2 * cbHash + 1) // 2 characters for each byte, plus null terminator
+    {
+        LogError("WriteHashValueAsText doesn't have enough space to write the output");
+        return false;
+    }
+
+    static const char hexDigits[] = { '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'A', 'B', 'C', 'D', 'E', 'F' };
+    char* pCur = hashTextBuffer;
+    for (size_t i = 0; i < cbHash; i++)
+    {
+        unsigned digit = pHash[i];
+        unsigned lowNibble = digit & 0xF;
+        unsigned highNibble = digit >> 4;
+        *pCur++ = hexDigits[highNibble];
+        *pCur++ = hexDigits[lowNibble];
+    }
+    *pCur++ = '\0';
+    return true;
+}
+
+// Hash::HashBuffer - Compute an MD5 hash of the data pointed to by 'pBuffer', of 'bufLen' bytes,
+// writing the hexadecimal ASCII text representation of the hash to the buffer pointed to by 'hash',
+// of 'hashLen' bytes in size, which must be at least MD5_HASH_BUFFER_SIZE bytes.
+//
+// Returns the number of bytes written, or -1 on error.
+int Hash::HashBuffer(BYTE* pBuffer, size_t bufLen, char* hash, size_t hashLen)
+{
+#ifdef TARGET_UNIX
+
+    MD5HASHDATA md5_hashdata;
+    MD5         md5_hasher;
+
+    if (hashLen < MD5_HASH_BUFFER_SIZE)
+        return -1;
+
+    md5_hasher.Hash(pBuffer, (ULONG)bufLen, &md5_hashdata);
+
+    DWORD md5_hashdata_size = sizeof(md5_hashdata.rgb) / sizeof(BYTE);
+    Assert(md5_hashdata_size == MD5_HASH_BYTE_SIZE);
+
+    if (!WriteHashValueAsText(md5_hashdata.rgb, md5_hashdata_size, hash, hashLen))
+        return -1;
+
+    return MD5_HASH_BUFFER_SIZE; // if we had success we wrote MD5_HASH_BUFFER_SIZE bytes to the buffer
+
+#else // !TARGET_UNIX
+
+    if (!m_Initialized)
+    {
+        LogError("Hash class not initialized");
+        return -1;
+    }
+
+    HCRYPTHASH hCryptHash;
+    BYTE       bHash[MD5_HASH_BYTE_SIZE];
+    DWORD      cbHash = MD5_HASH_BYTE_SIZE;
+
+    if (hashLen < MD5_HASH_BUFFER_SIZE)
+        return -1;
+
+    if (!CryptCreateHash(m_hCryptProv, CALG_MD5, 0, 0, &hCryptHash))
+        goto OnError;
+
+    if (!CryptHashData(hCryptHash, pBuffer, (DWORD)bufLen, 0))
+        goto OnError;
+
+    if (!CryptGetHashParam(hCryptHash, HP_HASHVAL, bHash, &cbHash, 0))
+        goto OnError;
+
+    if (cbHash != MD5_HASH_BYTE_SIZE)
+        goto OnError;
+
+    if (!WriteHashValueAsText(bHash, cbHash, hash, hashLen))
+        return -1;
+
+    // Clean up.
+    CryptDestroyHash(hCryptHash);
+    hCryptHash = NULL;
+
+    return MD5_HASH_BUFFER_SIZE; // if we had success we wrote MD5_HASH_BUFFER_SIZE bytes to the buffer
+
+OnError:
+    LogError("Failed to create a hash using the Crypto API (Error 0x%X)", GetLastError());
+
+    if (hCryptHash != NULL)
+    {
+        CryptDestroyHash(hCryptHash);
+        hCryptHash = NULL;
+    }
+
+    return -1;
+
+#endif // !TARGET_UNIX
+}

src/coreclr/src/ToolBox/superpmi/superpmi-shared/hash.h

@@ -0,0 +1,47 @@
+//
+// Copyright (c) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+//
+
+//----------------------------------------------------------
+// hash.h - Class for hashing a text stream using MD5 hashing
+//----------------------------------------------------------
+#ifndef _hash
+#define _hash
+
+#define MD5_HASH_BYTE_SIZE 16   // MD5 is 128-bit, so we need 16 bytes to store it
+#define MD5_HASH_BUFFER_SIZE 33 // MD5 is 128-bit, so we need 32 chars + 1 char to store null-terminator
+
+class Hash
+{
+public:
+
+    Hash();
+    ~Hash();
+
+    bool Initialize();
+    bool Destroy();
+
+    bool IsInitialized()
+    {
+#ifdef TARGET_UNIX
+        return true; // No initialization necessary.
+#else // TARGET_UNIX 
+        return m_Initialized;
+#endif // !TARGET_UNIX 
+
+    }
+
+    int HashBuffer(BYTE* pBuffer, size_t bufLen, char* hash, size_t hashLen);
+
+private:
+
+    bool WriteHashValueAsText(const BYTE* pHash, size_t cbHash, char* hashTextBuffer, size_t hashTextBufferLen);
+
+#ifndef TARGET_UNIX
+    bool m_Initialized;
+    HCRYPTPROV m_hCryptProv;
+#endif // !TARGET_UNIX
+};
+
+#endif