Skip to content

Commit 06ca675

Browse files
Copilotjkotas
andauthored
Merge ThreadTasks into ThreadState, remove ThreadTasks (#127884)
`ThreadTasks` was a single-entry enum (`TT_CleanupSyncBlock`) backed by a separate `m_ThreadTasks` field on `Thread`, even though `ThreadState` had an unused bit (`0x00000020`) and already supported atomic set/reset via `InterlockedOr`/`InterlockedAnd` on `m_State`. The old code even contained a TODO noting the two should probably be merged. Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com> Co-authored-by: Jan Kotas <jkotas@microsoft.com>
1 parent 8b06ce0 commit 06ca675

2 files changed

Lines changed: 5 additions & 19 deletions

File tree

src/coreclr/vm/threads.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1290,11 +1290,9 @@ Thread::Thread()
12901290
m_fHasDeadThreadBeenConsideredForGCTrigger = false;
12911291
m_TraceCallCount = 0;
12921292
m_ThrewControlForThread = 0;
1293-
m_ThreadTasks = (ThreadTasks)0;
12941293

1295-
// The state and the tasks must be 32-bit aligned for atomicity to be guaranteed.
1294+
// The state must be 32-bit aligned for atomicity to be guaranteed.
12961295
_ASSERTE((((size_t) &m_State) & 3) == 0);
1297-
_ASSERTE((((size_t) &m_ThreadTasks) & 3) == 0);
12981296

12991297
// On all callbacks, call the trap code, which we now have
13001298
// wired to cause a GC. Thus we will do a GC on all Transition Frame Transitions (and more).

src/coreclr/vm/threads.h

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -511,7 +511,7 @@ class Thread
511511
TS_DebugSuspendPending = 0x00000008, // Is the debugger suspending threads?
512512
TS_GCOnTransitions = 0x00000010, // Force a GC on stub transitions (GCStress only)
513513

514-
// unused = 0x00000020,
514+
TS_SyncBlockCleanup = 0x00000020, // The synch block needs to be cleaned up.
515515

516516
TS_ExecutingOnAltStack = 0x00000040, // Runtime is executing on an alternate stack located anywhere in the memory
517517

@@ -570,17 +570,8 @@ class Thread
570570
TS_CatchAtSafePoint = (TS_AbortRequested | TS_DebugSuspendPending | TS_GCOnTransitions),
571571
};
572572

573-
// Thread flags that aren't really states in themselves but rather things the thread
574-
// has to do.
575-
enum ThreadTasks
576-
{
577-
TT_CleanupSyncBlock = 0x00000001, // The synch block needs to be cleaned up.
578-
};
579-
580573
// Thread flags that have no concurrency issues (i.e., they are only manipulated by the owning thread). Use these
581574
// state flags when you have a new thread state that doesn't belong in the ThreadState enum above.
582-
//
583-
// <TODO>@TODO: its possible that the ThreadTasks from above and these flags should be merged.</TODO>
584575
enum ThreadStateNoConcurrency
585576
{
586577
TSNC_Unknown = 0x00000000, // threads are initialized this way
@@ -710,19 +701,19 @@ class Thread
710701
DWORD RequireSyncBlockCleanup()
711702
{
712703
LIMITED_METHOD_CONTRACT;
713-
return (m_ThreadTasks & TT_CleanupSyncBlock);
704+
return (m_State & TS_SyncBlockCleanup);
714705
}
715706

716707
void SetSyncBlockCleanup()
717708
{
718709
LIMITED_METHOD_CONTRACT;
719-
InterlockedOr((LONG*)&m_ThreadTasks, TT_CleanupSyncBlock);
710+
InterlockedOr((LONG*)&m_State, TS_SyncBlockCleanup);
720711
}
721712

722713
void ResetSyncBlockCleanup()
723714
{
724715
LIMITED_METHOD_CONTRACT;
725-
InterlockedAnd((LONG*)&m_ThreadTasks, ~TT_CleanupSyncBlock);
716+
InterlockedAnd((LONG*)&m_State, ~TS_SyncBlockCleanup);
726717
}
727718

728719
#ifdef FEATURE_COMINTEROP_APARTMENT_SUPPORT
@@ -915,9 +906,6 @@ class Thread
915906

916907
inline TypeHandle GetTHAllocContextObj() {LIMITED_METHOD_CONTRACT; return m_thAllocContextObj; }
917908

918-
// Flags used to indicate tasks the thread has to do.
919-
ThreadTasks m_ThreadTasks;
920-
921909
// Flags for thread states that have no concurrency issues.
922910
ThreadStateNoConcurrency m_StateNC;
923911

0 commit comments

Comments
 (0)