Address code review feedback
authorSergiy Kuryata <sergeyk@microsoft.com>
Wed, 2 Dec 2015 23:23:37 +0000 (15:23 -0800)
committerSergiy Kuryata <sergeyk@microsoft.com>
Wed, 2 Dec 2015 23:23:37 +0000 (15:23 -0800)
src/inc/switches.h
src/pal/src/include/pal/virtual.h
src/pal/src/map/virtual.cpp

index dd4da27..27f528c 100644 (file)
 
 #elif defined(_TARGET_AMD64_) || defined(_TARGET_ARM64_)
     #define PAGE_SIZE               0x1000
-    #define USE_UPPER_ADDRESS       1
     #define UPPER_ADDRESS_MAPPING_FACTOR 2
     #define CLR_UPPER_ADDRESS_MIN   0x64400000000
     #define CODEHEAP_START_ADDRESS  0x64480000000
     #define CLR_UPPER_ADDRESS_MAX   0x644FC000000
 
+#if !defined(FEATURE_PAL)
+    #define USE_UPPER_ADDRESS       1
+#else
+    #define USE_UPPER_ADDRESS       0
+#endif // !FEATURE_PAL
+
 #else
     #error Please add a new #elif clause and define all portability macros for the new platform
 #endif
index 7596813..161840a 100644 (file)
@@ -106,90 +106,97 @@ BOOL VIRTUALOwnedRegion( IN UINT_PTR address );
 #ifdef __cplusplus
 }
 
-/// <summary>
-/// This class implements a virtual memory allocator for JIT'ed code.
-/// The purpose of this allocator is to opportunistically reserve a chuck of virtual memory
-/// that is located near the coreclr library (within 2GB range) that can be later used by
-/// JIT. Having executable memory close to the coreclr library allows JIT to generate more
-/// efficient code (by avoiding usage of jump stubs) and thus it can significantly improve
-/// performance of the application.
-///
-/// This allocator is integrated with the VirtualAlloc/Reserve code. If VirtualAlloc has been
-/// called with the MEM_RESERVE_EXECUTABLE flag then it will first try to obtain the requested size
-/// of virtual memory from ExecutableMemoryAllocator. If ExecutableMemoryAllocator runs out of
-/// the reserved memory (or fails to allocate it during initialization) then VirtualAlloc/Reserve code
-/// will simply fall back to reserving memory using OS APIs.
-///
-/// Notes:
-///     - the memory allocated by this class is NOT committed by default. It is responsibility
-///       of the caller to commit the virtual memory before accessing it.
-///     - in addition, this class does not provide ability to free the reserved memory. The caller
-///       has full control of the memory it got from this allocator (i.e. the caller becomes
-///       the owner of the allocated memory), so it is caller's responsibility to free the memory
-///       if it is no longer needed.
-/// </summary>
+/*++
+Class:
+    ExecutableMemoryAllocator
+
+    This class implements a virtual memory allocator for JIT'ed code.
+    The purpose of this allocator is to opportunistically reserve a chunk of virtual memory
+    that is located near the coreclr library (within 2GB range) that can be later used by
+    JIT. Having executable memory close to the coreclr library allows JIT to generate more
+    efficient code (by avoiding usage of jump stubs) and thus it can significantly improve
+    performance of the application.
+
+    This allocator is integrated with the VirtualAlloc/Reserve code. If VirtualAlloc has been
+    called with the MEM_RESERVE_EXECUTABLE flag then it will first try to obtain the requested size
+    of virtual memory from ExecutableMemoryAllocator. If ExecutableMemoryAllocator runs out of
+    the reserved memory (or fails to allocate it during initialization) then VirtualAlloc/Reserve code
+    will simply fall back to reserving memory using OS APIs.
+
+    Notes:
+        - the memory allocated by this class is NOT committed by default. It is responsibility
+          of the caller to commit the virtual memory before accessing it.
+        - in addition, this class does not provide ability to free the reserved memory. The caller
+          has full control of the memory it got from this allocator (i.e. the caller becomes
+          the owner of the allocated memory), so it is caller's responsibility to free the memory
+          if it is no longer needed.
+--*/
 class ExecutableMemoryAllocator
 {
 public:
-    /// <summary>
-    /// This function initializes the allocator. It should be called early during process startup
-    /// (when process address space is pretty much empty) in order to have a chance to reserve
-    /// sufficient amount of memory that is close to the coreclr library.
-    /// </summary>
+    /*++
+    Function:
+        Initialize
+
+        This function initializes the allocator. It should be called early during process startup
+        (when process address space is pretty much empty) in order to have a chance to reserve
+        sufficient amount of memory that is close to the coreclr library.
+    --*/
     void Initialize();
 
-    /// <summary>
-    /// This function attempts to allocate the requested amount of memory from its reserved virtual
-    /// address space. The function will return NULL if the allocation request cannot
-    /// be satisfied by the memory that is currently available in the allocator.
-    /// </summary>
-    LPVOID AllocateMemory(int32_t allocationSize);
+    /*++
+    Function:
+        AllocateMemory
+
+        This function attempts to allocate the requested amount of memory from its reserved virtual
+        address space. The function will return NULL if the allocation request cannot
+        be satisfied by the memory that is currently available in the allocator.
+    --*/
+    void* AllocateMemory(SIZE_T allocationSize);
 
 private:
-    /// <summary>
-    /// This function is called during initialization. It opportunistically tries to reserve
-    /// a large chunk of virtual memory that can be later used to store JIT'ed code.
-    /// </summary>
+    /*++
+    Function:
+        TryReserveInitialMemory
+
+        This function is called during initialization. It opportunistically tries to reserve
+        a large chunk of virtual memory that can be later used to store JIT'ed code.
+    --*/
     void TryReserveInitialMemory();
 
-    /// <summary>
-    /// This function returns a random offset (in multiples of the virtual page size)
-    /// at which the allocator should start allocating memory from its reserved memory range.
-    /// </summary>
+    /*++
+    Function:
+        GenerateRandomStartOffset
+
+        This function returns a random offset (in multiples of the virtual page size)
+        at which the allocator should start allocating memory from its reserved memory range.
+    --*/
     int32_t GenerateRandomStartOffset();
 
 private:
-    /// <summary>
-    /// There does not seem to be an easy way find the size of a library on Unix.
-    /// So this constant represents an approximation of the libcoreclr size (on debug build)
-    /// that can be used to calculate an approximate location of the memory that
-    /// is in 2GB range from the coreclr library. In addition, having precise size of libcoreclr
-    /// is not necessary for the calculations.
-    /// </summary>
+    // There does not seem to be an easy way find the size of a library on Unix.
+    // So this constant represents an approximation of the libcoreclr size (on debug build)
+    // that can be used to calculate an approximate location of the memory that
+    // is in 2GB range from the coreclr library. In addition, having precise size of libcoreclr
+    // is not necessary for the calculations.
     const int32_t CoreClrLibrarySize = 100 * 1024 * 1024;
 
-    /// <summary>
-    /// This constant represent the max size of the virtual memory that this allocator
-    /// will try to reserve during initialization. We want all JIT-ed code and the
-    /// entire libcoreclr to be located in a 2GB range.
-    /// </summary>
+    // This constant represent the max size of the virtual memory that this allocator
+    // will try to reserve during initialization. We want all JIT-ed code and the
+    // entire libcoreclr to be located in a 2GB range.
     const int32_t MaxExecutableMemorySize = 0x7FFF0000 - CoreClrLibrarySize;
 
-    /// <summary>Start address of the reserved virtual address space</summary>
-    LPVOID m_startAddress;
+    // Start address of the reserved virtual address space
+    void* m_startAddress;
 
-    /// <summary>Next available address in the reserved address space</summary>
-    LPVOID m_nextFreeAddress;
+    // Next available address in the reserved address space
+    void* m_nextFreeAddress;
 
-    /// <summary>
-    /// Total size of the virtual memory that the allocator has been able to
-    /// reserve during its initialization.
-    /// </summary>
+    // Total size of the virtual memory that the allocator has been able to
+    // reserve during its initialization.
     int32_t m_totalSizeOfReservedMemory;
 
-    /// <summary>
-    /// Remaining size of the reserved virtual memory that can be used to satisfy allocation requests.
-    /// </summary>
+    // Remaining size of the reserved virtual memory that can be used to satisfy allocation requests.
     int32_t m_remainingReservedMemory;
 };
 
index d1a389e..01de2bb 100644 (file)
@@ -92,21 +92,22 @@ static int gBackingFile = -1;
 #define MAP_ANON MAP_ANONYMOUS
 #endif
 
-/******
- *
- *  ReserveVirtualMemory() - Helper function that is used by Virtual* APIs
- *  and ExecutableMemoryAllocator to reserve virtual memory from the OS.
- *
- */
+/*++
+Function:
+    ReserveVirtualMemory()
+
+    Helper function that is used by Virtual* APIs and ExecutableMemoryAllocator
+    to reserve virtual memory from the OS.
+
+--*/
 static LPVOID ReserveVirtualMemory(
                 IN CPalThread *pthrCurrent, /* Currently executing thread */
                 IN LPVOID lpAddress,        /* Region to reserve or commit */
                 IN SIZE_T dwSize);          /* Size of Region */
 
-/// <summary>
-/// A memory allocator that allocates memory from a pre-reserved region
-/// of virtual memory that is located near the coreclr library.
-/// </summary>
+
+// A memory allocator that allocates memory from a pre-reserved region
+// of virtual memory that is located near the coreclr library.
 static ExecutableMemoryAllocator g_executableMemoryAllocator;
 
 /*++
@@ -2352,11 +2353,15 @@ ResetWriteWatch(
     return 1;
 }
 
-/// <summary>
-/// This function initializes the allocator. It should be called early during process startup
-/// (when process address space is pretty much empty) in order to have a chance to reserve
-/// sufficient amount of memory that is close to the coreclr library.
-/// </summary>
+/*++
+Function:
+    ExecutableMemoryAllocator::Initialize()
+
+    This function initializes the allocator. It should be called early during process startup
+    (when process address space is pretty much empty) in order to have a chance to reserve
+    sufficient amount of memory that is close to the coreclr library.
+
+--*/
 void ExecutableMemoryAllocator::Initialize()
 {
     m_startAddress = NULL;
@@ -2372,10 +2377,14 @@ void ExecutableMemoryAllocator::Initialize()
 
 }
 
-/// <summary>
-/// This function is called during PAL initialization. It opportunistically tries to reserve
-/// a large chunk of virtual memory that can be later used to store JIT'ed code.
-/// </summary>
+/*++
+Function:
+    ExecutableMemoryAllocator::TryReserveInitialMemory()
+
+    This function is called during PAL initialization. It opportunistically tries to reserve
+    a large chunk of virtual memory that can be later used to store JIT'ed code.\
+
+--*/
 void ExecutableMemoryAllocator::TryReserveInitialMemory()
 {
     CPalThread* pthrCurrent = InternalGetCurrentThread();
@@ -2398,7 +2407,7 @@ void ExecutableMemoryAllocator::TryReserveInitialMemory()
     // the OS implementation, the library is usually loaded either at the end or at the start of the user
     // address space. If the library is loaded at low addresses then try to reserve memory above libcoreclr
     // (thus avoiding reserving memory below 4GB; besides some operating systems do not allow that).
-    // If libcorclr is loaded at high addresses then try to reserve memory below its location.
+    // If libcoreclr is loaded at high addresses then try to reserve memory below its location.
     coreclrLoadAddress = (UINT_PTR)PAL_GetSymbolModuleBase((void*)VirtualAlloc);
     if ((coreclrLoadAddress < 0xFFFFFFFF) || ((coreclrLoadAddress - MaxExecutableMemorySize) < 0xFFFFFFFF))
     {
@@ -2416,7 +2425,7 @@ void ExecutableMemoryAllocator::TryReserveInitialMemory()
     // Do actual memory reservation.
     do
     {
-        m_startAddress = ReserveVirtualMemory(pthrCurrent, (LPVOID)startAddress, sizeOfAllocation);
+        m_startAddress = ReserveVirtualMemory(pthrCurrent, (void*)startAddress, sizeOfAllocation);
         if (m_startAddress != NULL)
         {
             // Memory has been successfully reserved.
@@ -2424,7 +2433,7 @@ void ExecutableMemoryAllocator::TryReserveInitialMemory()
 
             // Randomize the location at which we start allocating from the reserved memory range.
             int32_t randomOffset = GenerateRandomStartOffset();
-            m_nextFreeAddress = (LPVOID)(((UINT_PTR)m_startAddress) + randomOffset);
+            m_nextFreeAddress = (void*)(((UINT_PTR)m_startAddress) + randomOffset);
             m_remainingReservedMemory = sizeOfAllocation - randomOffset;
             break;
         }
@@ -2436,17 +2445,20 @@ void ExecutableMemoryAllocator::TryReserveInitialMemory()
     } while (sizeOfAllocation >= MemoryProbingIncrement);
 }
 
-/// <summary>
-/// This function attempts to allocate the requested amount of memory from its reserved virtual
-/// address space. The function will return NULL if the allocation request cannot
-/// be satisfied by the memory that is currently available in the allocator.
-///
-/// Note: This function MUST be called with the virtual_critsec lock held.
-///
-/// </summary>
-LPVOID ExecutableMemoryAllocator::AllocateMemory(int32_t allocationSize)
+/*++
+Function:
+    ExecutableMemoryAllocator::AllocateMemory
+
+    This function attempts to allocate the requested amount of memory from its reserved virtual
+    address space. The function will return NULL if the allocation request cannot
+    be satisfied by the memory that is currently available in the allocator.
+
+    Note: This function MUST be called with the virtual_critsec lock held.
+
+--*/
+void* ExecutableMemoryAllocator::AllocateMemory(SIZE_T allocationSize)
 {
-    LPVOID allocatedMemory = NULL;
+    void* allocatedMemory = NULL;
 
     // Allocation size must be in multiples of the virtual page size.
     _ASSERTE((allocationSize & VIRTUAL_PAGE_MASK) == 0);
@@ -2456,7 +2468,7 @@ LPVOID ExecutableMemoryAllocator::AllocateMemory(int32_t allocationSize)
     if ((allocationSize > 0) && (allocationSize <= m_remainingReservedMemory))
     {
         allocatedMemory = m_nextFreeAddress;
-        m_nextFreeAddress = (LPVOID)(((UINT_PTR)m_nextFreeAddress) + allocationSize);
+        m_nextFreeAddress = (void*)(((UINT_PTR)m_nextFreeAddress) + allocationSize);
         m_remainingReservedMemory -= allocationSize;
 
     }
@@ -2464,10 +2476,14 @@ LPVOID ExecutableMemoryAllocator::AllocateMemory(int32_t allocationSize)
     return allocatedMemory;
 }
 
-/// <summary>
-/// This function returns a random offset (in multiples of the virtual page size)
-/// at which the allocator should start allocating memory from its reserved memory range.
-/// </summary>
+/*++
+Function:
+    ExecutableMemoryAllocator::GenerateRandomStartOffset()
+
+    This function returns a random offset (in multiples of the virtual page size)
+    at which the allocator should start allocating memory from its reserved memory range.
+
+--*/
 int32_t ExecutableMemoryAllocator::GenerateRandomStartOffset()
 {
     int32_t pageCount;