Use ranges for compilation thread IDs

[dsouzai]

This PR facilitates options processing post restore, specifically wrt to the number of compilation threads. In order to allow a user to change the number of compilation threads post restore, the JIT will likely have to create a set number of threads and then choose how many to actually allow to be activated.

However, because the current assumption is that the diagnostic thread ID is one more than the number of compilation threads, it complicates the code when taking into account that the number of compilation threads could change. Therefore, in this PR, the code is updated to use thread ID ranges rather than simply assuming the thread ID starts at 0 and goes up to the number of compilation threads.

[dsouzai]

@mpirvu could you please review?

[dsouzai]

I grepped for _numUsableCompilationThreads, _numCompThreads, getNumUsableCompilationThreads(), and getNumTotalCompilationThreads(), and updated all the relevant places. The risk of this change is low because the first comp thread ID is still 0.

[AlexeyKhrabrov]

JFYI in case it's helpful: see #12926 as a reference point for finding places in the code that iterate through compilation thread IDs.

[mpirvu]

Sometimes we have this form of iteration:
for (int32_t compId = 0; compId < _numCompThreads; compId++)

[dsouzai]

Sometimes we have this form of iteration:
for (int32_t compId = 0; compId < _numCompThreads; compId++)

Ah I missed one place where this happens; I'll update it accordingly.

The only place I found this form was in the _classUnloadMonitorHolders array, which doesn't need to worry about which ranges are comp threads and which are diagnostic; as long as we have a contiguous array of comp thread IDs (which will always have to be true), there's no issue.

[dsouzai]

@mpirvu good for review again.

[mpirvu]

J9::MonitorTable::readReleaseClassUnloadMonitor(int32_t compThreadIndex)
   {
   TR_ASSERT(compThreadIndex < _numCompThreads, "compThreadIndex is too high");
   if (_classUnloadMonitorHolders[compThreadIndex] > 0)
      {
      _classUnloadMonitorHolders[compThreadIndex]--;
      _classUnloadMonitor.exit_read();
      return _classUnloadMonitorHolders[compThreadIndex];
      }
   else
      {
      TR_ASSERT(false, "comp thread %d does not have classUnloadMonitor", compThreadIndex);
      return -1; // could not release monitor
      }
   }

_classUnloadMonitorHolders assumes that usable compilation threads (non-diagnostic) always start at 0.

_numCompThreads only refers to the non-diagnostic threads.
Diagnostic threads are counted with _numDiagnosticThreads

[dsouzai]

I took a closer look at the code, and it doesn't look like there's any assumption about the compThread ID having to start at 0. When the CRIU changes are added, allocInitClassUnloadMonitorHolders will have to be called with the total number of threads that will be allocated, so _numCompThreads will just be the size of the array.

In

openj9/runtime/compiler/infra/J9MonitorTable.cpp

Lines 233 to 240 in 219f2e8

	int32_t
	J9::MonitorTable::readAcquireClassUnloadMonitor(int32_t compThreadIndex)
	{
	_classUnloadMonitor.enter_read();
	// Need to determine the identity of the compilation thread
	TR_ASSERT(compThreadIndex < _numCompThreads, "compThreadIndex is too high");
	return ++(_classUnloadMonitorHolders[compThreadIndex]);
	}

as long as compThreadIndex < _numCompThreads there's no issue.

In

openj9/runtime/compiler/infra/J9MonitorTable.cpp

Lines 243 to 258 in 219f2e8

	int32_t
	J9::MonitorTable::readReleaseClassUnloadMonitor(int32_t compThreadIndex)
	{
	TR_ASSERT(compThreadIndex < _numCompThreads, "compThreadIndex is too high");
	if (_classUnloadMonitorHolders[compThreadIndex] > 0)
	{
	_classUnloadMonitorHolders[compThreadIndex]--;
	_classUnloadMonitor.exit_read();
	return _classUnloadMonitorHolders[compThreadIndex];
	}
	else
	{
	TR_ASSERT(false, "comp thread %d does not have classUnloadMonitor", compThreadIndex);
	return -1; // could not release monitor
	}
	}

the assert is again just checking that compThreadIndex < _numCompThreads. The if statement is ensuring that _classUnloadMonitorHolders[compThreadIndex] > 0 since at this point, if we're doing a read release, the count has to be greater than zero.

It should be noted that _numCompThreads here is a member of J9::MonitorTable and not TR::CompilationInfo. When J9::MonitorTable::allocInitClassUnloadMonitorHolders is called, it's called with the total number of threads (including diagnostic threads).

[mpirvu]

It should be noted that _numCompThreads here is a member of J9::MonitorTable and not TR::CompilationInfo. When J9::MonitorTable::allocInitClassUnloadMonitorHolders is called, it's called with the total number of threads (including diagnostic threads).

Ok. I missed that _numCompThreads in J9::MonitorTable has a different meaning than TR::CompilationInfo::_numThreads. In the first case is the total number of threads and in the latter case it's just the "usable" number of threads.

[mpirvu]

jenkins test sanity all jdk17