[ion] 12810: Significantly improved performance of Critical Section code by using a static buffer. Increased debug info dramatically and, if enabled, leaks can be found with ease. Future revision will include Spincount support.
Significantly improved performance of Critical Section code by using a static buffer. Increased debug info dramatically and, if enabled, leaks can be found with ease. Future revision will include Spincount support. Modified: trunk/reactos/lib/ntdll/ldr/startup.c Modified: trunk/reactos/lib/ntdll/rtl/critical.c Modified: trunk/reactos/lib/shell32/changenotify.c Modified: trunk/reactos/lib/shell32/iconcache.c _____
Modified: trunk/reactos/lib/ntdll/ldr/startup.c --- trunk/reactos/lib/ntdll/ldr/startup.c 2005-01-05 00:08:45 UTC (rev 12809) +++ trunk/reactos/lib/ntdll/ldr/startup.c 2005-01-05 01:09:30 UTC (rev 12810) @@ -278,8 +278,11 @@
RtlResetRtlTranslations (&NlsTable);
NTHeaders = (PIMAGE_NT_HEADERS)(ImageBase + PEDosHeader->e_lfanew); -
+ /* Initialize Critical Section Data */ + RtlpInitDeferedCriticalSection(); + + /* Get number of processors */ Status = ZwQuerySystemInformation(SystemBasicInformation, &SystemInformation, @@ -304,10 +307,7 @@ DPRINT1("Failed to create process heap\n"); ZwTerminateProcess(NtCurrentProcess(),STATUS_UNSUCCESSFUL); } - - /* Initialize Critical Section Data */ - RtlpInitDeferedCriticalSection(); - + /* initalize peb lock support */ RtlInitializeCriticalSection (&PebLock); Peb->FastPebLock = &PebLock; _____
Modified: trunk/reactos/lib/ntdll/rtl/critical.c --- trunk/reactos/lib/ntdll/rtl/critical.c 2005-01-05 00:08:45 UTC (rev 12809) +++ trunk/reactos/lib/ntdll/rtl/critical.c 2005-01-05 01:09:30 UTC (rev 12810) @@ -21,11 +21,13 @@
/* FUNCTIONS *****************************************************************/
+#define MAX_STATIC_CS_DEBUG_OBJECTS 64 + static RTL_CRITICAL_SECTION RtlCriticalSectionLock; static LIST_ENTRY RtlCriticalSectionList; static BOOLEAN RtlpCritSectInitialized = FALSE; -//static RTL_CRITICAL_SECTION_DEBUG RtlpStaticDebugInfo[64]; -//static PRTL_CRITICAL_SECTION_DEBUG RtlpDebugInfoFreeList; +static RTL_CRITICAL_SECTION_DEBUG RtlpStaticDebugInfo[MAX_STATIC_CS_DEBUG_OBJECTS]; +static BOOLEAN RtlpDebugInfoFreeList[MAX_STATIC_CS_DEBUG_OBJECTS];
/*++ * RtlDeleteCriticalSection @@ -50,26 +52,26 @@ { NTSTATUS Status = STATUS_SUCCESS;
+ DPRINT("Deleting Critical Section: %x\n", CriticalSection); /* Close the Event Object Handle if it exists */ if (CriticalSection->LockSemaphore) { + + /* In case NtClose fails, return the status */ Status = NtClose(CriticalSection->LockSemaphore); + }
/* Protect List */ RtlEnterCriticalSection(&RtlCriticalSectionLock); - - /* Delete the Debug Data, if it exists */ - if (CriticalSection->DebugInfo) { + + /* Remove it from the list */ + RemoveEntryList(&CriticalSection->DebugInfo->ProcessLocksList); - /* Remove it from the list */ - RemoveEntryList(&CriticalSection->DebugInfo->ProcessLocksList); - - /* Free it */ - RtlFreeHeap(RtlGetProcessHeap(), 0, CriticalSection->DebugInfo); - } - /* Unprotect */ RtlLeaveCriticalSection(&RtlCriticalSectionLock); + + /* Free it */ + RtlpFreeDebugInfo(CriticalSection->DebugInfo);
/* Wipe it out */ RtlZeroMemory(CriticalSection, sizeof(RTL_CRITICAL_SECTION)); @@ -148,7 +150,6 @@ }
/* We don't own it, so we must wait for it */ - DPRINT ("Waiting\n"); RtlpWaitForCriticalSection(CriticalSection); }
@@ -208,7 +209,6 @@ DWORD SpinCount) { PRTL_CRITICAL_SECTION_DEBUG CritcalSectionDebugData; - PVOID Heap;
/* First things first, set up the Object */ DPRINT("Initializing Critical Section: %x\n", CriticalSection); @@ -217,56 +217,56 @@ CriticalSection->OwningThread = 0; CriticalSection->SpinCount = (NtCurrentPeb()->NumberOfProcessors > 1) ? SpinCount : 0;
+ /* Allocate the Debug Data */ + CritcalSectionDebugData = RtlpAllocateDebugInfo(); + DPRINT("Allocated Debug Data: %x inside Process: %x\n", + CritcalSectionDebugData, + NtCurrentTeb()->Cid.UniqueProcess); + + if (!CritcalSectionDebugData) { + + /* This is bad! */ + DPRINT1("Couldn't allocate Debug Data for: %x\n", CriticalSection); + return STATUS_NO_MEMORY; + } + + /* Set it up */ + CritcalSectionDebugData->Type = RTL_CRITSECT_TYPE; + CritcalSectionDebugData->ContentionCount = 0; + CritcalSectionDebugData->EntryCount = 0; + CritcalSectionDebugData->CriticalSection = CriticalSection; + CriticalSection->DebugInfo = CritcalSectionDebugData; + /* - * Now set up the Debug Data - * Think of a better way to allocate it, because the Heap Manager won't - * have any debug data since the Heap isn't initalized! - */ - if ((Heap = RtlGetProcessHeap())) { + * Add it to the List of Critical Sections owned by the process. + * If we've initialized the Lock, then use it. If not, then probably + * this is the lock initialization itself, so insert it directly. + */ + if ((CriticalSection != &RtlCriticalSectionLock) && (RtlpCritSectInitialized)) { + + DPRINT("Securely Inserting into ProcessLocks: %x, %x, %x\n", + &CritcalSectionDebugData->ProcessLocksList, + CriticalSection, + &RtlCriticalSectionList); + + /* Protect List */ + RtlEnterCriticalSection(&RtlCriticalSectionLock);
- CritcalSectionDebugData = RtlAllocateHeap(Heap, 0, sizeof(RTL_CRITICAL_SECTION_DEBUG)); - DPRINT("Allocated Critical Section Debug Data: %x\n", CritcalSectionDebugData); - CritcalSectionDebugData->Type = RTL_CRITSECT_TYPE; - CritcalSectionDebugData->ContentionCount = 0; - CritcalSectionDebugData->EntryCount = 0; - CritcalSectionDebugData->CriticalSection = CriticalSection; - CriticalSection->DebugInfo = CritcalSectionDebugData; + /* Add this one */ + InsertTailList(&RtlCriticalSectionList, &CritcalSectionDebugData->ProcessLocksList);
- /* - * Add it to the List of Critical Sections owned by the process. - * If we've initialized the Lock, then use it. If not, then probably - * this is the lock initialization itself, so insert it directly. - */ - if ((CriticalSection != &RtlCriticalSectionLock) && (RtlpCritSectInitialized)) { - - DPRINT("Securely Inserting into ProcessLocks: %x, %x\n", - &CritcalSectionDebugData->ProcessLocksList, - CriticalSection); - - /* Protect List */ - RtlEnterCriticalSection(&RtlCriticalSectionLock); + /* Unprotect */ + RtlLeaveCriticalSection(&RtlCriticalSectionLock); + + } else {
- /* Add this one */ - InsertTailList(&RtlCriticalSectionList, &CritcalSectionDebugData->ProcessLocksList); - - /* Unprotect */ - RtlLeaveCriticalSection(&RtlCriticalSectionLock); + DPRINT("Inserting into ProcessLocks: %x, %x, %x\n", + &CritcalSectionDebugData->ProcessLocksList, + CriticalSection, + &RtlCriticalSectionList);
- } else { - - DPRINT("Inserting into ProcessLocks: %x, %x\n", - &CritcalSectionDebugData->ProcessLocksList, - CriticalSection); - - /* Add it directly */ - InsertTailList(&RtlCriticalSectionList, &CritcalSectionDebugData->ProcessLocksList); - } - - } else { - - DPRINT1("No Debug Data was allocated for: %x\n", CriticalSection); - /* This shouldn't happen... */ - CritcalSectionDebugData = NULL; + /* Add it directly */ + InsertTailList(&RtlCriticalSectionList, &CritcalSectionDebugData->ProcessLocksList); }
return STATUS_SUCCESS; @@ -397,16 +397,17 @@ }
/* Increase the Debug Entry count */ - DPRINT("Waiting on Critical Section: %x\n", CriticalSection); - if (CriticalSection->DebugInfo) CriticalSection->DebugInfo->EntryCount++; + DPRINT("Waiting on Critical Section Event: %x %x\n", + CriticalSection, + CriticalSection->LockSemaphore); + CriticalSection->DebugInfo->EntryCount++;
for (;;) {
/* Increase the number of times we've had contention */ - if (CriticalSection->DebugInfo) CriticalSection->DebugInfo->ContentionCount++; + CriticalSection->DebugInfo->ContentionCount++;
/* Wait on the Event */ - DPRINT("Waiting on Event: %x\n", CriticalSection->LockSemaphore); Status = NtWaitForSingleObject(CriticalSection->LockSemaphore, FALSE, &Timeout); @@ -470,13 +471,17 @@ }
/* Signal the Event */ - DPRINT("Signaling CS Event\n"); + DPRINT("Signaling Critical Section Event: %x, %x\n", + CriticalSection, + CriticalSection->LockSemaphore); Status = NtSetEvent(CriticalSection->LockSemaphore, NULL);
if (!NT_SUCCESS(Status)) {
/* We've failed */ - DPRINT1("Signaling Failed\n"); + DPRINT1("Signaling Failed for: %x, %x\n", + CriticalSection, + CriticalSection->LockSemaphore); RtlRaiseStatus(Status); } } @@ -508,7 +513,6 @@ /* Chevk if we have an event */ if (!hEvent) {
- DPRINT("Creating Event\n"); /* No, so create it */ if (!NT_SUCCESS(Status = NtCreateEvent(&hNewEvent, EVENT_ALL_ACCESS, @@ -522,23 +526,16 @@ RtlRaiseStatus(Status); return; } + DPRINT("Created Event: %x \n", hNewEvent);
- if (!(hEvent = InterlockedCompareExchangePointer((PVOID*)&CriticalSection->LockSemaphor e, + if ((hEvent = InterlockedCompareExchangePointer((PVOID*)&CriticalSection->LockSemaphor e,
(PVOID)hNewEvent, 0))) {
- /* We created a new event succesffuly */ - hEvent = hNewEvent; - } else { - /* Some just created an event */ - DPRINT("Closing already created event!\n"); + DPRINT("Closing already created event: %x\n", hNewEvent); NtClose(hNewEvent); } - - /* Set either the new or the old */ - CriticalSection->LockSemaphore = hEvent; - DPRINT("Event set!\n"); }
return; @@ -567,7 +564,7 @@
/* Initialize the Process Critical Section List */ InitializeListHead(&RtlCriticalSectionList); - + /* Initialize the CS Protecting the List */ RtlInitializeCriticalSection(&RtlCriticalSectionLock);
@@ -575,4 +572,98 @@ RtlpCritSectInitialized = TRUE; }
+/*++ + * RtlpAllocateDebugInfo + * + * Finds or allocates memory for a Critical Section Debug Object + * + * Params: + * None + * + * Returns: + * A pointer to an empty Critical Section Debug Object. + * + * Remarks: + * For optimization purposes, the first 64 entries can be cached. From + * then on, future Critical Sections will allocate memory from the heap. + * + *--*/ +PRTL_CRITICAL_SECTION_DEBUG +STDCALL +RtlpAllocateDebugInfo( + VOID) +{ + ULONG i; + + /* Try to allocate from our buffer first */ + for (i = 0; i < MAX_STATIC_CS_DEBUG_OBJECTS; i++) { + + /* Check if Entry is free */ + if (!RtlpDebugInfoFreeList[i]) { + + /* Mark entry in use */ + DPRINT("Using entry: %d. Buffer: %x\n", i, &RtlpStaticDebugInfo[i]); + RtlpDebugInfoFreeList[i] = TRUE; + + /* Use free entry found */ + return &RtlpStaticDebugInfo[i]; + } + + } + + /* We are out of static buffer, allocate dynamic */ + return RtlAllocateHeap(NtCurrentPeb()->ProcessHeap, + 0, + sizeof(RTL_CRITICAL_SECTION_DEBUG)); +} + +/*++ + * RtlpFreeDebugInfo + * + * Frees the memory for a Critical Section Debug Object + * + * Params: + * DebugInfo - Pointer to Critical Section Debug Object to free. + * + * Returns: + * None. + * + * Remarks: + * If the pointer is part of the static buffer, then the entry is made + * free again. If not, the object is de-allocated from the heap. + * + *--*/ +VOID +STDCALL +RtlpFreeDebugInfo( + PRTL_CRITICAL_SECTION_DEBUG DebugInfo) +{ + ULONG EntryId; + + /* Is it part of our cached entries? */ + if ((DebugInfo >= RtlpStaticDebugInfo) && + (DebugInfo <= &RtlpStaticDebugInfo[MAX_STATIC_CS_DEBUG_OBJECTS-1])) { + + /* Yes. zero it out */ + RtlZeroMemory(DebugInfo, sizeof(RTL_CRITICAL_SECTION_DEBUG)); + + /* Mark as free */ + EntryId = (DebugInfo - RtlpStaticDebugInfo); + DPRINT("Freeing from Buffer: %x. Entry: %d inside Process: %x\n", + DebugInfo, + EntryId, + NtCurrentTeb()->Cid.UniqueProcess); + RtlpDebugInfoFreeList[EntryId] = FALSE; + + } else { + + /* It's a dynamic one, so free from the heap */ + DPRINT("Freeing from Heap: %x inside Process: %x\n", + DebugInfo, + NtCurrentTeb()->Cid.UniqueProcess); + RtlFreeHeap(NtCurrentPeb()->ProcessHeap, 0, DebugInfo); + + } +} + /* EOF */ _____
Modified: trunk/reactos/lib/shell32/changenotify.c --- trunk/reactos/lib/shell32/changenotify.c 2005-01-05 00:08:45 UTC (rev 12809) +++ trunk/reactos/lib/shell32/changenotify.c 2005-01-05 01:09:30 UTC (rev 12810) @@ -180,8 +180,6 @@
DeleteNode( head );
LeaveCriticalSection(&SHELL32_ChangenotifyCS); - - DeleteCriticalSection(&SHELL32_ChangenotifyCS); }
/*********************************************************************** ** _____
Modified: trunk/reactos/lib/shell32/iconcache.c --- trunk/reactos/lib/shell32/iconcache.c 2005-01-05 00:08:45 UTC (rev 12809) +++ trunk/reactos/lib/shell32/iconcache.c 2005-01-05 01:09:30 UTC (rev 12810) @@ -292,7 +292,6 @@
ShellBigIconList = 0;
LeaveCriticalSection(&SHELL32_SicCS); - DeleteCriticalSection(&SHELL32_SicCS); }
/*********************************************************************** **
-
ion＠svn.reactos.com